RESOLVED FIXED 184285
MessagePorts created in WebWorker thread stops working 
https://bugs.webkit.org/show_bug.cgi?id=184285
Summary MessagePorts created in WebWorker thread stops working
Yann Cabon
Reported 2018-04-03 19:19:04 PDT
Communication between MessagePort instances created in a WebWorker thread stops after time. I suspect GC happening. The following test case is in 2 parts. An html page and a worker script. The worker creates a message channel and transfer one of the ports back to the main thread. The ports start communicating then after some 18 exchanges, the communication stops. I finally figured that if I kept the reference of the MessageChannel that create the ports around the problem disappeared. I looks like this shouldn't happen. The test cases creates some arraybuffers and discard them to trigger some GC. Test case: //--------------------------------------------------------------------------- // messagechannel_garbagecollected.html //--------------------------------------------------------------------------- <html> <body> <script> var worker = new Worker("./messagechannel_garbagecollected_worker.js"); var arrays = []; worker.addEventListener("message", (event) => { var port = event.data.port; port.addEventListener("message", (event) => { document.body.innerHTML += event.data + "<br />"; if (arrays.length === 10) { arrays = []; } arrays.push(new ArrayBuffer(500000)); setTimeout(function() { port.postMessage("ping"); }, 50); }); port.start(); port.postMessage("ping"); }); </script> </body> </html> //--------------------------------------------------------------------------- // messagechannel_garbagecollected_worker.js //--------------------------------------------------------------------------- var arrays = []; function startup() { var channel = new MessageChannel(); var port = channel.port1; var id = 0; port.addEventListener("message", (event) => { if (arrays.length === 10) { arrays = []; } arrays.push(new ArrayBuffer(50000)); setTimeout(function() { port.postMessage("pong " + id++); }, 50); }); port.start(); // keeping a dummy reference to the channel keeps it working // port["channel"] = channel; self.postMessage({ port: channel.port2 }, [channel.port2]); } startup();
Attachments
Patch (4.20 KB, patch)
2018-04-13 18:07 PDT, Tadeu Zagallo 		no flags
DetailsFormatted DiffDiff
Archive of layout-test-results from ews104 for mac-sierra-wk2 (2.64 MB, application/zip)
2018-04-13 19:17 PDT, EWS Watchlist 		no flags
Details
Archive of layout-test-results from ews122 for ios-simulator-wk2 (2.24 MB, application/zip)
2018-04-13 19:40 PDT, EWS Watchlist 		no flags
Details
Patch (4.23 KB, patch)
2018-04-16 13:18 PDT, Tadeu Zagallo 		no flags
DetailsFormatted DiffDiff
Patch (4.30 KB, patch)
2018-04-17 11:24 PDT, Tadeu Zagallo 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2018-04-06 21:02:41 PDT
<rdar://problem/39256714>
Tadeu Zagallo
Comment 2 2018-04-13 18:07:50 PDT
Created attachment 337935 [details] Patch
EWS Watchlist
Comment 3 2018-04-13 19:17:20 PDT
Comment on attachment 337935 [details] Patch Attachment 337935 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/7311332 New failing tests: fast/events/message-channel-gc-4.html
EWS Watchlist
Comment 4 2018-04-13 19:17:22 PDT
Created attachment 337945 [details] Archive of layout-test-results from ews104 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
EWS Watchlist
Comment 5 2018-04-13 19:40:08 PDT
Comment on attachment 337935 [details] Patch Attachment 337935 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/7311418 New failing tests: fast/events/message-channel-gc-4.html
EWS Watchlist
Comment 6 2018-04-13 19:40:09 PDT
Created attachment 337948 [details] Archive of layout-test-results from ews122 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews122 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.4
Tadeu Zagallo
Comment 7 2018-04-16 13:18:24 PDT
Created attachment 338028 [details] Patch
Darin Adler
Comment 8 2018-04-16 18:03:26 PDT
Comment on attachment 338028 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=338028&action=review > Source/WebCore/dom/MessagePort.cpp:70 > - allMessagePorts().remove(m_identifier); > + if (allMessagePorts().get(m_identifier) == this) > + allMessagePorts().remove(m_identifier); The correct way to implement this without double hashing is: auto iterator = allMessagePorts().find(m_identifier); if (iterator != allMessagePorts().end() && iterator->value == this) allMessagePorts().remove(iterator); Messier than the above, but avoids a second hash table lookup.
Tadeu Zagallo
Comment 9 2018-04-17 11:24:44 PDT
Created attachment 338133 [details] Patch
WebKit Commit Bot
Comment 10 2018-04-17 15:57:34 PDT
Comment on attachment 338133 [details] Patch Clearing flags on attachment: 338133 Committed r230735: <https://trac.webkit.org/changeset/230735>
WebKit Commit Bot
Comment 11 2018-04-17 15:57:36 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.