Bug 217986 - [GLIB] imported/w3c/web-platform-tests/html/canvas/offscreen/line-styles/2d.line.width.transformed.html is a flaky crash
Summary: [GLIB] imported/w3c/web-platform-tests/html/canvas/offscreen/line-styles/2d.l...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Lord
URL:
Keywords: InRadar
Depends on:
Blocks: 202798
  Show dependency treegraph
 
Reported: 2020-10-20 13:53 PDT by Diego Pino
Modified: 2020-10-27 10:09 PDT (History)
13 users (show)

See Also:


Attachments
Patch (8.05 KB, patch)
2020-10-27 07:11 PDT, Chris Lord
no flags Details | Formatted Diff | Diff
Patch (8.04 KB, patch)
2020-10-27 09:09 PDT, Chris Lord
no flags Details | Formatted Diff | Diff
Patch (9.24 KB, patch)
2020-10-27 09:36 PDT, Chris Lord
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Diego Pino 2020-10-20 13:53:55 PDT
These two tests are following with the same stacktrace:

  imported/w3c/web-platform-tests/html/canvas/offscreen/line-styles/2d.line.width.transformed.html [ Crash ]
  imported/w3c/web-platform-tests/html/canvas/offscreen/path-objects/2d.path.arc.selfintersect.1.worker.html [ Crash ]

First reported crash in test history is r267763. Exploring several revisions before, r267735 might be a likely cause for the failures.

Crash-log: https://build.webkit.org/results/GTK-Linux-64-bit-Release-Tests/r268734%20(16534)/imported/w3c/web-platform-tests/html/canvas/offscreen/line-styles/2d.line.width.transformed-crash-log.txt

Thread 1 (Thread 0x7fbff0b7e9c0 (LWP 99757)):
#0  0x00007fbffb2558f0 in WTF::Detail::CallableWrapper<WebCore::OffscreenCanvas::pushBufferToPlaceholder()::$_6, void>::call() () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#1  0x00007fbff74bed36 in WTF::RunLoop::performWork() () at /app/webkit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#2  0x00007fbff7519cb6 in WTF::RunLoop::RunLoop()::$_1::__invoke(void*) () at /app/webkit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#3  0x00007fbff75191aa in WTF::RunLoop::$_0::__invoke(_GSource*, int (*)(void*), void*) () at /app/webkit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#4  0x00007fbff2f5704f in g_main_dispatch (context=0x1bd3970) at ../glib/gmain.c:3325
#5  g_main_context_dispatch (context=0x1bd3970) at ../glib/gmain.c:4016
#6  0x00007fbff2f573f8 in g_main_context_iterate (context=0x1bd3970, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at ../glib/gmain.c:4092
#7  0x00007fbff2f57713 in g_main_loop_run (loop=0x1c01c10) at ../glib/gmain.c:4290
#8  0x00007fbff751974b in WTF::RunLoop::run() () at /app/webkit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#9  0x00007fbffa2dd87f in int WebKit::AuxiliaryProcessMain<WebKit::WebProcess, WebKit::WebProcessMainGtk>(int, char**) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#10 0x00007fbff1ab6022 in __libc_start_main (main=0x400c20 <main>, argc=4, argv=0x7ffed176fd68, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7ffed176fd58) at ../csu/libc-start.c:308
#11 0x0000000000400b5e in _start () at ../sysdeps/x86_64/start.S:120
Comment 1 Chris Lord 2020-10-21 03:30:24 PDT
Will investigate. I suspect a race on destruction.
Comment 2 Lauro Moura 2020-10-22 18:06:13 PDT
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.
Comment 3 Chris Lord 2020-10-27 03:56:03 PDT
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.
Comment 4 Chris Lord 2020-10-27 04:38:09 PDT
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.
Comment 5 Chris Lord 2020-10-27 07:11:30 PDT
Created attachment 412417 [details]
Patch
Comment 6 Chris Lord 2020-10-27 07:13:56 PDT
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.
Comment 7 Chris Lord 2020-10-27 09:09:33 PDT
Created attachment 412427 [details]
Patch
Comment 8 Chris Lord 2020-10-27 09:33:54 PDT
Comment on attachment 412427 [details]
Patch

Should also re-enable the disabled tests...
Comment 9 Chris Lord 2020-10-27 09:36:01 PDT
Created attachment 412432 [details]
Patch
Comment 10 EWS 2020-10-27 10:08:55 PDT
Committed r269046: <https://trac.webkit.org/changeset/269046>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 412432 [details].
Comment 11 Radar WebKit Bug Importer 2020-10-27 10:09:27 PDT
<rdar://problem/70727106>