Bug 287728
| Summary: | REGRESSION(285337@main) [GTK] EWS layout test bots getting out of space due to sequence of assertions in IPC usage of g_socket_get_fd | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Lauro Moura <lmoura> |
| Component: | WebKitGTK | Assignee: | Carlos Garcia Campos <cgarcia> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | bugs-noreply, cgarcia, mcatanzaro |
| Priority: | P2 | ||
| Version: | WebKit Nightly Build | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=287491 | ||
Lauro Moura
Usually late in the GTK EWS layout tests (with assertions enabled), this assertion starts getting triggered in multiple tests, causing the disk to fill with core dumps that pass under the tooling radar.
It seems that the socket is somehow being invalidated while `Connection::readyReadHandler` is run, leading to the assertion.
Example run-webkit-tests output:
12:38:07.692 2 worker/8 worker/8 imported/w3c/web-platform-tests/css/css-position/hypothetical-dynamic-change-003.html output stderr lines:
12:38:07.693 2 worker/8
12:38:07.693 2 worker/8 (process:917822): GLib-GIO-CRITICAL **: 12:38:07.292: g_socket_get_fd: assertion 'G_IS_SOCKET (socket)' failed
12:38:07.693 2 worker/8 imported/w3c/web-platform-tests/css/css-position/hypothetical-dynamic-change-003.html passed
Note that as the test passed, the tooling has no idea a crash happened and ignores an eventual cleanup.
Information about the core file:
# file core-pid_917822.dump
core-pid_917822.dump: ELF 64-bit LSB core file, x86-64, version 1 (SYSV), SVR4-style, from '/app/webkit/WebKitBuild/GTK/Release/bin/WebKitWebProcess 13769 28 32 --configur', real uid: 101, effective uid: 101, real gid: 103, effective gid: 103, execfn: '/app/webkit/WebKitBuild/GTK/Release/bin/WebKitWebProcess', platform: 'x86_64'
And trace:
(gdb) bt
#0 g_logv (log_domain=0x7efd58d80082 "GLib-GIO", log_level=G_LOG_LEVEL_CRITICAL, format=<optimized out>, args=args@entry=0x7efbc4ffa770) at ../glib/gmessages.c:1422
#1 0x00007efd5b11f7a3 in g_log (log_domain=log_domain@entry=0x7efd58d80082 "GLib-GIO", log_level=log_level@entry=G_LOG_LEVEL_CRITICAL, format=format@entry=0x7efd5b177f92 "%s: assertion '%s' failed") at ../glib/gmessages.c:1460
#2 0x00007efd5b12261d in g_return_if_fail_warning (log_domain=log_domain@entry=0x7efd58d80082 "GLib-GIO", pretty_function=pretty_function@entry=0x7efd58dac090 <__func__.26> "g_socket_get_fd", expression=expression@entry=0x7efd58d86669 "G_IS_SOCKET (socket)") at ../glib/gmessages.c:2930
#3 0x00007efd58cd1fd2 in g_socket_get_fd (socket=<optimized out>) at ../gio/gsocket.c:1966
#4 g_socket_get_fd (socket=0x0) at ../gio/gsocket.c:1964
#5 0x00007efd63b93330 in IPC::Connection::readyReadHandler() () at /app/webkit/WebKitBuild/GTK/Release/lib/libwebkitgtk-6.0.so.4
#6 0x00007efd5ddae157 in WTF::RunLoop::performWork() () at /app/webkit/WebKitBuild/GTK/Release/lib/libjavascriptcoregtk-6.0.so.1
#7 0x00007efd5de94049 in WTF::RunLoop::RunLoop()::{lambda(void*)#1}::_FUN(void*) () at /app/webkit/WebKitBuild/GTK/Release/lib/libjavascriptcoregtk-6.0.so.1
#8 0x00007efd5de956ef in WTF::RunLoop::{lambda(_GSource*, int (*)(void*), void*)#1}::_FUN(_GSource*, int (*)(void*), void*) () at /app/webkit/WebKitBuild/GTK/Release/lib/libjavascriptcoregtk-6.0.so.1
#9 0x00007efd5b117d36 in g_main_dispatch (context=0x7efbbc000b70) at ../glib/gmain.c:3460
#10 g_main_context_dispatch (context=0x7efbbc000b70) at ../glib/gmain.c:4200
#11 0x00007efd5b1752b8 in g_main_context_iterate.isra.0 (context=0x7efbbc000b70, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at ../glib/gmain.c:4276
#12 0x00007efd5b1173ff in g_main_loop_run (loop=0x7efbbc000da0) at ../glib/gmain.c:4479
#13 0x00007efd5de95820 in WTF::RunLoop::run() () at /app/webkit/WebKitBuild/GTK/Release/lib/libjavascriptcoregtk-6.0.so.1
#14 0x00007efd5de046c5 in WTF::Thread::entryPoint(WTF::Thread::NewThreadContext*) () at /app/webkit/WebKitBuild/GTK/Release/lib/libjavascriptcoregtk-6.0.so.1
#15 0x00007efd5de9b7a9 in WTF::wtfThreadEntryPoint(void*) () at /app/webkit/WebKitBuild/GTK/Release/lib/libjavascriptcoregtk-6.0.so.1
#16 0x00007efd583b8e39 in start_thread (arg=<optimized out>) at pthread_create.c:444
#17 0x00007efd584408c4 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:100
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Lauro Moura
Checking the post commit GTK log, these are the occurrences of `g_socket_get_fd` in the `run-webkit-tests` log, close to when this issue appeared:
290383@main.txt: 9
290390@main.txt: 925
In this range, we have 290386@main, which was also the base commit of the first failed PR.
The PR that introduced 290386@main was https://github.com/WebKit/WebKit/pull/40420. The gtk-wk2 step passed there, but checking the zip of its layout test results, we have 750 files showing the `g_socket_get_fd` assertion.
Carlos Garcia Campos
I think this actually regressed in 285337@main, where socketDescriptor() was introduced. Before, if readyReadHandler is called after the connection was closed, the read operation failed because the fd was -1, but now the socket is used and it's already nullptr. I think the safest approach would be to close the socket instead of setting it to nullptr and relay on the destructor to close it. That way the socket won't be nullptr, but -1 will be returned from g_socket_get_fd() when called after closed.
Carlos Garcia Campos
Pull request: https://github.com/WebKit/WebKit/pull/40843
EWS
Committed 290602@main (a7040fb63290): <https://commits.webkit.org/290602@main>
Reviewed commits have been landed. Closing PR #40843 and removing active labels.
Michael Catanzaro
Oops, thanks for fixing this!