Skip to content

Commit cc13f08

Browse files
rmacnak-googlecommit-bot@chromium.org
authored andcommitted
[observatory] Fix race in getCpuProfileTimeline.
This function would iterate vm.isolates twice with an async gap in between and expect no isolates to start or exit in the mean time. Change-Id: I1a79dd9494a4d0431b112db61c0660ce2df00cca Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/136560 Reviewed-by: Ben Konyi <[email protected]> Commit-Queue: Ryan Macnak <[email protected]>
1 parent 4ef4371 commit cc13f08

File tree

2 files changed

+13
-17
lines changed

2 files changed

+13
-17
lines changed

runtime/observatory/lib/src/repositories/timeline_base.dart

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,11 @@ class TimelineRepositoryBase {
1414
static const kTimeOriginMicros = 'timeOriginMicros';
1515
static const kTimeExtentMicros = 'timeExtentMicros';
1616

17-
Future<void> _formatSamples(
18-
M.Isolate isolate, Map traceObject, S.ServiceMap cpuSamples) async {
17+
Future<void> _formatSamples(M.Isolate isolate, Map traceObject,
18+
Future<S.ServiceObject> cpuSamples) async {
1919
const kRootFrameId = 0;
2020
final profile = SampleProfile();
21-
await profile.load(isolate as S.ServiceObjectOwner, cpuSamples);
21+
await profile.load(isolate as S.ServiceObjectOwner, await cpuSamples);
2222
final trie = profile.loadFunctionTree(M.ProfileTreeDirection.inclusive);
2323
final root = trie.root;
2424
int nextId = kRootFrameId;
@@ -66,24 +66,20 @@ class TimelineRepositoryBase {
6666
Future<Map> getCpuProfileTimeline(M.VMRef ref,
6767
{int timeOriginMicros, int timeExtentMicros}) async {
6868
final S.VM vm = ref as S.VM;
69-
final sampleRequests = <Future>[];
69+
final traceObject = <String, dynamic>{
70+
_kStackFrames: {},
71+
_kTraceEvents: [],
72+
};
7073

71-
for (final isolate in vm.isolates) {
72-
sampleRequests.add(vm.invokeRpc('getCpuSamples', {
74+
await Future.wait(vm.isolates.map((isolate) {
75+
final samples = vm.invokeRpc('getCpuSamples', {
7376
'isolateId': isolate.id,
7477
if (timeOriginMicros != null) kTimeOriginMicros: timeOriginMicros,
7578
if (timeExtentMicros != null) kTimeExtentMicros: timeExtentMicros,
76-
}));
77-
}
79+
});
80+
return _formatSamples(isolate, traceObject, samples);
81+
}));
7882

79-
final completed = await Future.wait(sampleRequests);
80-
final traceObject = <String, dynamic>{
81-
_kStackFrames: {},
82-
_kTraceEvents: [],
83-
};
84-
for (int i = 0; i < vm.isolates.length; ++i) {
85-
await _formatSamples(vm.isolates[i], traceObject, completed[i]);
86-
}
8783
return traceObject;
8884
}
8985

runtime/observatory/tests/service/get_zone_memory_info_rpc_test.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ var tests = <VMTest>[
1212
isInstanceOf<int> isInt = new isInstanceOf<int>();
1313
// Just iterate over all the isolates to confirm they have
1414
// the correct fields needed to examine zone memory usage.
15-
for (Isolate isolate in vm.isolates) {
15+
for (Isolate isolate in new List.from(vm.isolates)) {
1616
await isolate.reload();
1717
expect(isolate.zoneHighWatermark, isInt);
1818
expect(isolate.threads, isNotNull);

0 commit comments

Comments
 (0)