Bug 184502 – Safari 11.1: MessageChannel no longer works between Workers

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Ashley Gullen]

This is a critical issue that causes our PWA Construct 3 (editor.construct.net) to no longer work in Safari 11.1 on either iOS 11.3 or macOS 10.13.4.

Repro URL: https://www.scirra.com/labs/bugs/safarimessagechannel/
Steps to reproduce: open page, open JavaScript console, and look at log messages

Expected result: last log message should read: "[Worker2] Received forwarded message:  Hello world"
Observed result: last log message reads: "[Worker1] Received message to forward, posting to Worker 2:  Hello world"

This URL creates two Web Workers, creates a MessageChannel, and transfers port1 and port2 to the first and second web workers. This is a typical way to allow Web Workers to directly communicate without going through the main document, and is a workaround to the fact nested workers aren't widely supported. For example now if Worker 1 posts data to its message port, it will be received by port2 in Worker 2.

In Safari 11.1, a message posted down the MessageChannel is never received at the other end. This causes this setup to fail, since no messages are received by the other worker. In our PWA, this causes the UI to permanently hang, since we use this arrangement to dispatch several kinds of processing jobs to web workers. It works correctly in Chrome 65, Firefox 59 and Edge 16.

The log message "[Worker2] Received forwarded message:  Hello world" indicates that Worker 2 received a message sent down the MessageChannel from Worker 1. This log message is missing in Safari 11.1. The final log message ("[Worker1] Received message to forward, posting to Worker 2:  Hello world") indicates the message was posted, but it is never received.

[Ashley Gullen]

I'd add it appeared to work in Safari 11.0 so this appears to be a regression.

[Radar WebKit Bug Importer]

<rdar://problem/39372771>

[Chris Dumez]

Potential dupe of Bug 184254?

[Tadeu Zagallo]

(In reply to Chris Dumez from comment #3)
> Potential dupe of Bug 184254?

I reduced the test case and I don't believe it's a dupe. This issue only appears when sending messages from workers to workers, and is not affected by the kind of data being sent. I will investigate more.

[Tadeu Zagallo]

Created attachment 337908 [details]
Patch

[Tadeu Zagallo]

The issue was that when transferring a MessagePort, we only retained the MessagePortChannel for the duration of the transfer if it was being sent via another MessagePort. When sending both ports via workers, there was no reference to the MessagePortChannel left, so it got deallocated. By the time the message was received, the reference from the port back to the channel had been cleared, so the message was dropped. I added a minimal test case, and also checked the example provided, both work. I'm not sure if entangling/disentangling is the best place to retain the channel though, but it seemed like the most central place, where it would require the least changes.

[Tadeu Zagallo]

Created attachment 337938 [details]
Patch

[Geoffrey Garen]

Comment on attachment 337938 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 337938 [details]
Patch

Clearing flags on attachment: 337938

Committed r230738: <https://trac.webkit.org/changeset/230738>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Ryan Haddad]

The LayoutTest added with this change is a flaky failure due to messages being received/printed out of order:

--- /Volumes/Data/slave/sierra-release-tests-wk1/build/layout-test-results/workers/worker-to-worker-expected.txt
+++ /Volumes/Data/slave/sierra-release-tests-wk1/build/layout-test-results/workers/worker-to-worker-actual.txt
@@ -6,6 +6,6 @@
 PASS successfullyParsed is true
 
 TEST COMPLETE
+[Worker 2] Received message: hello
 [Worker 1] Received message: hello
-[Worker 2] Received message: hello
 

https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=workers%2Fworker-to-worker.html

[Geoffrey Garen]

Comment on attachment 337938 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=337938&action=review

> LayoutTests/workers/worker-to-worker.html:23
> +  worker("Worker 1", mc.port1, 1),

Looks like the problem here is that 1ms is not a deterministic or reliable guarantee of ordering.

If you change the worker function to pass e.data to its resolver, then you can process log the e.data results in deterministic order at the very end, after all promises have resolved.

[Alexey Proskuryakov]

Tadeu has a fix in bug 184769.

[Huess Yil]

The Status says "Resolved Fixed" but i'm still facing the same issue with current versions (macOS 10.13.6/Safari 11.1.2 & iOS 11.4). Posting messages from worker to worker using messagechannel ports won't work at all.

You can still use Ashley's Repro URL to reproduce this in the mentioned versions.

[Ashley Gullen]

Apple are typically opaque about process and slow to get fixes to release. However as far as I can tell by myself, it should be fixed in the latest Safari Technology Preview, and in Safari 12 when that's released. Yes, this feature has been broken for the entire time Safari 11.1 has been out. They did the same thing when they broke WebAssembly...

[Huess Yil]

Thanks for the info Ashley! I tested it with Safari Tech Preview now and can confirm that it's working.. Unfortunately i will need to build a workaround if this fix only ships with 12 since we found out that most of our macOS/IOs users don't update their devices/systems frequently.