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

MessagePort not transferring via WebWorker postMessage(). #35730

Closed
rayk opened this issue Jan 23, 2019 · 9 comments

Comments

Projects
None yet
3 participants
@rayk
Copy link

commented Jan 23, 2019

Environment:

  • Dart VM version: 2.1.0 (Tue Nov 13 18:22:02 2018 +0100) on "macos_x64"
  • Version 73.0.3673.0 (Official Build) dev (64-bit)

Issue Or Not
Not black & white, as there is no documentation and a reading of the source also does not shed any light. Never the less, I have raised the issue on StackOverflow and have stalked a few folks on gitter. Unfortunately no luck, I can not accept that I am the first people to attempt to do this, hence it could just be a knowledge gap.

Area

  • dart:html
  • Worker, in particular postMessage(Object message, [ List<Object> transfer]) -> void
  • MessageChannel/MessagePort

Assumptions

  1. MessagePort is a cloneable transferable object. MDN
  2. MessagePort is the way to go when needing to communicating between two workers, independent of the thread which launched each of the workers.

Core Problem
After passing the messagePort to the work it arrives in as an instance of 'JsLinkedHashMap<dynamic, dynamic>' and there is no way to retrieve the MessagePort instance.

Sounds stupid right!

Reproduction
The simplest reproduction I have managed is the following.

  1. Inside a main.dart:
import 'dart:html';

import 'dart:collection';

void main() {
  querySelector('#output').text = 'Your Dart app is running.';

  Worker w = Worker('lib/my_worker.dart.js');

  MessageChannel msgChn = MessageChannel();

  HashMap trsMsg = HashMap.from({'port': msgChn.port1});
  w.postMessage(trsMsg, [msgChn.port1]);
}
  1. In our worker code.
import 'dart:html';

class MyWorker {
  DedicatedWorkerGlobalScope dws = DedicatedWorkerGlobalScope.instance;

  MyWorker() {
    print('Worker Running');

    dws.onMessage.listen((MessageEvent evt) {
      print(evt.data);
      print(evt.data.runtimeType);
      var strSec = evt.data['_strings'];
      print(strSec);
      MessagePort mp = strSec['port'];
    });
  }
}

main() {
  MyWorker();
}
  1. In the build.yaml.
targets:
  $default:
    builders:
      build_web_compilers|entrypoint:
        options:
          compiler: dart2js
          dart2js_args:
            - -O1

The result:

screen shot 2019-01-23 at 12 34 01 pm

I have tried a number of different approaches to this whole problem, including but not limited:

  1. Using a LinkedHashMap for the trsMsg.
  2. Building the trsMsg Hash from Iterables, that was a longshot.
  3. Sending port 2 instead of port 1,
  4. Inlining anything and everything I could.
  5. Cast and recast on the worker side.
  6. Changing every build options I could think of.
  7. Using the Js library to create an anonymous MessageEvent
  8. Not using the dart:html library at all. Instead using @JS() for everything from the worker to the port.

Clearly running out of ideas and getting down to long shots.

Expected Behaviour

  • Instead of getting an unusable [object MessagePort] showing up in the worker, simply being able to access an Instance of MessagePort from within the MessageEvent.data would be ideal. For that matter being to access the transferred MessagePort in everywhere in the worker would do the job.

Thanks in advance and I hope it is something thing simple I am doing or not; communicating between web workers without messagePort is a nightmare.

@rayk rayk changed the title Transfering a MessagePort via WebWorker postMessage() does not preserve messagePort instance. MessagePort not transferring via WebWorker postMessage(). Jan 23, 2019

@sigmundch

This comment has been minimized.

Copy link
Member

commented Jan 23, 2019

@rayk thanks for filing this issue

I took a closer look and it appears there are several issues at play.

  • Not sure why, but the data cloning seems broken at the moment.

    Technically before sending a message we should be doing a structural clone that converts our Dart types to the clonable JS types.

    The fact that you see internals of the map implementation (like _strings) is pretty odd.

    I tried switching to send an array instead, and I also run into issues, although I got some things working by accident. In particular, type information we may attach to object instances seems to be interfering with the cloning and validation that happens when calling postMessage. If I use the dart2js flag --omit-implicit-checks a lot of this type information is removed, which made my example below work.

  • the worker is missing the implementation of MessagePort. This is why the cast fails. We are checking that something is an instance of MessagePort, but we don't have the type because we didn't see a creation for that type. Adding code like new MessageChannel() somewhere in the worker program is a possible workaround.

I'm not recommending you do this, but the program below was able to exchange a message back and forth. I'm just posting it here to help investigate our next steps:

main program:

import 'dart:html';

void main() {
  Worker w = Worker('worker.dart.js');
  MessageChannel msgChn = MessageChannel();
  w.postMessage([msgChn.port1], [msgChn.port1]);
  msgChn.port2.onMessage.listen((m) => print("parent got: " + m.data));
}

the worker:

import 'dart:html';

class MyWorker {
  DedicatedWorkerGlobalScope dws = DedicatedWorkerGlobalScope.instance;

  MyWorker() {
    print('Worker Running');

    dws.onMessage.listen((MessageEvent evt) {
      print(evt.data);
      print(evt.data.runtimeType);
      print(new MessageChannel().port1);
      MessagePort port = evt.data[0];
      print(port);
      port.postMessage("hola hola");
    });
  }
}

main() {
  MyWorker();
}

/cc @rakudrama

@sigmundch

This comment has been minimized.

Copy link
Member

commented Jan 23, 2019

A few more details:

  • the worker.postMessage API directly invokes the DOM API instead of first doing a structural clone (the cloning call is missing entirely, see how we do have that clone call in MessagePort.postMesage)
  • if called manually, cloning fails because it doesn't have a rule to bypass MessagePort instances

@terrylucas - Both of issues above appear to be bugs in the dart:html generated library

  • finally, to have the type available in the worker, it is possible that all we need to do is to update what values are expected on deserialization (_serializedScriptValue in our dart:html_common library)
@sigmundch

This comment has been minimized.

Copy link
Member

commented Jan 23, 2019

Quick update: I can verify that the 3 issues above address the original bug:

  • need to update Worker.postMessage to ensure we do a structural clone (adding the clone manually fixes the issue too)
  • need to update serialization code to include MessagePort
  • need to list MessagePort as a possible deserialization type (this fixes the worker issue)

I'll prepare a fix tomorrow for the 2nd and 3rd issue.
@terrylucas - could you help us fix the first issue on your end?

@rayk

This comment has been minimized.

Copy link
Author

commented Jan 23, 2019

Thanks @sigmundch and @terrylucas, the --omit-implicit-checks really is the big clue here I totally missed. Keep me updated and I shall make sure I seed some article into the community showing off port passing using dart:html_common library.

@leonsenft

This comment has been minimized.

Copy link

commented Jan 24, 2019

Related issue #32688. MessageEvent from dart:html is missing the ports property.

dart-bot pushed a commit that referenced this issue Jan 25, 2019

Include MessagePort in convertions for workers
Address 2 of the 3 issues mentioned in #35730

Change-Id: I4ad67eddf266cdfb8847a277a6d19243dce23184
Reviewed-on: https://dart-review.googlesource.com/c/91106
Reviewed-by: Stephen Adams <sra@google.com>
Commit-Queue: Sigmund Cherem <sigmund@google.com>
@sigmundch

This comment has been minimized.

Copy link
Member

commented Jan 25, 2019

I just landed 60d7e8f and 6bcb017 and things are working much better now.

I can compile these 2 programs with no flags (no --omit-implicit-checks needed), and run them correctly:

main.dart:

import 'dart:html';

void main() {
  Worker w = Worker('worker.dart.js');
  MessageChannel msgChn = MessageChannel();
  w.postMessage({'port': msgChn.port1}, [msgChn.port1]);
  msgChn.port2.onMessage.listen((m) => print("received message from worker: " + m.data));
}

worker.dart:

import 'dart:html';

class MyWorker {
  DedicatedWorkerGlobalScope dws = DedicatedWorkerGlobalScope.instance;

  MyWorker() {
    print('Worker Running');

    dws.onMessage.listen((MessageEvent evt) {
      print(evt.data);
      print(evt.data.runtimeType);
      MessagePort port = evt.data["port"];
      print(port);
      port.postMessage("port is working!");
    });
  }
}

main() {
  MyWorker();
}

The output I get is:

Worker Running
{port: Instance of 'MessagePort'}
JsLinkedHashMap<dynamic, dynamic>
Instance of 'MessagePort'
received message from worker: port is working!

The fixes are on the master branch at this time, and will be available in the next dev release.

@sigmundch sigmundch closed this Jan 25, 2019

@leonsenft

This comment has been minimized.

Copy link

commented Jan 25, 2019

Thanks for this fix @sigmundch. Is there any chance we could have the transferred MessagePort exposed on the MessageEvent as well? #32688

It's passed in, and presumably transferred correctly, but the ports property is missing from MessageEvent in dart:html.

@sigmundch

This comment has been minimized.

Copy link
Member

commented Jan 25, 2019

Just spoke with @terrylucas - he will be adding it shortly

@leonsenft

This comment has been minimized.

Copy link

commented Jan 25, 2019

Awesome thanks! 👍

dart-bot pushed a commit that referenced this issue Jan 27, 2019

Expose data member 'port' for MessageEvents and automatically pull in…
… markupsafe for go.sh script.

Fixes #35730

R=sigmund@google.com

Change-Id: I91d9622601c99bbbfaad87c24f01db2b26303744
Reviewed-on: https://dart-review.googlesource.com/c/91165
Commit-Queue: Terry Lucas <terry@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.