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

Make SendPort and ReceivePort generic #53962

Closed
Levi-Lesches opened this issue Oct 26, 2023 · 5 comments
Closed

Make SendPort and ReceivePort generic #53962

Levi-Lesches opened this issue Oct 26, 2023 · 5 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. closed-duplicate Closed in favor of an existing report library-isolate type-enhancement A request for a change that isn't a bug

Comments

@Levi-Lesches
Copy link

Levi-Lesches commented Oct 26, 2023

Currently, SendPort.send accepts any Object?, and ReceivePort implements Stream<dynamic>. I see a nice opportunity to add some type safety here:

class SendPort<T> {
  void send(T object);
}

class ReceivePort<T> implements Stream<T> {
  final SendPort<T> sendPort;
}

// Safer code: 
void main() {
  final numberPort = ReceivePort<num>();
  Isolate.spawn(runIsolate, numberPort.sendPort);
  // implied: Isolate.spawn<SendPort<num>>(...)
}

void runIsolate(SendPort<num> port) {
  port.send(1);  // OK
  port.send(3.14);  // OK
  port.send("Hello");  // Error
}

Or even, with pattern matching:

sealed class IsolateMessage { } 
class RefreshPage extends IsolateMessage { }
class FetchData extends IsolateMessage { }
class YouForgotThis extends IsolateMessage { }


// Error: The type 'IsolateMessage' is not exhaustively matched by the switch cases since it doesn't match 'FetchData()'.
final port = ReceivePort<IsolateMessage>()
  ..listen((message) => switch(message) {
    RefreshPage() => print("Refreshing..."),
    // FetchData() => print("Fetching..."),
  });

/cc @mraleph from the Discord conversation. Also, see #53877 for a related proposal

@abitofevrything
Copy link
Contributor

Duplicate of #23059

@lrhn
Copy link
Member

lrhn commented Oct 26, 2023

Good find. And the issues mentioned on that issue still apply.

A solution could be "TypeReveivePort"/"TypedSendPort" types which cannot be sent to isolated that are not in the same isolate group.
They would likely not implement ReveivePort or SendPort, because then upcasting them would lose that information. It's a new kind of port.

It's very likely easier to just create wrappers yourself, or just do receivePort.cast<MyMessage>().
(If native code gets hold of a send port, they can still send any value, so some receiver side checking is necessary anyway.)

@Levi-Lesches
Copy link
Author

Levi-Lesches commented Oct 27, 2023

Right, I think limiting this to the case of isolates in the same group would still be useful. Although I agree that making our own wrappers is easy, they can't implement the original SendPort or ReceivePort, because then they'll be forced to inherit/define the unsafe methods, so they won't be compatible with any of the existing APIs (although, like you pointed out, ReceivePort.cast<T>() is often enough -- I'm more focused on SendPorts and a typed ReceivePort<T>() creating a SendPort<T>).

@lrhn lrhn transferred this issue from dart-lang/language Nov 6, 2023
@lrhn
Copy link
Member

lrhn commented Nov 6, 2023

Closing as an SDK feature. If someone wants to make a package, or we want to add it to a resurrected package:isolate, then that is an option.

@lrhn lrhn closed this as completed Nov 6, 2023
@lrhn lrhn added area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. closed-duplicate Closed in favor of an existing report library-isolate type-enhancement A request for a change that isn't a bug labels Nov 6, 2023
@Levi-Lesches
Copy link
Author

Since my last comments, I published package:typed_isolate, which uses type-safe ports internally. I'll also update #53877 as well.

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. closed-duplicate Closed in favor of an existing report library-isolate type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

3 participants