Bug 199516 – Add threading assertion to WTF::CompletionHandler

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Template Framework

[Chris Dumez]

Add threading assertion to WTF::CompletionHandler to make sure it is constructed and called on the same thread.

[Chris Dumez]

Created attachment 373493 [details]
WIP Patch

[Chris Dumez]

Created attachment 373497 [details]
WIP Patch

[EWS Watchlist]

Comment on attachment 373497 [details]
WIP Patch

Attachment 373497 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/12668712

New failing tests:
fast/workers/worker-messageport-gc.html
fast/workers/termination-with-port-messages.html
http/tests/workers/worker-messageport.html
workers/worker-to-worker.html
fast/workers/worker-messageport.html
http/tests/workers/worker-messageport-2.html
workers/message-port-gc.html

[EWS Watchlist]

Created attachment 373506 [details]
Archive of layout-test-results from ews102 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 373497 [details]
WIP Patch

Attachment 373497 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/12668752

New failing tests:
imported/w3c/web-platform-tests/service-workers/service-worker/ServiceWorkerGlobalScope/postmessage.https.html
fast/workers/termination-with-port-messages.html
http/tests/workers/worker-messageport.html
imported/w3c/web-platform-tests/service-workers/service-worker/postmessage-msgport-to-client.https.html
http/tests/workers/service/basic-messageport.html
imported/w3c/web-platform-tests/service-workers/service-worker/postmessage.https.html
workers/worker-to-worker.html
imported/w3c/web-platform-tests/service-workers/service-worker/extendable-event-waituntil.https.html
fast/workers/worker-messageport.html
http/tests/workers/worker-messageport-2.html
imported/w3c/web-platform-tests/service-workers/service-worker/fetch-event-throws-after-respond-with.https.html
workers/message-port-gc.html
fast/workers/worker-messageport-gc.html

[EWS Watchlist]

Created attachment 373509 [details]
Archive of layout-test-results from ews107 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 373497 [details]
WIP Patch

Attachment 373497 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/12668786

New failing tests:
workers/message-port-gc.html
fast/workers/termination-with-port-messages.html
http/tests/workers/worker-messageport.html
workers/worker-to-worker.html
fast/workers/worker-messageport.html
http/tests/workers/worker-messageport-2.html
fast/workers/worker-close-more.html
fast/workers/worker-messageport-gc.html

[EWS Watchlist]

Created attachment 373513 [details]
Archive of layout-test-results from ews112 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 373497 [details]
WIP Patch

Attachment 373497 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/12668808

New failing tests:
imported/w3c/web-platform-tests/service-workers/service-worker/ServiceWorkerGlobalScope/postmessage.https.html
http/tests/workers/worker-messageport.html
imported/w3c/web-platform-tests/service-workers/service-worker/postmessage-msgport-to-client.https.html
http/tests/workers/service/basic-messageport.html
imported/w3c/web-platform-tests/service-workers/service-worker/postmessage.https.html
workers/worker-to-worker.html
imported/w3c/web-platform-tests/service-workers/service-worker/extendable-event-waituntil.https.html
fast/workers/worker-messageport.html
http/tests/workers/worker-messageport-2.html
imported/w3c/web-platform-tests/service-workers/service-worker/fetch-event-throws-after-respond-with.https.html
workers/message-port-gc.html
fast/workers/worker-messageport-gc.html

[EWS Watchlist]

Created attachment 373514 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.14.5

[EWS Watchlist]

Comment on attachment 373497 [details]
WIP Patch

Attachment 373497 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/12669368

New failing tests:
fast/workers/worker-messageport-gc.html
fast/workers/termination-with-port-messages.html
http/tests/workers/worker-messageport.html
workers/worker-to-worker.html
fast/workers/worker-messageport.html
http/tests/workers/worker-messageport-2.html
fast/workers/worker-close-more.html
workers/message-port-gc.html

[EWS Watchlist]

Created attachment 373528 [details]
Archive of layout-test-results from ews211 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews211  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit

[EWS Watchlist]

Comment on attachment 373497 [details]
WIP Patch

Attachment 373497 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/12670092

New failing tests:
fast/workers/worker-messageport-gc.html
fast/workers/termination-with-port-messages.html
http/tests/workers/worker-messageport.html
workers/worker-to-worker.html
fast/workers/worker-messageport.html
fast/workers/worker-close-more.html
workers/message-port-gc.html

[EWS Watchlist]

Created attachment 373530 [details]
Archive of layout-test-results from ews210 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews210  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit

[Chris Dumez]

Created attachment 373536 [details]
WIP Patch

[Chris Dumez]

Created attachment 373549 [details]
Patch

[Michael Catanzaro]

As Fujii discovered, this is a duplicate of bug #186171 and we have existing good discussion there, so I recommend using that bug instead.

FWIW: I think it's very important to add this assertion, but in the other bug you had found a problem....

[Chris Dumez]

(In reply to Michael Catanzaro from comment #17)
> As Fujii discovered, this is a duplicate of bug #186171 and we have existing
> good discussion there, so I recommend using that bug instead.
> 
> FWIW: I think it's very important to add this assertion, but in the other
> bug you had found a problem....

The check in this patch is less strict and is compatible with work queues.

[Chris Dumez]

(In reply to Michael Catanzaro from comment #17)
> As Fujii discovered, this is a duplicate of bug #186171 and we have existing
> good discussion there, so I recommend using that bug instead.
> 
> FWIW: I think it's very important to add this assertion, but in the other
> bug you had found a problem....

We can dupe the other one to this once it lands (less work).

[Alex Christensen]

Comment on attachment 373549 [details]
Patch

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

> Source/WebKit/ChangeLog:11
> +        worker thread. Ideally, this code would be refactored so that the callback gets called on
> +        the worker thread directly.

Should we add a FIXME comment?  This ChangeLog entry will probably never get looked at again.

[Chris Dumez]

Created attachment 373641 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 373641 [details]
Patch

Clearing flags on attachment: 373641

Committed r247218: <https://trac.webkit.org/changeset/247218>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.