Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
duzenko opened this issue Oct 18, 2019 · 25 comments
Closed
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. library-isolate type-documentation A request to add or improve documentation type-question A question about expected behavior or functionality

Comments

@duzenko
Copy link

duzenko commented Oct 18, 2019

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?

@duzenko duzenko changed the title Ambiguous rules for compute message (Illegal argument in isolate message) Ambiguous rules for Isolate message (Illegal argument in isolate message) Oct 18, 2019
@srawlins
Copy link
Member

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

@srawlins srawlins added area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. library-isolate type-question A question about expected behavior or functionality labels Oct 18, 2019
@duzenko
Copy link
Author

duzenko commented Oct 19, 2019

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
Copy link
Contributor

a-siva commented Nov 7, 2019

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
Copy link
Author

duzenko commented Nov 7, 2019

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

@a-siva
Copy link
Contributor

a-siva commented Nov 7, 2019

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
Copy link
Author

duzenko commented Nov 8, 2019

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
Copy link

hutchings90 commented Mar 30, 2020

@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
Copy link
Author

duzenko commented Mar 30, 2020

@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
Copy link

rdnobrega commented Jul 9, 2020

@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
Copy link

szotp commented Aug 26, 2020

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
Copy link
Member

mraleph commented Aug 27, 2020

@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
Copy link

szotp commented Aug 27, 2020

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
Copy link
Member

mit-mit commented Aug 28, 2020

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

dart-bot pushed a commit that referenced this issue Sep 6, 2020
The comment referred to "dart vm" which confused users reading
documentation. Use "Dart Native" instead and link to the page on the
site describing it.

R=lrn@google.com, mit@google.com

Issue #38964

Change-Id: I50cf1b67d9e709d81a1ca7dced8da2f84488873e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/161781
Reviewed-by: Lasse R.H. Nielsen <lrn@google.com>
Commit-Queue: Vyacheslav Egorov <vegorov@google.com>
@lrhn lrhn added the type-documentation A request to add or improve documentation label Oct 5, 2020
@wustrong
Copy link

wustrong commented Jan 4, 2021

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
Copy link
Member

@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 #46623. We plan on addressing the closure issue in the near future as part of that.

@adamstyrc
Copy link

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
Copy link
Author

duzenko commented May 20, 2022

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
Copy link

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

@mkustermann
Copy link
Member

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
Copy link
Author

duzenko commented May 24, 2022

non trivial synchronous work

For example?

@mkustermann
Copy link
Member

@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
Copy link
Author

duzenko commented May 25, 2022

@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
Copy link
Member

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.

@duzenko
Copy link
Author

duzenko commented May 26, 2022

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
Copy link
Member

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 ).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. library-isolate type-documentation A request to add or improve documentation type-question A question about expected behavior or functionality
Projects
None yet
Development

No branches or pull requests