Bug 184285

Summary:

MessagePorts created in WebWorker thread stops working

Product:

WebKit

Reporter:

Yann Cabon <ycabon>

Component:

New Bugs

Assignee:

Tadeu Zagallo <tzagallo>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ap, beidson, cdumez, commit-queue, darin, dbates, esprehn+autocc, ews-watchlist, kangil.han, rniwa, tzagallo, webkit-bug-importer, youennf

Priority:

Keywords:

InRadar

Version:

Safari 11

Hardware:

Mac

OS:

macOS 10.13

See Also:

https://bugs.webkit.org/show_bug.cgi?id=184502
https://bugs.webkit.org/show_bug.cgi?id=184254

Attachments:

Description	Flags
Patch	none
Archive of layout-test-results from ews104 for mac-sierra-wk2	none
Archive of layout-test-results from ews122 for ios-simulator-wk2	none
Patch	none
Patch	none

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	Details Formatted Diff Diff
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	Details Formatted Diff Diff
Patch (4.30 KB, patch) 2018-04-17 11:24 PDT, Tadeu Zagallo	no flags	Details Formatted Diff Diff
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.