[vm, service] Remove unsafe querying across threads during Isolate::PrintJSON.

Note this would be unsafe even if under a safepoint operation because not all of the queried threads participate in safepoints.

TEST=tsan
Bug: #44304
Bug: #44385
Change-Id: I8156e8c6049165e5c53b66c3391f3e8a496ddaaf
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/175000
Commit-Queue: Ryan Macnak <[email protected]>
Reviewed-by: Alexander Aprelev <[email protected]>
Reviewed-by: Ben Konyi <[email protected]>

Author: rmacnak-google

pkg/vm_service/test/get_isolate_rpc_test.dart

@@ -22,8 +22,6 @@ var tests = <VMTest>[
     expect(result.pauseOnExit, isFalse);
     expect(result.pauseEvent.type, 'Event');
     expect(result.error, isNull);
-    expect(result.json['_numZoneHandles'], isPositive);
-    expect(result.json['_numScopedHandles'], isPositive);
     expect(result.rootLib, isNotNull);
     expect(result.libraries.length, isPositive);
     expect(result.libraries[0], isNotNull);

runtime/observatory/lib/models.dart

@@ -50,14 +50,12 @@ part 'src/models/objects/single_target_cache.dart';
 part 'src/models/objects/source_location.dart';
 part 'src/models/objects/subtype_test_cache.dart';
 part 'src/models/objects/target.dart';
-part 'src/models/objects/thread.dart';
 part 'src/models/objects/timeline.dart';
 part 'src/models/objects/timeline_event.dart';
 part 'src/models/objects/type_arguments.dart';
 part 'src/models/objects/unknown.dart';
 part 'src/models/objects/unlinked_call.dart';
 part 'src/models/objects/vm.dart';
-part 'src/models/objects/zone.dart';
 
 part 'src/models/repositories/allocation_profile.dart';
 part 'src/models/repositories/breakpoint.dart';

runtime/observatory/lib/src/elements/isolate_view.dart

@@ -116,7 +116,6 @@ class IsolateViewElement extends CustomElement implements Renderable {
   void render() {
     final uptime = new DateTime.now().difference(_isolate.startTime!);
     final libraries = _isolate.libraries!.toList();
-    final List<M.Thread> threads = _isolate.threads!.toList();
     children = <Element>[
       navBar(<Element>[
         new NavTopMenuElement(queue: _r.queue).element,
@@ -256,26 +255,6 @@ class IsolateViewElement extends CustomElement implements Renderable {
                     ..classes = ['memberValue']
                     ..text = '${_isolate.extensionRPCs}'
                 ],
-              new DivElement()
-                ..classes = ['memberItem']
-                ..children = <Element>[
-                  new DivElement()
-                    ..classes = ['memberName']
-                    ..text = 'allocated zone handle count',
-                  new DivElement()
-                    ..classes = ['memberValue']
-                    ..text = '${_isolate.numZoneHandles}'
-                ],
-              new DivElement()
-                ..classes = ['memberItem']
-                ..children = <Element>[
-                  new DivElement()
-                    ..classes = ['memberName']
-                    ..text = 'allocated scoped handle count',
-                  new DivElement()
-                    ..classes = ['memberValue']
-                    ..text = '${_isolate.numScopedHandles}'
-                ],
               new DivElement()
                 ..classes = ['memberItem']
                 ..children = <Element>[
@@ -289,19 +268,6 @@ class IsolateViewElement extends CustomElement implements Renderable {
                         ..text = 'object store'
                     ]
                 ],
-              new DivElement()
-                ..classes = ['memberItem']
-                ..children = <Element>[
-                  new DivElement()
-                    ..classes = ['memberName']
-                    ..text = 'zone capacity high watermark'
-                    ..title = '''The maximum amount of native zone memory
-                    allocated by the isolate over it\'s life.''',
-                  new DivElement()
-                    ..classes = ['memberValue']
-                    ..text = Utils.formatSize(_isolate.zoneHighWatermark)
-                    ..title = '${_isolate.zoneHighWatermark}B'
-                ],
               new BRElement(),
               new DivElement()
                 ..classes = ['memberItem']
@@ -324,21 +290,6 @@ class IsolateViewElement extends CustomElement implements Renderable {
                           .element
                     ]
                 ],
-              new DivElement()
-                ..classes = ['memberItem']
-                ..children = <Element>[
-                  new DivElement()
-                    ..classes = ['memberName']
-                    ..text = 'threads (${threads.length})',
-                  new DivElement()
-                    ..classes = ['memberValue']
-                    ..children = <Element>[
-                      (new CurlyBlockElement(queue: _r.queue)
-                            ..content =
-                                threads.map<Element>(_populateThreadInfo))
-                          .element
-                    ]
-                ]
             ],
           new HRElement(),
           new EvalBoxElement(_isolate, _isolate.rootLibrary!, _objects, _eval,
@@ -360,31 +311,6 @@ class IsolateViewElement extends CustomElement implements Renderable {
     ];
   }
 
-  DivElement _populateThreadInfo(M.Thread t) {
-    return new DivElement()
-      ..classes = ['indent']
-      ..children = <Element>[
-        new SpanElement()..text = '${t.id} ',
-        (new CurlyBlockElement(queue: _r.queue)
-              ..content = <Element>[
-                new DivElement()
-                  ..classes = ['indent']
-                  ..text = 'kind ${t.kindString}',
-                new DivElement()
-                  ..classes = ['indent']
-                  ..title = '${t.zoneHighWatermark}B'
-                  ..text = 'zone capacity high watermark '
-                      '${Utils.formatSize(t.zoneHighWatermark)}',
-                new DivElement()
-                  ..classes = ['indent']
-                  ..title = '${t.zoneCapacity}B'
-                  ..text = 'current zone capacity ' +
-                      '${Utils.formatSize(t.zoneCapacity)}',
-              ])
-            .element
-      ];
-  }
-
   Future _loadExtraData() async {
     _function = null;
     _rootScript = null;

runtime/observatory/lib/src/models/objects/isolate.dart

@@ -57,20 +57,6 @@ abstract class Isolate extends IsolateRef {
   /// [optional] The error that is causing this isolate to exit, if applicable.
   Error? get error;
 
-  /// The list of threads associated with this isolate.
-  Iterable<Thread>? get threads;
-
-  /// The maximum amount of zone memory in bytes allocated by the isolate in
-  /// all threads at a given time. Calculated using the high watermarks of each
-  /// thread alive when a thread is unscheduled.
-  int? get zoneHighWatermark;
-
-  /// The number of zone handles currently held by this isolate.
-  int? get numZoneHandles;
-
-  /// The number of scoped handles currently held by this isolate.
-  int? get numScopedHandles;
-
   /// The current pause on exception mode for this isolate.
   //ExceptionPauseMode get exceptionPauseMode;
 

runtime/observatory/lib/src/models/objects/thread.dart

@@ -238,9 +238,6 @@ abstract class ServiceObject implements M.ObjectRef {
       case 'SourceLocation':
         obj = new SourceLocation._empty(owner);
         break;
-      case '_Thread':
-        obj = new Thread._empty(owner);
-        break;
       case 'UnresolvedSourceLocation':
         obj = new UnresolvedSourceLocation._empty(owner);
         break;
@@ -1617,18 +1614,6 @@ class Isolate extends ServiceObjectOwner implements M.Isolate {
   DartError? error;
   SnapshotReader? _snapshotFetch;
 
-  List<Thread> get threads => _threads;
-  final List<Thread> _threads = <Thread>[];
-
-  int get zoneHighWatermark => _zoneHighWatermark;
-  int _zoneHighWatermark = 0;
-
-  int get numZoneHandles => _numZoneHandles;
-  int _numZoneHandles = 0;
-
-  int get numScopedHandles => _numScopedHandles;
-  int _numScopedHandles = 0;
-
   bool? isSystemIsolate;
 
   void _loadHeapSnapshot(ServiceEvent event) {
@@ -1731,23 +1716,6 @@ class Isolate extends ServiceObjectOwner implements M.Isolate {
     if (map['extensionRPCs'] != null) {
       for (String e in map['extensionRPCs']) extensionRPCs.add(e);
     }
-
-    threads.clear();
-    if (map['_threads'] != null) {
-      for (Thread t in map['_threads']) threads.add(t);
-    }
-
-    int currentZoneHighWatermark = 0;
-    for (var i = 0; i < threads.length; i++) {
-      currentZoneHighWatermark += threads[i].zoneHighWatermark!;
-    }
-
-    if (currentZoneHighWatermark > _zoneHighWatermark) {
-      _zoneHighWatermark = currentZoneHighWatermark;
-    }
-
-    _numZoneHandles = map['_numZoneHandles'];
-    _numScopedHandles = map['_numScopedHandles'];
   }
 
   Future<TagProfile> updateTagProfile() {
@@ -3285,60 +3253,6 @@ class Sentinel extends ServiceObject implements M.Sentinel {
   String get shortName => valueAsString;
 }
 
-class Thread extends ServiceObject implements M.Thread {
-  M.ThreadKind? get kind => _kind;
-  M.ThreadKind? _kind;
-  String? get kindString => _kindString;
-  String? _kindString;
-  int? get zoneHighWatermark => _zoneHighWatermark;
-  int? _zoneHighWatermark;
-  int? get zoneCapacity => _zoneCapacity;
-  int? _zoneCapacity;
-
-  Thread._empty(ServiceObjectOwner? owner) : super._empty(owner);
-
-  void _update(Map map, bool mapIsRef) {
-    String rawKind = map['kind'];
-
-    switch (rawKind) {
-      case "kUnknownTask":
-        _kind = M.ThreadKind.unknownTask;
-        _kindString = 'unknown';
-        break;
-      case "kMutatorTask":
-        _kind = M.ThreadKind.mutatorTask;
-        _kindString = 'mutator';
-        break;
-      case "kCompilerTask":
-        _kind = M.ThreadKind.compilerTask;
-        _kindString = 'compiler';
-        break;
-      case "kSweeperTask":
-        _kind = M.ThreadKind.sweeperTask;
-        _kindString = 'sweeper';
-        break;
-      case "kMarkerTask":
-        _kind = M.ThreadKind.markerTask;
-        _kindString = 'marker';
-        break;
-      default:
-        assert(false);
-    }
-
-    _zoneHighWatermark = int.parse(map['_zoneHighWatermark']);
-    _zoneCapacity = int.parse(map['_zoneCapacity']);
-  }
-}
-
-class Zone implements M.Zone {
-  int get capacity => _capacity;
-  int _capacity;
-  int get used => _used;
-  int _used;
-
-  Zone(this._capacity, this._used);
-}
-
 class Field extends HeapObject implements M.Field {
   // Library or Class.
   HeapObject? dartOwner;

runtime/observatory/lib/src/models/objects/zone.dart

@@ -183,14 +183,12 @@ observatory_sources = [
   "lib/src/models/objects/source_location.dart",
   "lib/src/models/objects/subtype_test_cache.dart",
   "lib/src/models/objects/target.dart",
-  "lib/src/models/objects/thread.dart",
   "lib/src/models/objects/timeline.dart",
   "lib/src/models/objects/timeline_event.dart",
   "lib/src/models/objects/type_arguments.dart",
   "lib/src/models/objects/unknown.dart",
   "lib/src/models/objects/unlinked_call.dart",
   "lib/src/models/objects/vm.dart",
-  "lib/src/models/objects/zone.dart",
   "lib/src/models/repositories/allocation_profile.dart",
   "lib/src/models/repositories/breakpoint.dart",
   "lib/src/models/repositories/class.dart",

runtime/observatory/lib/src/service/object.dart

@@ -25,8 +25,6 @@ var tests = <VMTest>[
     expect(result['pauseOnExit'], isFalse);
     expect(result['pauseEvent']['type'], equals('Event'));
     expect(result['error'], isNull);
-    expect(result['_numZoneHandles'], isPositive);
-    expect(result['_numScopedHandles'], isPositive);
     expect(result['rootLib']['type'], equals('@Library'));
     expect(result['libraries'].length, isPositive);
     expect(result['libraries'][0]['type'], equals('@Library'));

runtime/observatory/observatory_sources.gni

@@ -70,8 +70,6 @@ Future<Null> testeeBefore() async {
     Expect.isFalse(result['pauseOnExit']);
     Expect.equals(result['pauseEvent']['type'], 'Event');
     Expect.isNull(result['error']);
-    Expect.isTrue(result['_numZoneHandles'] > 0);
-    Expect.isTrue(result['_numScopedHandles'] > 0);
     Expect.equals(result['rootLib']['type'], '@Library');
     Expect.isTrue(result['libraries'].length > 0);
     Expect.equals(result['libraries'][0]['type'], '@Library');

runtime/observatory/tests/service/get_isolate_rpc_test.dart

@@ -50,14 +50,12 @@ part 'src/models/objects/single_target_cache.dart';
 part 'src/models/objects/source_location.dart';
 part 'src/models/objects/subtype_test_cache.dart';
 part 'src/models/objects/target.dart';
-part 'src/models/objects/thread.dart';
 part 'src/models/objects/timeline.dart';
 part 'src/models/objects/timeline_event.dart';
 part 'src/models/objects/type_arguments.dart';
 part 'src/models/objects/unknown.dart';
 part 'src/models/objects/unlinked_call.dart';
 part 'src/models/objects/vm.dart';
-part 'src/models/objects/zone.dart';
 
 part 'src/models/repositories/allocation_profile.dart';
 part 'src/models/repositories/breakpoint.dart';