Skip to content

Commit 4f30b5c

Browse files
aamCommit Queue
authored and
Commit Queue
committed
[vm/isolates] Include retaining path into an illegal send argument exception.
The exception contains retaining path that looks like this: ``` Invalid argument(s): Illegal argument in isolate message: (object extends NativeWrapper - Library:'dart:io' Class: _RandomAccessFileOpsImpl@14069316) <- Library:'dart:io' Class: _RandomAccessFile@14069316 <- Library:'file:///vm/dart/isolates/send_unsupported_objects_test.dart' Class: SomeLog <- Library:'file:///vm/dart/isolates/send_unsupported_objects_test.dart' Class: SomeState <- Class: Context <- Library:'dart:core' Class: _Closure@0150898 ``` BUG=#51115 BUG=#48592 TEST=send_unsupported_objects_test Change-Id: I022e693adccf43a7d2c95e1c7283fd7f210cf1d7 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/280523 Commit-Queue: Alexander Aprelev <[email protected]> Reviewed-by: Martin Kustermann <[email protected]> Reviewed-by: Ryan Macnak <[email protected]>
1 parent 1f6d4ae commit 4f30b5c

14 files changed

+834
-39
lines changed

runtime/lib/isolate.cc

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,13 @@ static ObjectPtr ValidateMessageObject(Zone* zone,
278278

279279
const Array& args = Array::Handle(zone, Array::New(3));
280280
args.SetAt(0, illegal_object);
281-
args.SetAt(2, String::Handle(zone, String::New(exception_message)));
281+
args.SetAt(2, String::Handle(
282+
zone, String::NewFormatted(
283+
"%s%s",
284+
FindRetainingPath(
285+
zone, isolate, obj, illegal_object,
286+
TraversalRules::kInternalToIsolateGroup),
287+
exception_message)));
282288
const Object& exception = Object::Handle(
283289
zone, Exceptions::Create(Exceptions::kArgumentValue, args));
284290
return UnhandledException::New(Instance::Cast(exception),
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'dart:async';
6+
import 'dart:isolate';
7+
8+
import "package:async_helper/async_helper.dart";
9+
import 'package:expect/expect.dart';
10+
11+
import 'send_unsupported_objects_test.dart';
12+
13+
const NESTED_DEPTH = 500;
14+
Future<List> buildNestedList(List<dynamic> list, int level) async {
15+
final fu = level == NESTED_DEPTH ? Fu.unsendable("$level") : Fu("$level");
16+
final newlist = <dynamic>[list, fu];
17+
if (--level == 0) {
18+
return newlist;
19+
}
20+
return await buildNestedList(newlist, level);
21+
}
22+
23+
main() async {
24+
asyncStart();
25+
try {
26+
final nestedList = await buildNestedList(<dynamic>[], NESTED_DEPTH);
27+
// Send closure capturing nestedList
28+
await Isolate.spawn((arg) {
29+
arg();
30+
}, () {
31+
print('$nestedList');
32+
});
33+
} catch (e) {
34+
checkForRetainingPath(e, <String>[
35+
'NativeWrapper',
36+
'Baz',
37+
'Fu',
38+
'Closure',
39+
]);
40+
41+
final msg = e.toString();
42+
Expect.isTrue(msg.split('\n').length > NESTED_DEPTH * 2);
43+
asyncEnd();
44+
}
45+
}
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'dart:async';
6+
import 'dart:io';
7+
import 'dart:isolate';
8+
9+
import "package:async_helper/async_helper.dart";
10+
import 'package:expect/expect.dart';
11+
12+
import 'send_unsupported_objects_test.dart';
13+
14+
worker(SendPort sp) async {
15+
try {
16+
Isolate.exit(sp, Fu.unsendable('fu'));
17+
} catch (e) {
18+
checkForRetainingPath(e, <String>[
19+
'NativeWrapper',
20+
'Baz',
21+
'Fu',
22+
]);
23+
sp.send(true);
24+
}
25+
}
26+
27+
main() async {
28+
asyncStart();
29+
final rp = ReceivePort();
30+
await Isolate.spawn(worker, rp.sendPort);
31+
Expect.isTrue(await rp.first);
32+
asyncEnd();
33+
}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'dart:async';
6+
import 'dart:isolate';
7+
8+
import 'package:async_helper/async_helper.dart';
9+
import 'package:expect/expect.dart';
10+
11+
import 'send_unsupported_objects_test.dart';
12+
13+
main() async {
14+
asyncStart();
15+
16+
final fu = Fu.unsendable('fu');
17+
try {
18+
// Pass a closure that captures [fu]
19+
await Isolate.spawn((arg) {
20+
arg();
21+
}, () {
22+
print('${fu.label}');
23+
Expect.fail('This closure should fail to be sent, shouldn\'t be called');
24+
});
25+
} catch (e) {
26+
checkForRetainingPath(e, <String>[
27+
'NativeWrapper',
28+
'Baz',
29+
'Fu',
30+
'Context',
31+
'Closure',
32+
]);
33+
asyncEnd();
34+
}
35+
}
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
// Verifies retaining path in error message for spawnUri'ed worker attempt
6+
// to send regular class instance.
7+
8+
import 'dart:async';
9+
import 'dart:io';
10+
import 'dart:isolate';
11+
import 'dart:nativewrappers';
12+
13+
import 'package:async_helper/async_helper.dart';
14+
import 'package:expect/expect.dart';
15+
16+
import 'send_unsupported_objects_test.dart';
17+
18+
class ConstFoo {
19+
const ConstFoo(this.name);
20+
final String name;
21+
}
22+
23+
Future<void> main(args, message) async {
24+
if (message == null) {
25+
final receivePort = ReceivePort();
26+
final isolate = await Isolate.spawnUri(
27+
Platform.script, <String>['worker'], <SendPort>[receivePort.sendPort],
28+
errorsAreFatal: true);
29+
final result = await receivePort.first;
30+
Expect.equals('done', result);
31+
return;
32+
}
33+
34+
Expect.equals('worker', args[0]);
35+
final SendPort sendPort = message[0] as SendPort;
36+
Expect.throws(() {
37+
sendPort.send(<dynamic>[
38+
<dynamic>[
39+
<dynamic>[const ConstFoo("42")],
40+
],
41+
]);
42+
}, (e) {
43+
print(e);
44+
checkForRetainingPath(e, <String>['ConstFoo']);
45+
46+
final msg = e.toString();
47+
Expect.equals(3, msg.split('\n').where((s) => s.contains('_List')).length);
48+
return true;
49+
});
50+
sendPort.send('done');
51+
}
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'dart:async';
6+
import 'dart:io';
7+
import 'dart:isolate';
8+
import 'dart:nativewrappers';
9+
10+
import 'package:async_helper/async_helper.dart';
11+
import 'package:expect/expect.dart';
12+
13+
import 'send_unsupported_objects_init_isolate_test.dart';
14+
15+
class Foo {
16+
int i = 42;
17+
}
18+
19+
class Bar {
20+
Foo foo = Foo();
21+
}
22+
23+
class NativeClass extends NativeFieldWrapperClass1 {}
24+
25+
class Baz {
26+
@pragma('vm:entry-point') // prevent tree-shaking of the field.
27+
NativeClass? nativeClass;
28+
Baz();
29+
}
30+
31+
class Fu {
32+
String label;
33+
Bar bar = Bar();
34+
Baz baz = Baz();
35+
36+
Fu(this.label);
37+
Fu.unsendable(this.label) {
38+
baz.nativeClass = NativeClass();
39+
}
40+
}
41+
42+
void checkForRetainingPath(Object? e, List<String> list) {
43+
Expect.isTrue(e is ArgumentError);
44+
final msg = e.toString();
45+
list.forEach((s) {
46+
Expect.contains(s, msg);
47+
});
48+
}
49+
50+
main() async {
51+
asyncStart();
52+
final rp = ReceivePort();
53+
try {
54+
rp.sendPort.send(Fu.unsendable('fu'));
55+
} catch (e) {
56+
checkForRetainingPath(e, <String>[
57+
'NativeWrapper',
58+
'Baz',
59+
'Fu',
60+
]);
61+
}
62+
rp.close();
63+
asyncEnd();
64+
}
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
// @dart = 2.9
6+
7+
import 'dart:async';
8+
import 'dart:isolate';
9+
10+
import "package:async_helper/async_helper.dart";
11+
import 'package:expect/expect.dart';
12+
13+
import 'send_unsupported_objects_test.dart';
14+
15+
const NESTED_DEPTH = 500;
16+
Future<List> buildNestedList(List<dynamic> list, int level) async {
17+
final fu = level == NESTED_DEPTH ? Fu.unsendable("$level") : Fu("$level");
18+
final newlist = <dynamic>[list, fu];
19+
if (--level == 0) {
20+
return newlist;
21+
}
22+
return await buildNestedList(newlist, level);
23+
}
24+
25+
main() async {
26+
asyncStart();
27+
try {
28+
final nestedList = await buildNestedList(<dynamic>[], NESTED_DEPTH);
29+
// Send closure capturing nestedList
30+
await Isolate.spawn((arg) {
31+
arg();
32+
}, () {
33+
print('$nestedList');
34+
});
35+
} catch (e) {
36+
checkForRetainingPath(e, <String>[
37+
'NativeWrapper',
38+
'Baz',
39+
'Fu',
40+
'Closure',
41+
]);
42+
43+
final msg = e.toString();
44+
Expect.isTrue(msg.split('\n').length > NESTED_DEPTH * 2);
45+
asyncEnd();
46+
}
47+
}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
// @dart = 2.9
6+
7+
import 'dart:async';
8+
import 'dart:io';
9+
import 'dart:isolate';
10+
11+
import "package:async_helper/async_helper.dart";
12+
import 'package:expect/expect.dart';
13+
14+
import 'send_unsupported_objects_test.dart';
15+
16+
worker(SendPort sp) async {
17+
try {
18+
Isolate.exit(sp, Fu.unsendable('fu'));
19+
} catch (e) {
20+
checkForRetainingPath(e, <String>[
21+
'NativeWrapper',
22+
'Baz',
23+
'Fu',
24+
]);
25+
sp.send(true);
26+
}
27+
}
28+
29+
main() async {
30+
asyncStart();
31+
final rp = ReceivePort();
32+
await Isolate.spawn(worker, rp.sendPort);
33+
Expect.isTrue(await rp.first);
34+
asyncEnd();
35+
}
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
// @dart = 2.9
6+
7+
import 'dart:async';
8+
import 'dart:isolate';
9+
10+
import 'package:async_helper/async_helper.dart';
11+
import 'package:expect/expect.dart';
12+
13+
import 'send_unsupported_objects_test.dart';
14+
15+
main() async {
16+
asyncStart();
17+
18+
final fu = Fu.unsendable('fu');
19+
try {
20+
// Pass a closure that captures [fu]
21+
await Isolate.spawn((arg) {
22+
arg();
23+
}, () {
24+
print('${fu.label}');
25+
Expect.fail('This closure should fail to be sent, shouldn\'t be called');
26+
});
27+
} catch (e) {
28+
checkForRetainingPath(e, <String>[
29+
'NativeWrapper',
30+
'Baz',
31+
'Fu',
32+
'Context',
33+
'Closure',
34+
]);
35+
asyncEnd();
36+
}
37+
}

0 commit comments

Comments
 (0)