Summary: | [GLIB] imported/w3c/web-platform-tests/html/canvas/offscreen/line-styles/2d.line.width.transformed.html is a flaky crash | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Diego Pino <dpino> | ||||||||
Component: | New Bugs | Assignee: | Chris Lord <clord> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | cdumez, changseok, clord, dino, esprehn+autocc, ews-watchlist, gyuyoung.kim, kbr, lmoura, rniwa, sabouhallawa, simon.fraser, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 202798 | ||||||||||
Attachments: |
|
Description
Diego Pino
2020-10-20 13:53:55 PDT
Will investigate. I suspect a race on destruction. The same is happening to imported/w3c/web-platform-tests/html/canvas/offscreen/path-objects/2d.path.arc.selfintersect.1.html Results db: https://results.webkit.org/?suite=layout-tests&test=imported%2Fw3c%2Fweb-platform-tests%2Fhtml%2Fcanvas%2Foffscreen%2Fpath-objects%2F2d.path.arc.selfintersect.1.html Gardened in r268892. As I suspected, this is due to the script execution context being destroyed while the buffer is being pushed to the placeholder. I've got a fix that just involves holding a reference to the context while this is happening, but it feels ugly to me - I'll verify/tidy/upload and hopefully review will confirm whether this is a valid fix or not. So I thought I had a fix, but it really just replaces the slightly more frequent crash with a much rarer, but still possible exception. Still looking into this, will see if I can get any advice on Slack if I can't come up with anything. Created attachment 412417 [details]
Patch
As is often the case, I was overcomplicating things - there's no need to pass the OffscreenCanvas object to the main thread, instead I've moved all the placeholder canvas communication related data into a separate ThreadSafeRefCounted structure which we can safely use and release on the main thread. This removes the need to call back into the Worker thread to release a reference on the OffscreenCanvas and side-steps this issue entirely. Resolves the crashes in my local testing. Created attachment 412427 [details]
Patch
Comment on attachment 412427 [details]
Patch
Should also re-enable the disabled tests...
Created attachment 412432 [details]
Patch
Committed r269046: <https://trac.webkit.org/changeset/269046> All reviewed patches have been landed. Closing bug and clearing flags on attachment 412432 [details]. |