Bug 193569

Summary: [GLIB] Remote Inspector: no data displayed
Product: WebKit Reporter: Serban Ungureanu <s.ungureanu>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bburg, bugs-noreply, cgarcia, drousso, ews-watchlist, joepeck, keith_miller, mark.lam, mcatanzaro, msaboff, sbarati, t.bernard, webkit-bug-importer
Priority: P2    
Version: WebKit Nightly Build   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Patch mcatanzaro: review+

Description Serban Ungureanu 2019-01-18 05:41:36 PST
Overview: After the 2.23.2(and 2.23.3) webkit gtk releases, remote inspector is not working anymore. The remote inspector attaches to a debugged minibrowser but doesn't display any data in any of the remote inspector's tabs. Moreover, the debugged minibrowser hangs even after the debugging session is ended.

Reproduction steps:
1. Run MiniBrowser and load a web page.
2. Attach the remote inspector in order to debug the minibrowser session.

Expected result: Remote debugger attaches to the running minibrowser and retrieves inspector data.
Actual result: No data is displayed in any of the tabs. The minibrowser session hangs after remote debugging is closed.

Build date and hardware:
Built on 17th January 2019, using webkitgtk 2.23.2 and 2.23.3 on Ubuntu 18.04.1 TLS

Build parameters: 
cmake -DPORT=GTK -DENABLE_OPENGL=ON -DENABLE_GLES2=ON -DENABLE_MINIBROWSER=ON -DCMAKE_BUILD_TYPE=Release -DENABLE_WEBGL=ON -DENABLE_ENCRYPTED_MEDIA=ON -DENABLE_MEDIA_SOURCE=ON -GNinja ..

Run command: 
WEBKIT_INSPECTOR_SERVER=0.0.0.0:12345 ./bin/MiniBrowser
Comment 1 Carlos Garcia Campos 2019-01-18 06:36:41 PST
This is because the target is in a deadlock in the main thread:

Thread 1 (Thread 0x7fa2e998e9c0 (LWP 32133)):
#0  futex_wait_cancelable (private=0, expected=0, futex_word=0x7fa2e81fb964) at ../sysdeps/unix/sysv/linux/futex-internal.h:88
#1  __pthread_cond_wait_common (abstime=0x0, mutex=0x7fa2e81fb910, cond=0x7fa2e81fb938) at pthread_cond_wait.c:502
#2  __pthread_cond_wait (cond=0x7fa2e81fb938, mutex=0x7fa2e81fb910) at pthread_cond_wait.c:655
#3  0x00007fa2f145326b in WTF::ThreadCondition::timedWait(WTF::Mutex&, WTF::WallTime) () from /home/cgarcia/src/git/gnome/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#4  0x00007fa2f13f145b in WTF::ParkingLot::parkConditionallyImpl(void const*, WTF::ScopedLambda<bool ()> const&, WTF::ScopedLambda<void ()> const&, WTF::TimeWithDynamicClockType const&) ()
   from /home/cgarcia/src/git/gnome/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#5  0x00007fa2f13e5aae in WTF::LockAlgorithm<unsigned char, (unsigned char)1, (unsigned char)2, WTF::EmptyLockHooks<unsigned char> >::lockSlow(WTF::Atomic<unsigned char>&) ()
   from /home/cgarcia/src/git/gnome/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#6  0x00007fa2f0ecb3aa in Inspector::RemoteInspector::sendMessageToRemote(unsigned int, WTF::String const&) ()
   from /home/cgarcia/src/git/gnome/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#7  0x00007fa2f0e5cd10 in Inspector::FrontendRouter::sendEvent(WTF::String const&) const () from /home/cgarcia/src/git/gnome/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#8  0x00007fa2f0e83f91 in Inspector::TargetFrontendDispatcher::targetCreated(WTF::RefPtr<Inspector::Protocol::Target::TargetInfo, WTF::DumbPtrTraits<Inspector::Protocol::Target::TargetInfo> >) () from /home/cgarcia/src/git/gnome/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#9  0x00007fa2f0ebe158 in Inspector::InspectorTargetAgent::connectToTargets() () from /home/cgarcia/src/git/gnome/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
--Type <RET> for more, q to quit, c to continue without paging--
#10 0x00007fa2f0e5c323 in Inspector::AgentRegistry::didCreateFrontendAndBackend(Inspector::FrontendRouter*, Inspector::BackendDispatcher*) ()
   from /home/cgarcia/src/git/gnome/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#11 0x00007fa2f4678f51 in WebKit::WebPageInspectorController::connectFrontend(Inspector::FrontendChannel&, bool, bool) ()
   from /home/cgarcia/src/git/gnome/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#12 0x00007fa2f0ece081 in Inspector::RemoteConnectionToTarget::setup(bool, bool) () from /home/cgarcia/src/git/gnome/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#13 0x00007fa2f0ece25c in Inspector::RemoteInspector::setup(unsigned int) () from /home/cgarcia/src/git/gnome/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#14 0x00007fa2f0ece2dd in Inspector::RemoteInspector::receivedSetupMessage(unsigned int) () from /home/cgarcia/src/git/gnome/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#15 0x00007fa2f0ece385 in Inspector::RemoteInspector::{lambda(_GDBusConnection*, char const*, char const*, char const*, char const*, _GVariant*, _GDBusMethodInvocation*, void*)#1}::_FUN(_GDBusConnection*, char const*, char const*, char const*, char const*, _GVariant*, _GDBusMethodInvocation*, void*) ()
   from /home/cgarcia/src/git/gnome/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#16 0x00007fa2f2078fe9 in call_in_idle_cb (user_data=<optimized out>) at ../gio/gdbusconnection.c:4847
#17 0x00007fa2f1e63588 in g_main_dispatch (context=0x55f1f3a5b150) at ../glib/gmain.c:3190
#18 g_main_context_dispatch (context=context@entry=0x55f1f3a5b150) at ../glib/gmain.c:3855
#19 0x00007fa2f1e63948 in g_main_context_iterate (context=0x55f1f3a5b150, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at ../glib/gmain.c:3928
#20 0x00007fa2f1e63c32 in g_main_loop_run (loop=0x55f1f40fd470) at ../glib/gmain.c:4124
#21 0x00007fa2f267f155 in gtk_main () at gtkmain.c:1323
#22 0x000055f1f1acf914 in main ()

See:

#12 0x00007fa2f0ece081 in Inspector::RemoteConnectionToTarget::setup(bool, bool) ()

Here we take the remote inspector mutex that is still locked when:

#6  0x00007fa2f0ecb3aa in Inspector::RemoteInspector::sendMessageToRemote(unsigned int, WTF::String const&) ()

is called. This doesn't happen in cocoa inspector because RemoteConnectionToTarget::setup() calls RemoteInspectionTarget::connect() asynchronously, so the remote inspector mutex is no longer locked.

In the GLib case I think we can get rid of the mutex when receiving DBus messages, because they are dispatched to the main thread by GDBus.
Comment 2 Carlos Garcia Campos 2019-01-21 02:56:47 PST
Created attachment 359682 [details]
Patch
Comment 3 Michael Catanzaro 2019-01-21 14:41:23 PST
Comment on attachment 359682 [details]
Patch

r=me. Moving the lock to the smaller scope is safer anyway.
Comment 4 Michael Catanzaro 2019-01-21 14:42:09 PST
Comment on attachment 359682 [details]
Patch

And thanks for this bug report, Serban. We would surely have released 2.24 with this broken had you not reported it.
Comment 5 Carlos Garcia Campos 2019-01-23 02:13:02 PST
Committed r240330: <https://trac.webkit.org/changeset/240330>