223069 2021-03-11 07:25:24 -0800 REGRESSION(r271560): [Linux] release assert in Thread::initializePlatformThreading 2021-03-25 05:12:28 -0700 1 1 1 Unclassified WebKit WebKitGTK WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=220641 P2 Normal --- 1 seb128 berto benjamin berto bugs-noreply cdumez cgarcia cmarcelo ews-watchlist gjp mark.lam mcatanzaro webkit-bug-importer ysuzuki oldest_to_newest 1738441 0 seb128 2021-03-11 07:25:24 -0800 The webkit2gtk Ubuntu hirsute update (2.30.5 to 2.31.90) is blocked because the ruby-gnome tests started failing Steps to trigger - install Ubuntu hirsute - enable sources and proposed repositories - $ sudo apt install quilt xvfb xauth pkg-config libgtk-3-dev dbus-x11 gstreamer1.0-plugins-good gnome-icon-theme libxml2-utils ruby-webkit2-gtk - $ apt source ruby-gnome; cd ruby-gnome-3.4.3 - $ xvfb-run ruby webkit2-gtk/test/run-test.rb ... Loaded suite test Started Aborted (core dumped) Gdb backtrace after rebuildwing webkit without optimization #0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:49 set = {__val = {0, 140737488338759, 140737488338480, 140736484799089, 139642271679776, 140737488338688, 140737488338512, 140736484799131, 140737352090288, 67108868, 140737348587584, 0, 140736495473024, 140737348252777, 140737348252848, 140736495473024}} pid = <optimized out> tid = <optimized out> #1 0x00007ffff7a9a864 in __GI_abort () at abort.c:79 save_stage = 1 act = {__sigaction_handler = {sa_handler = 0x7ffff7e0c2b0, sa_sigaction = 0x7ffff7e0c2b0}, sa_mask = {__val = {0, 140736495473024, 140737348252777, 140737348252848, 140736495473024, 140737488338624, 140737488339296, 140737488338688, 3623148696358275072, 140736484799134, 140737488338640, 140737488339296, 140737349401473, 140737232721296, 128, 140737488338816}}, sa_flags = 67108868, sa_restorer = 0x7ffff7ab5040 <__restore_rt>} sigs = {__val = {32, 0, 140737488338784, 140736435747119, 140737488338864, 17177430016, 140737488338960, 12884967423, 140737488338896, 140736435765782, 1099511627778, 140736495472776, 8029613824315247090, 140733461823492, 12884901891, 3}} #2 0x00007fffc1427942 in CRASH_WITH_INFO(...) () at DerivedSources/ForwardingHeaders/wtf/Assertions.h:713 #3 0x00007fffc43a8af2 in WTF::Thread::initializePlatformThreading() () at ../Source/WTF/wtf/posix/ThreadingPOSIX.cpp:217 #4 0x00007fffc42ffdc8 in operator()() () at ../Source/WTF/wtf/Threading.cpp:379 #5 0x00007fffc430017c in __invoke_impl<void, WTF::initialize()::<lambda()> >(void) () at /usr/include/c++/10/bits/invoke.h:60 #6 0x00007fffc4300128 in __invoke<WTF::initialize()::<lambda()> >(void) () at /usr/include/c++/10/bits/invoke.h:95 #7 0x00007fffc42fffb5 in operator()() () at /usr/include/c++/10/mutex:717 #8 0x00007fffc42fffdf in operator()() () at /usr/include/c++/10/mutex:722 #9 0x00007fffc42ffff4 in _FUN() () at /usr/include/c++/10/mutex:722 #10 0x00007ffff7a6345f in __pthread_once_slow (once_control=0x7fffc4d1d240 <WTF::initialize()::onceKey>, init_routine=0x7ffff0c35590 <__once_proxy>) at pthread_once.c:116 _buffer = {__routine = 0x7ffff7a634b0 <clear_once_control>, __arg = 0x7fffc4d1d240 <WTF::initialize()::onceKey>, __canceltype = 44121, __prev = 0x7fffffffc310} val = <optimized out> newval = <optimized out> #11 0x00007fffc42ff016 in __gthread_once() () at /usr/include/x86_64-linux-gnu/c++/10/bits/gthr-default.h:700 #12 0x00007fffc430008d in call_once<WTF::initialize()::<lambda()> >(void) () at /usr/include/c++/10/mutex:729 #13 0x00007fffc42ffe41 in WTF::initialize() () at ../Source/WTF/wtf/Threading.cpp:372 #14 0x00007fffc3a2feaa in operator()() () at ../Source/JavaScriptCore/runtime/InitializeThreading.cpp:58 #15 0x00007fffc3a354d8 in __invoke_impl<void, JSC::initialize()::<lambda()> >(void) () at /usr/include/c++/10/bits/invoke.h:60 #16 0x00007fffc3a351c3 in __invoke<JSC::initialize()::<lambda()> >(void) () at /usr/include/c++/10/bits/invoke.h:95 #17 0x00007fffc3a34d6f in operator()() () at /usr/include/c++/10/mutex:717 #18 0x00007fffc3a34d99 in operator()() () at /usr/include/c++/10/mutex:722 #19 0x00007fffc3a34dae in _FUN() () at /usr/include/c++/10/mutex:722 #20 0x00007ffff7a6345f in __pthread_once_slow (once_control=0x7fffc4d1ca64 <JSC::initialize()::onceFlag>, init_routine=0x7ffff0c35590 <__once_proxy>) at pthread_once.c:116 _buffer = {__routine = 0x7ffff7a634b0 <clear_once_control>, __arg = 0x7fffc4d1ca64 <JSC::initialize()::onceFlag>, __canceltype = 1435833040, __prev = 0x7fffffffc4e0} val = <optimized out> newval = <optimized out> #21 0x00007fffc3a2ddc4 in __gthread_once() () at /usr/include/x86_64-linux-gnu/c++/10/bits/gthr-default.h:700 #22 0x00007fffc3a34e47 in call_once<JSC::initialize()::<lambda()> >(void) () at /usr/include/c++/10/mutex:729 #23 0x00007fffc3a30056 in JSC::initialize() () at ../Source/JavaScriptCore/runtime/InitializeThreading.cpp:57 #24 0x00007fffc5d4e383 in WebKit::InitializeWebKit2() () at ../Source/WebKit/Shared/WebKit2Initialize.cpp:43 #25 0x00007fffc606b00b in operator()() () at ../Source/WebKit/UIProcess/API/glib/WebKitInitialize.cpp:65 #26 0x00007fffc606b1fe in __invoke_impl<void, WebKit::webkitInitialize()::<lambda()> >(void) () at /usr/include/c++/10/bits/invoke.h:60 #27 0x00007fffc606b1cd in __invoke<WebKit::webkitInitialize()::<lambda()> >(void) () at /usr/include/c++/10/bits/invoke.h:95 #28 0x00007fffc606b099 in operator()() () at /usr/include/c++/10/mutex:717 #29 0x00007fffc606b0c3 in operator()() () at /usr/include/c++/10/mutex:722 #30 0x00007fffc606b0d8 in _FUN() () at /usr/include/c++/10/mutex:722 #31 0x00007ffff7a6345f in __pthread_once_slow (once_control=0x7fffcbfe44f8 <WebKit::webkitInitialize()::onceFlag>, init_routine=0x7ffff0c35590 <__once_proxy>) at pthread_once.c:116 _buffer = {__routine = 0x7ffff7a634b0 <clear_once_control>, __arg = 0x7fffcbfe44f8 <WebKit::webkitInitialize()::onceFlag>, __canceltype = 0, __prev = 0x0} val = <optimized out> newval = <optimized out> #32 0x00007fffc606aecc in __gthread_once() () at /usr/include/x86_64-linux-gnu/c++/10/bits/gthr-default.h:700 #33 0x00007fffc606b171 in call_once<WebKit::webkitInitialize()::<lambda()> >(void) () at /usr/include/c++/10/mutex:729 #34 0x00007fffc606b05f in WebKit::webkitInitialize() () at ../Source/WebKit/UIProcess/API/glib/WebKitInitialize.cpp:64 #35 0x00007fffc606d9eb in webkit_input_method_context_class_init() () at ../Source/WebKit/UIProcess/API/glib/WebKitInputMethodContext.cpp:207 #36 0x00007fffc606d682 in webkit_input_method_context_class_intern_init() () at ../Source/WebKit/UIProcess/API/glib/WebKitInputMethodContext.cpp:137 #37 0x00007ffff39dba82 in g_type_class_ref () at /lib/x86_64-linux-gnu/libgobject-2.0.so.0 #38 0x00007ffff3b85058 in () at /usr/lib/x86_64-linux-gnu/ruby/vendor_ruby/2.7.0/glib2.so #39 0x00007ffff7d0450f in rb_ensure () at /lib/x86_64-linux-gnu/libruby-2.7.so.2.7 Downgrading the webkitgtk binaries to the hirsute 2.30.5 fixes the issue so it seems to be an issue with webkitgtk itself and not other distribution components 1740114 1 berto 2021-03-16 12:25:26 -0700 Ok, so here's what happens: JSC uses SIGUSR1 to suspend and resume threads. After r271560 (see bug 220641) this signal is configurable because some users want to use SIGUSR1 for other purposes, so the patch added the JSC_SIGNAL_FOR_GC environment variable. So far so good. The problem is that the new code in WebKit now specifically checks that no one else is handling that signal already: https://github.com/WebKit/WebKit/blob/a807a4f6d013ac51005bb5c3153bc670fee47065/Source/WTF/wtf/posix/ThreadingPOSIX.cpp#L211 The RELEASE_ASSERT immediately after that check aborts the program. In this specific case the problem appears when running a set of tests using Ruby. WebKit is loaded directly by the Ruby process (using gobject-introspection), and the reason why this crashes is that Ruby is installing handlers for SIGUSR1, SIGUSR2 among others: https://sources.debian.org/src/ruby2.7/2.7.2-4/signal.c/#L1551 With a different signal there's no crash: $ JSC_SIGNAL_FOR_GC=30 xvfb-run ruby webkit2-gtk/test/run-test.rb 1740398 2 seb128 2021-03-16 23:27:49 -0700 Thanks, the JSC_SIGNAL_FOR_GC trick is fixing the issue ... should it be reported to ruby-gnome instead now? 1740623 3 berto 2021-03-17 02:38:13 -0700 I suspect that this can be a common problem and therefore WebKit should not abort on a situation like this. I wouldn't report the bug to ruby-gnome yet. Some possible alternatives: 1) Keep the previous behavior and don't abort if SIGUSR1 was already being handled. Optionally emit a warning. 2) Try different signals (USR1, USR2, ...) before aborting. 1740640 4 ysuzuki 2021-03-17 03:22:32 -0700 I have a question. (In reply to Alberto Garcia from comment #3) > I suspect that this can be a common problem and therefore WebKit should not > abort on a situation like this. I wouldn't report the bug to ruby-gnome yet. > > Some possible alternatives: > > 1) Keep the previous behavior and don't abort if SIGUSR1 was already being > handled. Optionally emit a warning. In this case, what signal handler is installed? If JSC signal handler is not set, then JSC will not work. On the other hand, is it OK to discard ruby's signal handler? 1740668 5 berto 2021-03-17 04:59:45 -0700 (In reply to Yusuke Suzuki from comment #4) > > Some possible alternatives: > > > > 1) Keep the previous behavior and don't abort if SIGUSR1 was > > already being handled. Optionally emit a warning. > In this case, what signal handler is installed? If JSC signal > handler is not set, then JSC will not work. On the other hand, is it > OK to discard ruby's signal handler? In this case the JSC handler would be installed, overriding the previous one. What I'm trying to avoid is that an app that was working fine suddenly breaks after upgrading WebKit (as this bug report shows). Even if the cost is that an app that was already broken remains broken. In the case of Ruby: I just took a look at the code now and as far as I can see the only reason why it is handling SIGUSR1 (among others) is to run any potential handler for those signals in the Ruby script being executed. So yes, if a Ruby script needs to handle the same signal that JSC is using then there is a problem. I understand that this is true in general for any WebKit embedder. 1740978 6 ysuzuki 2021-03-17 17:58:43 -0700 (In reply to Alberto Garcia from comment #5) > (In reply to Yusuke Suzuki from comment #4) > > > Some possible alternatives: > > > > > > 1) Keep the previous behavior and don't abort if SIGUSR1 was > > > already being handled. Optionally emit a warning. > > > In this case, what signal handler is installed? If JSC signal > > handler is not set, then JSC will not work. On the other hand, is it > > OK to discard ruby's signal handler? > > In this case the JSC handler would be installed, overriding the > previous one. > > What I'm trying to avoid is that an app that was working fine suddenly > breaks after upgrading WebKit (as this bug report shows). Even if the > cost is that an app that was already broken remains broken. > > In the case of Ruby: I just took a look at the code now and as far as > I can see the only reason why it is handling SIGUSR1 (among others) is > to run any potential handler for those signals in the Ruby script > being executed. > > So yes, if a Ruby script needs to handle the same signal that JSC is > using then there is a problem. I understand that this is true in > general for any WebKit embedder. I think it is OK to remove this RELEASE_ASSERT for now, and we should also open a bug in Ruby binding implementations to configure JSC signal from SIGUSR1 to different one instead (e.g. SIGPWR, SIGXCPU etc.). 1741094 7 berto 2021-03-18 01:58:59 -0700 (In reply to Yusuke Suzuki from comment #6) > > So yes, if a Ruby script needs to handle the same signal that JSC > > is using then there is a problem. I understand that this is true > > in general for any WebKit embedder. > I think it is OK to remove this RELEASE_ASSERT for now, and we > should also open a bug in Ruby binding implementations to configure > JSC signal from SIGUSR1 to different one instead (e.g. SIGPWR, > SIGXCPU etc.). If we just remove the RELEASE_ASSERT then the JSC signal handler won't be installed, which is different from the previous behavior, is that ok? My initial idea was to replace the handler (as WebKit used to do) and emit a warning / log message. 1743286 8 mcatanzaro 2021-03-24 06:10:50 -0700 (In reply to Alberto Garcia from comment #7) > My initial idea was to replace the handler (as WebKit used to do) and > emit a warning / log message. That seems better than not registering our SIGUSR1 handler. Alternatively, we could just crash, which seems like a more reliable way to ensure apps know they must not handle SIGUSR1 if they link to WebKit. 1743302 9 berto 2021-03-24 07:26:27 -0700 (In reply to Michael Catanzaro from comment #8) > Alternatively, we could just crash, which seems like a more reliable way to > ensure apps know they must not handle SIGUSR1 if they link to WebKit. "Just crashing" is what's happening now. 1743729 10 424233 berto 2021-03-25 04:33:11 -0700 Created attachment 424233 Patch This patch replaces any existing handler and emits a warning. I verified that the ruby-gnome tests pass successfully with it. 1743731 11 424233 cgarcia 2021-03-25 04:36:20 -0700 Comment on attachment 424233 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=424233&action=review > Source/WTF/wtf/posix/ThreadingPOSIX.cpp:213 > + WTFLogAlways("Overriding existing handler for signal %d. " > + "Set JSC_SIGNAL_FOR_GC if you want WebKit to use a different signal", signal); I would use a single line even if it's a bit long. 1743733 12 424234 berto 2021-03-25 04:44:08 -0700 Created attachment 424234 Patch v2 Same patch with a single line for the warning message 1743737 13 berto 2021-03-25 05:12:28 -0700 Committed r275014: <https://commits.webkit.org/r275014> 424233 2021-03-25 04:33:11 -0700 2021-03-25 04:44:08 -0700 Patch 223069.diff text/plain 1453 berto ZGlmZiAtLWdpdCBhL1NvdXJjZS9XVEYvQ2hhbmdlTG9nIGIvU291cmNlL1dURi9DaGFuZ2VMb2cK aW5kZXggYjZhMmE1MjgyY2U5Li5mZDU3Mzg0YjEwYzYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYv Q2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTUgQEAKKzIw MjEtMDMtMjUgIEFsYmVydG8gR2FyY2lhICA8YmVydG9AaWdhbGlhLmNvbT4KKworICAgICAgICBS RUdSRVNTSU9OKHIyNzE1NjApOiBbTGludXhdIHJlbGVhc2UgYXNzZXJ0IGluIFRocmVhZDo6aW5p dGlhbGl6ZVBsYXRmb3JtVGhyZWFkaW5nCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3Jn L3Nob3dfYnVnLmNnaT9pZD0yMjMwNjkKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9P UFMhKS4KKworICAgICAgICBSZXBsYWNlIGFuIGV4aXN0aW5nIHNpZ25hbCBoYW5kbGVyIGluc3Rl YWQgb2YgYWJvcnRpbmcuCisKKyAgICAgICAgKiB3dGYvcG9zaXgvVGhyZWFkaW5nUE9TSVguY3Bw OgorICAgICAgICAoV1RGOjpUaHJlYWQ6OmluaXRpYWxpemVQbGF0Zm9ybVRocmVhZGluZyk6CisK IDIwMjEtMDMtMjQgIE1hcmsgTGFtICA8bWFyay5sYW1AYXBwbGUuY29tPgogCiAgICAgICAgIFdU Rjo6c2V0UGVybWlzc2lvbnNPZkNvbmZpZ1BhZ2UoKSBzaG91bGQgYWxsb3cgaXRzIFZNX0ZMQUdT X1BFUk1BTkVOVCB3b3JrYXJvdW5kIHVuY29uZGl0aW9uYWxseS4KZGlmZiAtLWdpdCBhL1NvdXJj ZS9XVEYvd3RmL3Bvc2l4L1RocmVhZGluZ1BPU0lYLmNwcCBiL1NvdXJjZS9XVEYvd3RmL3Bvc2l4 L1RocmVhZGluZ1BPU0lYLmNwcAppbmRleCA5ZTczNjRiODU2YTUuLjhmMWJmNGQzZDA3NyAxMDA2 NDQKLS0tIGEvU291cmNlL1dURi93dGYvcG9zaXgvVGhyZWFkaW5nUE9TSVguY3BwCisrKyBiL1Nv dXJjZS9XVEYvd3RmL3Bvc2l4L1RocmVhZGluZ1BPU0lYLmNwcApAQCAtMjA5LDcgKzIwOSw4IEBA IHZvaWQgVGhyZWFkOjppbml0aWFsaXplUGxhdGZvcm1UaHJlYWRpbmcoKQogICAgICAgICAgICAg cmV0dXJuIGZhbHNlOwogICAgICAgICAvLyBJdCBoYXMgc2lnbmFsIGFscmVhZHkuCiAgICAgICAg IGlmIChvbGRBY3Rpb24uc2FfaGFuZGxlciAhPSBTSUdfREZMIHx8IGJpdHdpc2VfY2FzdDx2b2lk Kj4ob2xkQWN0aW9uLnNhX3NpZ2FjdGlvbikgIT0gYml0d2lzZV9jYXN0PHZvaWQqPihTSUdfREZM KSkKLSAgICAgICAgICAgIHJldHVybiBmYWxzZTsKKyAgICAgICAgICAgIFdURkxvZ0Fsd2F5cygi T3ZlcnJpZGluZyBleGlzdGluZyBoYW5kbGVyIGZvciBzaWduYWwgJWQuICIKKyAgICAgICAgICAg ICAgICAiU2V0IEpTQ19TSUdOQUxfRk9SX0dDIGlmIHlvdSB3YW50IFdlYktpdCB0byB1c2UgYSBk aWZmZXJlbnQgc2lnbmFsIiwgc2lnbmFsKTsKICAgICAgICAgcmV0dXJuICFzaWdhY3Rpb24oc2ln bmFsLCAmYWN0aW9uLCAwKTsKICAgICB9OwogCg== 424234 2021-03-25 04:44:08 -0700 2021-03-25 05:07:30 -0700 Patch v2 223069.diff text/plain 1433 berto ZGlmZiAtLWdpdCBhL1NvdXJjZS9XVEYvQ2hhbmdlTG9nIGIvU291cmNlL1dURi9DaGFuZ2VMb2cK aW5kZXggYjZhMmE1MjgyY2U5Li5mZDU3Mzg0YjEwYzYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYv Q2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTUgQEAKKzIw MjEtMDMtMjUgIEFsYmVydG8gR2FyY2lhICA8YmVydG9AaWdhbGlhLmNvbT4KKworICAgICAgICBS RUdSRVNTSU9OKHIyNzE1NjApOiBbTGludXhdIHJlbGVhc2UgYXNzZXJ0IGluIFRocmVhZDo6aW5p dGlhbGl6ZVBsYXRmb3JtVGhyZWFkaW5nCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3Jn L3Nob3dfYnVnLmNnaT9pZD0yMjMwNjkKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9P UFMhKS4KKworICAgICAgICBSZXBsYWNlIGFuIGV4aXN0aW5nIHNpZ25hbCBoYW5kbGVyIGluc3Rl YWQgb2YgYWJvcnRpbmcuCisKKyAgICAgICAgKiB3dGYvcG9zaXgvVGhyZWFkaW5nUE9TSVguY3Bw OgorICAgICAgICAoV1RGOjpUaHJlYWQ6OmluaXRpYWxpemVQbGF0Zm9ybVRocmVhZGluZyk6CisK IDIwMjEtMDMtMjQgIE1hcmsgTGFtICA8bWFyay5sYW1AYXBwbGUuY29tPgogCiAgICAgICAgIFdU Rjo6c2V0UGVybWlzc2lvbnNPZkNvbmZpZ1BhZ2UoKSBzaG91bGQgYWxsb3cgaXRzIFZNX0ZMQUdT X1BFUk1BTkVOVCB3b3JrYXJvdW5kIHVuY29uZGl0aW9uYWxseS4KZGlmZiAtLWdpdCBhL1NvdXJj ZS9XVEYvd3RmL3Bvc2l4L1RocmVhZGluZ1BPU0lYLmNwcCBiL1NvdXJjZS9XVEYvd3RmL3Bvc2l4 L1RocmVhZGluZ1BPU0lYLmNwcAppbmRleCA5ZTczNjRiODU2YTUuLmJlZmRmYzJlN2Q4ZiAxMDA2 NDQKLS0tIGEvU291cmNlL1dURi93dGYvcG9zaXgvVGhyZWFkaW5nUE9TSVguY3BwCisrKyBiL1Nv dXJjZS9XVEYvd3RmL3Bvc2l4L1RocmVhZGluZ1BPU0lYLmNwcApAQCAtMjA5LDcgKzIwOSw3IEBA IHZvaWQgVGhyZWFkOjppbml0aWFsaXplUGxhdGZvcm1UaHJlYWRpbmcoKQogICAgICAgICAgICAg cmV0dXJuIGZhbHNlOwogICAgICAgICAvLyBJdCBoYXMgc2lnbmFsIGFscmVhZHkuCiAgICAgICAg IGlmIChvbGRBY3Rpb24uc2FfaGFuZGxlciAhPSBTSUdfREZMIHx8IGJpdHdpc2VfY2FzdDx2b2lk Kj4ob2xkQWN0aW9uLnNhX3NpZ2FjdGlvbikgIT0gYml0d2lzZV9jYXN0PHZvaWQqPihTSUdfREZM KSkKLSAgICAgICAgICAgIHJldHVybiBmYWxzZTsKKyAgICAgICAgICAgIFdURkxvZ0Fsd2F5cygi T3ZlcnJpZGluZyBleGlzdGluZyBoYW5kbGVyIGZvciBzaWduYWwgJWQuIFNldCBKU0NfU0lHTkFM X0ZPUl9HQyBpZiB5b3Ugd2FudCBXZWJLaXQgdG8gdXNlIGEgZGlmZmVyZW50IHNpZ25hbCIsIHNp Z25hbCk7CiAgICAgICAgIHJldHVybiAhc2lnYWN0aW9uKHNpZ25hbCwgJmFjdGlvbiwgMCk7CiAg ICAgfTsKIAo=