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

Illegal argument in isolate message that works or break without changing much of the code #59866

Open
stephane-archer opened this issue Jan 8, 2025 · 8 comments
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@stephane-archer
Copy link

this code works:

import 'package:image/image.dart' as img;

@immutable
class _AdjustColorIsolatemessage {
  final img.Image inputImage;
  final num brightness;
  const _AdjustColorIsolatemessage(this.inputImage, this.brightness);
}

void main () {
    img.Image editedImage =
        await compute<_AdjustColorIsolatemessage, img.Image>(
      (message) {
        return img.adjustColor(
            // convert to 8-bit because adjustColor doesn't support 16-bit images
            message.inputImage.convert(format: img.Format.uint8),
            brightness: message.brightness);
      },
      _AdjustColorIsolatemessage(resizeImage, brightness),
    );
}

but moving the function executed by compute to it's own function result in the following error:

ArgumentError (Invalid argument(s): Illegal argument in isolate message: object is unsendable - Library:'dart:async' Class: _Future@5048458 (see restrictions listed at `SendPort.send()` documentation for more information)
 <- Instance of 'AsyncNotifierProviderElement<_RefImageEditedBeforeLutPathNotifier, String?>' (from package:riverpod/src/async_notifier.dart)
 <- Instance of '_RefImageEditedBeforeLutPathNotifier' (from package:lutme/ref_image_edited_before_lut_path_provider.dart)
 <- Closure: (_AdjustColorIsolatemessage) => Image from Function 'foo':. (from package:lutme/ref_image_edited_before_lut_path_provider.dart)
 <- field callback in compute.<anonymous closure> (from package:flutter/src/foundation/_isolates_io.dart)
 <- resultPort in Instance of '_RemoteRunner<Image>' (from dart:isolate)
)

Here is the new code:

import 'package:image/image.dart' as img;

@immutable
class _AdjustColorIsolatemessage {
  final img.Image inputImage;
  final num brightness;
  const _AdjustColorIsolatemessage(this.inputImage, this.brightness);
}

void main () {
    img.Image editedImage =
        await compute<_AdjustColorIsolatemessage, img.Image>(
       foo,
      _AdjustColorIsolatemessage(resizeImage, brightness),
    );
}

  img.Image foo(_AdjustColorIsolatemessage message) {
    return img.adjustColor(
        // convert to 8-bit because adjustColor doesn't support 16-bit images
        message.inputImage.convert(format: img.Format.uint8),
        brightness: message.brightness);
  }
@dart-github-bot
Copy link
Collaborator

Summary: Using compute with an anonymous function works, but referencing a named function causes an "Illegal argument in isolate message" error due to an unsendable object in the closure's context. The error points to a Riverpod provider.

@dart-github-bot dart-github-bot added area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Jan 8, 2025
@stephane-archer
Copy link
Author

for clarification, this code is not in the main in my app but in a Riverpod provider. this is why you see it referenced

@stephane-archer
Copy link
Author

stephane-archer commented Jan 8, 2025

making foo static work, I think the error message should be clearer that what is not serializable is the function call. that foo needs to be static, I imagine static is not part of the function type signature so this can only be detected at runtime.

@lrhn
Copy link
Member

lrhn commented Jan 8, 2025

It's the foo method taken out of context here? A top-level function should serialize perfectly, it's a compile-time constant.

Using an instance meeting will include the this reference and all its state in the serialization (unless the compiler can tree-shake the this reference. Not something I'd rent on.
That object state seems to include a Future here.

So this looks like it's working as intended. You have to be very careful about functions that are sent to other isolates.

@stephane-archer
Copy link
Author

stephane-archer commented Jan 9, 2025

@lrhn foo was a member function without reference to this. adding the static keyword or making it a top-level function works.

I didn't expect that referencing a member function from another member function that uses compute would bring the entire object into the isolate especially because this wasn't referenced.

I would have expected the error message to be "foo is not a top-level function" rather than this:

ArgumentError (Invalid argument(s): Illegal argument in isolate message: object is unsendable - Library:'dart:async' Class: _Future@5048458 (see restrictions listed at `SendPort.send()` documentation for more information)
 <- Instance of 'AsyncNotifierProviderElement<_RefImageEditedBeforeLutPathNotifier, String?>' (from package:riverpod/src/async_notifier.dart)
 <- Instance of '_RefImageEditedBeforeLutPathNotifier' (from package:lutme/ref_image_edited_before_lut_path_provider.dart)
 <- Closure: (_AdjustColorIsolatemessage) => Image from Function 'foo':. (from package:lutme/ref_image_edited_before_lut_path_provider.dart)
 <- field callback in compute.<anonymous closure> (from package:flutter/src/foundation/_isolates_io.dart)
 <- resultPort in Instance of '_RemoteRunner<Image>' (from dart:isolate)
)

now I know it means "We bring the word attached to this function and the world is not serializable" but I find it really hard to understand without context

@stephane-archer
Copy link
Author

You have to be very careful about functions that are sent to other isolates.

Does it make sense to restrict them to static and top-level functions, at least at the "linter level"?

@lrhn
Copy link
Member

lrhn commented Jan 9, 2025

Does it make sense to restrict them to static and top-level functions, at least at the "linter level"?

That's where it started. The VM made an effort to allow passing closures too because it's useful. You can't generally tell where a closure (a function value) comes from, unless you can see the expression that creates it. Otherwise all you know is it's function type.
The VM does have some issues with over-capturing, but I think capturing this in instance method tear-offs is something you should expect, after all their equality depends on their this reference. (I guess they could capture only some unique object ID rather than the entire this reference, if they don't need the actual object.)

Maybe it would be useful to warn if the closure passed to an isolate communicating function (Isolate.spawn, Isolate.run, SendPort.send and probably Flutter's compute) is an instance method tear-off that doesn't us its reference to this.
It probably won't be possible to recognize such a closure being stuffed into an object before being sent, not unless it's very obvious, like a collection or record literal as the isolate function argument, and the warning would basically say "this function could be static".

That could be an analyzer warning, or it could be something the VM does itself. It should probably be the analyzer, because it supports ignoring warnings. It would only trigger for arguments to a few recognized functions, which are only available to native code, so the cost of checking should be fairly limited.

@stephane-archer
Copy link
Author

@lrhn analyzer warning/linter would be great when using compute "this function could be static" or "this function is not static, are you sure you want to transfer the whole object to the isolate". I imagine it has some performance consequences.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

3 participants