dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.19k stars 1.57k forks source link

Ambiguous rules for Isolate message (Illegal argument in isolate message) #38964

Closed duzenko closed 3 years ago

duzenko commented 4 years ago

Consider the following (originally flutter) code

class ExternalClass {
//  final VoidCallback _internalClosure = () => print('zxc');
  anyMethod() => print('asd');
}

class Worker {
  final ExternalClass externalReference;
  final SendPort sendPort;
  Worker({this.externalReference, this.sendPort});
}

void backgroundWork(Worker worker) async {
  worker.externalReference?.anyMethod();
  worker.sendPort.send('qwe');
}

Future main() async {
  final externalObject = ExternalClass();

  ReceivePort receivePort = ReceivePort();
  Worker object1 = new Worker(
    externalReference: externalObject,
    sendPort: receivePort.sendPort,
  );
  await Isolate.spawn(backgroundWork, object1);
  receivePort.listen((data) {
    print(data);
  });
}

It works as intended. Now uncomment the line with the _internalClosure and now it throws an exception:

E/flutter (29643): [ERROR:flutter/lib/ui/ui_dart_state.cc(148)] Unhandled Exception: Invalid argument(s): Illegal argument in isolate message : (object is a closure - Function '<anonymous closure>':.)
E/flutter (29643): #0      Isolate._spawnFunction (dart:isolate-patch/isolate_patch.dart:549:55)
E/flutter (29643): #1      Isolate.spawn (dart:isolate-patch/isolate_patch.dart:389:7)
E/flutter (29643): <asynchronous suspension>
E/flutter (29643): #2      main (package:pavement/main.dart:32:17)
E/flutter (29643): <asynchronous suspension>
E/flutter (29643): #3      _runMainZoned.<anonymous closure>.<anonymous closure> (dart:ui/hooks.dart:229:25)
E/flutter (29643): #4      _rootRun (dart:async/zone.dart:1124:13)
E/flutter (29643): #5      _CustomZone.run (dart:async/zone.dart:1021:19)
E/flutter (29643): #6      _runZoned (dart:async/zone.dart:1516:10)
E/flutter (29643): #7      runZoned (dart:async/zone.dart:1500:12)
E/flutter (29643): #8      _runMainZoned.<anonymous closure> (dart:ui/hooks.dart:221:5)
E/flutter (29643): #9      _startIsolate.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:305:19)
E/flutter (29643): #10     _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:172:12)
E/flutter (29643): 

What exactly rule does this code with a private closure breaks? Why is the message so ambiguous? What classes can we and what can we not send into an isolate as a message? Where is it stated in the docs?

Note that the code above is the just a repro case. The real use case is much more complicated and took a significant effort to debug and analyze, as well as to create a repro case starting from a business flutter app.

The exception is thrown from the 'external' code that is already a head bump as it's not easy to see what condition fails. Where do I go looking for the source code for that part? Supposedly this is a dart VM?

srawlins commented 4 years ago

I'd guess that at a minimum, closures are not allowed. I'll let the VM team answer more generally.

duzenko commented 4 years ago

I'd guess that at a minimum, closures are not allowed. I'll let the VM team answer more generally.

Can't I access the externalObject instance via a global variable or a static field? What is the point of this limitation?

a-siva commented 4 years ago

The VM currently only allows closures of top level methods or static methods in a class to be sent in isolate messages. This is currently not documented here https://api.dartlang.org/stable/2.6.0/dart-isolate/SendPort/send.html, we should improve that documentation.

duzenko commented 4 years ago

@a-siva I'm sending an object instance, not a closure here.

a-siva commented 4 years ago

Sending an object entails serializing all it's fields, you are sending object1 which is a Worker object, one of it's fields is externalReference which is of Type ExternalClass and one of the fields of ExternalClass is a closure (_internalClosure).

duzenko commented 4 years ago

Why serialize at all here? I might want to send a ByteBuffer with 100MB of data to an isolate. What will happen to it? Are all objects linked to the message via member references get serialized as well? Does it mean that all changes made to passed objects are lost to the caller? What if I want to do some parallel processing on a single block of data? Does this 'serialization' happen on the main thread? How is it implemented? Where is its source code? Is there any way I can get 'regular' SMP in Dart? I.e. just use what the underlying platform (Android/iOS) already supports.

hutchings90 commented 4 years ago

@duzenko I tore my hair out trying to figure this out. What solved my issue is realizing that I could call .then((Isolate isolate) { ... }, thereby having access to my object's code.

I have no idea why a static or top-level function is required. Maybe it provides some useful capabilities, but, in my case, it was completely useless and misleading. In order to avoid that error, I just made a top-level function that has an empty body. Let me know if this helped, if you have questions, or if you need further clarification.

Here is an example of what I did:

topLevelSpawnEntryPoint(Map<dynamic, dynamic> message) {}

class MyState extends State<MyPage> {
    @override
    void initState() {
        Isolate.spawn(topLevelSpawnEntryPoint, {}).then((Isolate isolate) => proveItWorked());
    }

    proveItWorked() {
      print('It worked');
    }
}
duzenko commented 4 years ago

@duzenko I tore my hair out trying to figure this out. What solved my issue is

I think your case is different. You need a completion notification but I want to send an object reference that has complicated links to other objects which leads to unwanted serialization error.

rdnobrega commented 4 years ago

@duzenko Hi, I had a somewhat similar issue. My workaround was to use good old class derivation. If you came up with another solution I would be happy to hear. Here's the code:

import 'dart:async';
import 'dart:isolate';
import 'dart:io';

void main() async {
  var manager = await WorkManager.start((data) {
    stdout.write('[MAIN] RECEIVED: $data\n');
  }, Miner());

  await Future.delayed(Duration(seconds: 1));
  manager.send('sending message');
}

//Wrapper class for duplex comm
class WorkChannel {
  ReceivePort receiver;
  SendPort sender;
  //Just a wrapper
  send(dynamic data) => sender?.send(data);
  bool get isChannelReady => sender == null;
}

//Manager - the parent thread part of the communication
class WorkManager extends WorkChannel {
  Isolate workerTask;
  Function(dynamic) onMessageReceived;

  //setup duplex comm. receives sendport from minion
  setupMessage(dynamic data) {
    if (data is SendPort) {
      sender = data;
    } else {
      onMessageReceived(data);
    }
  }

  //creates a manager and registers a callback for when mainthread receives messages on the minion worker
  static Future<WorkManager> start(
      Function(dynamic data) onMessageReceived, Worker minion) async {
    var manager = WorkManager();
    manager.onMessageReceived = onMessageReceived;
    manager.receiver = ReceivePort()..listen(manager.setupMessage);
    minion.sender = manager.receiver.sendPort;
    manager.workerTask = await Isolate.spawn(_createWorkerTask, minion);
    return manager;
  }

  //isolate-thread code. Send back a receiver for the manager
  static _createWorkerTask(Worker worker) {
    worker.receiver = ReceivePort()..listen(worker.messageReceived);
    worker.send(worker.receiver.sendPort);
    worker.work();
  }
}

//boilerplate code to be overriden
abstract class Worker extends WorkChannel {
  work();
  messageReceived(dynamic data);
}

//example class
class Miner extends Worker {
  int id = 1;
  int counter = 0;
  @override
  messageReceived(data) {
    stdout.write('[MINER #$id] received: $data\n');
  }

  @override
  work() {
    Timer.periodic(Duration(seconds: 1), (timer) {
      send('Miner #$id working at ${counter++}');
    });
  }
}
szotp commented 4 years ago

Documentation is still confusing:

https://api.dart.dev/stable/2.6.0/dart-isolate/SendPort/send.html

it is also possible to send object instances (which would be copied in the process). This is currently only supported by the dart vm.

So it works in only command line apps? Only in flutter's debug mode? Does it work in the browser? https://flutter.dev/docs/cookbook/networking/background-parsing#notes-on-working-with-isolates

Isolates communicate by passing messages back and forth. These messages can be primitive values, such as null, num, bool, double, or String, or simple objects such as the List in this example.

So I guess flutter supports it fully, even o the web?

Also, how this can work without reflection in AOT mode?

mraleph commented 4 years ago

@szotp

So it works in only command line apps? Only in flutter's debug mode? Does it work in the browser?

As the comment states: it works everywhere where Dart VM is used. Which is in CLI and native Flutter applications irrespective of mode (debug, profile and release all use Dart VM).

@mit-mit do we have some term to replace Dart VM in that comment. Should we rewrite it to say "Dart Native Runtime"?

So I guess flutter supports it fully, even o the web?

Flutter Web is not a released thing - so documentation is not updated to reflect its limitations. Consider filing a bug on Flutter repo.

Also, how this can work without reflection in AOT mode?

Similarly to how GC works - you don't need to know a lot of things (e.g. field names), you need to know just how many fields are there and which ones are pointers and which ones are primitives.

szotp commented 4 years ago

Thanks for explanation, it makes sense now. I raised this topic because I've seen multiple people confused by this.

I think this method requires a good detailed explanation, including what can't be copied, what's the performance of copying, in which environments Dart VM is not used (Is this only in regular Dart transpiled to js?), etc.

mit-mit commented 4 years ago

do we have some term to replace Dart VM in that comment.

Yes, I'd use the term 'Dart Native platform' as on https://dart.dev/platforms

wustrong commented 3 years ago

Sending an object entails serializing all it's fields, you are sending object1 which is a Worker object, one of it's fields is externalReference which is of Type ExternalClass and one of the fields of ExternalClass is a closure (_internalClosure).

@a-siva what if I want to send a http request or socket to a new isolate ? (for less computating time)

mkustermann commented 3 years ago

@wustrong A http request or socket instance refers internally to a ReceivePort which cannot be sent to other isolates atm.

Since this issue is mainly about sending closures and better clarification of what can be sent, I'm going to merge it into the more general https://github.com/dart-lang/sdk/issues/46623. We plan on addressing the closure issue in the near future as part of that.

adamstyrc commented 2 years ago

I try to send WebSocket object via Isolate.spawn() as a part of List args, but then I get:

Invalid argument(s): Illegal argument in isolate message: (object extends NativeWrapper - Library:'dart:io' Class: _SecureFilter

Any solution to that?

duzenko commented 2 years ago

The way I understand their design, anything CPU-intensive should be done in native platform code (except Web, where that must happen on backend), So if your code is fast enough, do it on main thread, otherwise see above. I'm still yet to figure out a good use case for isolates, short of mining crypto.

adamstyrc commented 2 years ago

Ok, good to know. I suspected using Isolates for fetching data from websocket was an overkill.

mkustermann commented 2 years ago

In general it's a fine solution to move background work (especially if it does some non trivial synchronous work) to helper isolates and send results back to the main isolate - this avoids having longer pause times or other work interrupting the UI.

Though the helper isolate will have to make its own http client / websocket / ... (such resources cannot be sent across isolates) and the result message sent to the main isolate must not contain native resources either.

This is explicitly mentioned in the documentation of SendPort.send:

If the sender and receiver isolate share the same code (e.g. isolates created via Isolate.spawn), the transitive object graph of message can contain any object, with the following exceptions:

  • Objects with native resources (subclasses of e.g. NativeFieldWrapperClass1). A Socket object for example referrs internally to objects that have native resources attached and can therefore not be sent.
duzenko commented 2 years ago

non trivial synchronous work

For example?

mkustermann commented 2 years ago

@duzenko non trivial synchronous work

Many tasks can easily take up several milliseconds (utf8/json/protobuf decoding/encoding, ...). To maintain responsive UI the main isolate has < 16 ms to render a frame, so having too much synchronous work in there can cause drops in FPS.

duzenko commented 2 years ago

@duzenko non trivial synchronous work

Many tasks can easily take up several milliseconds (utf8/json/protobuf decoding/encoding, ...). To maintain responsive UI the main isolate has < 16 ms to render a frame, so having too much synchronous work in there can cause drops in FPS.

Last time I checked everything going in and out of isolate is force-converted to json. So decoding json in an isolate is guaranteed to be at least 2x slower than on main thread. Has anything changed there?

mkustermann commented 2 years ago

Last time I checked everything going in and out of isolate is force-converted to json.

There was never any conversion to json involved in inter-isolate messages. Though as part of https://github.com/dart-lang/sdk/issues/36097 we have made a number of improvements to isolates and the communication mechanisms.

Using flutter's compute() function - which builds uponIsolate.spawn() and Isolate.exit() - does not make any copies/conversions - though it does currently have a verification pass over the object graph that is sent (we disallow sending certain objects across ports - as stated in SendPort.send() documentation).

Using SendPort.send() does make one transitive copy of the mutable part of the object graph.

duzenko commented 2 years ago

Last time I checked everything going in and out of isolate is force-converted to json.

There was never any conversion to json involved in inter-isolate messages. Though as part of #36097 we have made a number of improvements to isolates and the communication mechanisms.

Using flutter's compute() function - which builds uponIsolate.spawn() and Isolate.exit() - does not make any copies/conversions - though it does currently have a verification pass over the object graph that is sent (we disallow sending certain objects across ports - as stated in SendPort.send() documentation).

Using SendPort.send() does make one transitive copy of the mutable part of the object graph.

Is Flutter compute 'special' in that way? If yes, we should not discuss it and focus on dart isolates as this repo is general dart.

If not, then how come the input data via Isolate.spawn serializes very fast while output of Isolate.exit takes a similar time to jsonEncode to pass?

mkustermann commented 2 years ago

If not, then how come the input data via Isolate.spawn serializes very fast while output of Isolate.exit takes a similar time to jsonEncode to pass?

Could you maybe file a new issue with example dart code you're having performance problems with? Then we can diagnose what happens and possibly make some improvements.

(Hypothesising: If the input to Isolate.spawn is a String it doesn't have to be copied (and Uint8List may be very fast to copy). As mentioned Isolate.exit() does a O(n) verification pass - which may explain why it's slower than you'd expect /cc @aam ).