Bug 184502 - Safari 11.1: MessageChannel no longer works between Workers
Summary: Safari 11.1: MessageChannel no longer works between Workers
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: Safari 11
Hardware: Unspecified Unspecified
: P2 Critical
Assignee: Tadeu Zagallo
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-04-11 08:27 PDT by Ashley Gullen
Modified: 2018-08-30 03:54 PDT (History)
14 users (show)

See Also:


Attachments
Patch (7.45 KB, patch)
2018-04-13 10:59 PDT, Tadeu Zagallo
no flags Details | Formatted Diff | Diff
Patch (7.43 KB, patch)
2018-04-13 18:24 PDT, Tadeu Zagallo
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ashley Gullen 2018-04-11 08:27:12 PDT
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.
Comment 1 Ashley Gullen 2018-04-11 08:29:17 PDT
I'd add it appeared to work in Safari 11.0 so this appears to be a regression.
Comment 2 Radar WebKit Bug Importer 2018-04-11 23:24:31 PDT
<rdar://problem/39372771>
Comment 3 Chris Dumez 2018-04-12 09:01:00 PDT
Potential dupe of Bug 184254?
Comment 4 Tadeu Zagallo 2018-04-12 19:25:46 PDT
(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.
Comment 5 Tadeu Zagallo 2018-04-13 10:59:06 PDT
Created attachment 337908 [details]
Patch
Comment 6 Tadeu Zagallo 2018-04-13 11:05:03 PDT
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.
Comment 7 Tadeu Zagallo 2018-04-13 18:24:14 PDT
Created attachment 337938 [details]
Patch
Comment 8 Geoffrey Garen 2018-04-17 16:12:16 PDT
Comment on attachment 337938 [details]
Patch

r=me
Comment 9 WebKit Commit Bot 2018-04-17 16:38:51 PDT
Comment on attachment 337938 [details]
Patch

Clearing flags on attachment: 337938

Committed r230738: <https://trac.webkit.org/changeset/230738>
Comment 10 WebKit Commit Bot 2018-04-17 16:38:52 PDT
All reviewed patches have been landed.  Closing bug.
Comment 11 Ryan Haddad 2018-04-18 16:39:38 PDT
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
Comment 12 Geoffrey Garen 2018-04-19 10:05:23 PDT
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.
Comment 13 Alexey Proskuryakov 2018-04-19 10:18:23 PDT
Tadeu has a fix in bug 184769.
Comment 14 Huess Yil 2018-08-30 02:52:04 PDT
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.
Comment 15 Ashley Gullen 2018-08-30 03:36:05 PDT
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...
Comment 16 Huess Yil 2018-08-30 03:54:44 PDT
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.