Bug 116672

Summary: [WPE][GTK] WebKit2.WebContext register_uri_scheme does segfault
Product: WebKit Reporter: Simon Schampijer <simon>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: UNCONFIRMED ---    
Severity: Normal CC: bugs-noreply, mcatanzaro, philip.chimento, waffle.iron+webkit
Priority: P2 Keywords: Gtk
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   

Description Simon Schampijer 2013-05-23 07:22:57 PDT
import os

from gi.repository import WebKit2
from gi.repository import Gio

def _app_scheme_cb(request, user_data):
    path = os.path.join(activity.get_bundle_path(), request.get_path())
    request.finish(Gio.File.new_for_path(path).read(None),
                   -1, Gio.content_type_guess(path, None)[0])

context = WebKit2.WebContext.get_default()
context.register_uri_scheme("activity", _app_scheme_cb, None)


The following does segfault with the trace:

Program received signal SIGSEGV, Segmentation fault.
new_threadstate (interp=0x0, init=init@entry=1) at /usr/src/debug/Python-2.7.3/Python/pystate.c:200
200 tstate->next = interp->tstate_head;
(gdb) where
#0 new_threadstate (interp=0x0, init=init@entry=1) at /usr/src/debug/Python-2.7.3/Python/pystate.c:200
#1 0x0000003161af620a in PyThreadState_New (interp=) at /usr/src/debug/Python-2.7.3/Python/pystate.c:211
#2 0x0000003161af6a8c in PyGILState_Ensure () at /usr/src/debug/Python-2.7.3/Python/pystate.c:598
#3 0x00007f44a260899e in _pygi_invoke_closure_clear_py_data (invoke_closure=, invoke_closure=)
at /home/erikos/sugar-build/source/pygobject/gi/pygi-closure.c:505
#4 0x00007f44a2608a3a in _pygi_invoke_closure_free (data=0x276af70) at /home/erikos/sugar-build/source/pygobject/gi/pygi-closure.c:594
#5 0x0000003162a05cb0 in ffi_closure_unix64_inner (closure=0x7f44acd0e3d0, rvalue=0x7fff2fa31e10, reg_args=0x7fff2fa31d60, argp=0x7fff2fa31e30 "\360Xh\002") at ../src/x86/ffi64.c:629
#6 0x0000003162a06040 in ffi_closure_unix64 () at ../src/x86/unix64.S:228
#7 0x00007f449a23779a in WTF::HashTable >, WTF::KeyValuePairKeyExtractor > >, WTF::StringHash, WTF::HashMapValueTraitsWTF::HashTraits<WTF::String, WTF::HashTraitsWTF::RefPtr<WebKitURISchemeHandler > >, WTF::HashTraitsWTF::String >::deallocateTable(WTF::KeyValuePair >, int) () from /home/erikos/sugar-build/install/lib64/libwebkit2gtk-3.0.so.25
#8 0x00007f449a235122 in webkit_web_context_finalize(_GObject) () from /home/erikos/sugar-build/install/lib64/libwebkit2gtk-3.0.so.25
#9 0x00007f44a493ef18 in g_object_unref (_object=0x26858d0) at /home/erikos/sugar-build/source/glib/gobject/gobject.c:3024
#10 0x000000315fa38df1 in run_exit_handlers () from /lib64/libc.so.6
#11 0x000000315fa38e75 in exit () from /lib64/libc.so.6
#12 0x000000315fa21a0c in libc_start_main () from /lib64/libc.so.6
#13 0x0000000000400721 in _start ()

Probably the annotation are wrong, and presume something with the destroy is going wrong here.
Comment 1 Will 2015-04-15 18:50:57 PDT
I can confirm this bug still exists in WebKitGTK 2.6.5 as packaged by Fedora 21.

It also is triggered when using WebKit via gjs and was previously reported on the GNOME bugzilla at https://bugzilla.gnome.org/show_bug.cgi?id=729611


gjs backtrace:

#0  0x00007ffff4be0a16 in js_RemoveRoot(JSRuntime*, void*) () at /lib64/libmozjs-24.so
#1  0x00007ffff7b79d1c in gjs_callback_trampoline_unref () at /lib64/libgjs.so.0
#2  0x00007fffdd112afa in WTF::HashTable<WTF::String, WTF::KeyValuePair<WTF::String, WTF::RefPtr<WebKitURISchemeHandler> >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WTF::String, WTF::RefPtr<WebKitURISchemeHandler> > >, WTF::StringHash, WTF::HashMap<WTF::String, WTF::RefPtr<WebKitURISchemeHandler>, WTF::StringHash, WTF::HashTraits<WTF::String>, WTF::HashTraits<WTF::RefPtr<WebKitURISchemeHandler> > >::KeyValuePairTraits, WTF::HashTraits<WTF::String> >::deallocateTable(WTF::KeyValuePair<WTF::String, WTF::RefPtr<WebKitURISchemeHandler> >*, int) () at /lib64/libwebkit2gtk-4.0.so.37
#3  0x00007fffdd11133b in webkit_web_context_finalize(_GObject*) () at /lib64/libwebkit2gtk-4.0.so.37
#4  0x00007ffff54e8c46 in g_object_unref () at /lib64/libgobject-2.0.so.0
#5  0x00007ffff3390392 in __run_exit_handlers () at /lib64/libc.so.6
#6  0x00007ffff33903e5 in  () at /lib64/libc.so.6
#7  0x00000000004011cc in main ()

python2.7 backtrace:

#0  0x00007ffff7b08f60 in new_threadstate () at /lib64/libpython2.7.so.1.0
#1  0x00007ffff7b0980c in PyGILState_Ensure () at /lib64/libpython2.7.so.1.0
#2  0x00007ffff058cd7f in _pygi_invoke_closure_clear_py_data.isra.3 ()
    at /usr/lib64/python2.7/site-packages/gi/_gi.so
#3  0x00007ffff058ce28 in _pygi_invoke_closure_free () at /usr/lib64/python2.7/site-packages/gi/_gi.so
#4  0x00007fffebde3afa in WTF::HashTable<WTF::String, WTF::KeyValuePair<WTF::String, WTF::RefPtr<WebKitURISchemeHandler> >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WTF::String, WTF::RefPtr<WebKitURISchemeHandler> > >, WTF::StringHash, WTF::HashMap<WTF::String, WTF::RefPtr<WebKitURISchemeHandler>, WTF::StringHash, WTF::HashTraits<WTF::String>, WTF::HashTraits<WTF::RefPtr<WebKitURISchemeHandler> > >::KeyValuePairTraits, WTF::HashTraits<WTF::String> >::deallocateTable(WTF::KeyValuePair<WTF::String, WTF::RefPtr<WebKitURISchemeHandler> >*, int) () at /lib64/libwebkit2gtk-4.0.so.37
#5  0x00007fffebde233b in webkit_web_context_finalize(_GObject*) () at /lib64/libwebkit2gtk-4.0.so.37
#6  0x00007fffefef1c46 in g_object_unref () at /lib64/libgobject-2.0.so.0
#7  0x00007ffff6d5e392 in __run_exit_handlers () at /lib64/libc.so.6
#8  0x00007ffff6d5e3e5 in  () at /lib64/libc.so.6
#9  0x00007ffff6d44fe7 in __libc_start_main () at /lib64/libc.so.6
#10 0x000000000040071e in _start ()

python3 backtrace:

#0  0x00007ffff77366e0 in sem_wait () at /lib64/libpthread.so.0
#1  0x00007ffff7a9070d in PyThread_acquire_lock_timed () at /lib64/libpython3.4m.so.1.0
#2  0x00007ffff7a78677 in new_threadstate () at /lib64/libpython3.4m.so.1.0
#3  0x00007ffff7a795c1 in PyGILState_Ensure () at /lib64/libpython3.4m.so.1.0
#4  0x00007ffff023f19f in _pygi_invoke_closure_clear_py_data.isra.3 ()
    at /usr/lib64/python3.4/site-packages/gi/_gi.cpython-34m.so
#5  0x00007ffff023f248 in _pygi_invoke_closure_free ()
    at /usr/lib64/python3.4/site-packages/gi/_gi.cpython-34m.so
#6  0x00007fffebdf1afa in WTF::HashTable<WTF::String, WTF::KeyValuePair<WTF::String, WTF::RefPtr<WebKitURISchemeHandler> >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WTF::String, WTF::RefPtr<WebKitURISchemeHandler> > >, WTF::StringHash, WTF::HashMap<WTF::String, WTF::RefPtr<WebKitURISchemeHandler>, WTF::StringHash, WTF::HashTraits<WTF::String>, WTF::HashTraits<WTF::RefPtr<WebKitURISchemeHandler> > >::KeyValuePairTraits, WTF::HashTraits<WTF::String> >::deallocateTable(WTF::KeyValuePair<WTF::String, WTF::RefPtr<WebKitURISchemeHandler> >*, int) () at /lib64/libwebkit2gtk-4.0.so.37
#7  0x00007fffebdf033b in webkit_web_context_finalize(_GObject*) () at /lib64/libwebkit2gtk-4.0.so.37
#8  0x00007fffefba3c46 in g_object_unref () at /lib64/libgobject-2.0.so.0
#9  0x00007ffff6c95392 in __run_exit_handlers () at /lib64/libc.so.6
#10 0x00007ffff6c953e5 in  () at /lib64/libc.so.6
#11 0x00007ffff6c7bfe7 in __libc_start_main () at /lib64/libc.so.6
#12 0x0000000000400b76 in _start ()
Comment 2 Philip Chimento 2015-05-18 17:33:19 PDT
What's going wrong here is that the default WebContext object is destroyed in an atexit() handler. When that happens, the GDestroyNotify function passed to register_uri_scheme() is called, but at that point the Python or JS runtime has already been shut down, so it segfaults.

A library should definitely not register any atexit() handlers. Probably it should be OK to not destroy the default WebContext, since the memory will be reclaimed at the end of the process anyhow.
Comment 3 Michael Catanzaro 2018-11-13 10:27:15 PST
(In reply to Philip Chimento from comment #2)
> What's going wrong here is that the default WebContext object is destroyed
> in an atexit() handler. When that happens, the GDestroyNotify function
> passed to register_uri_scheme() is called, but at that point the Python or
> JS runtime has already been shut down, so it segfaults.
> 
> A library should definitely not register any atexit() handlers.

Easier said than done. :(

> Probably it
> should be OK to not destroy the default WebContext, since the memory will be
> reclaimed at the end of the process anyhow.

Problem is a mismatch between C++ best-practices, and GObject.

C++ allows static objects. Best practice is to only ever store trivially-destructible types into static objects. If you store something that's not trivially-destructible, then its destructor will be run in an exit handler. In WebKit cross-platform code, we ban exit-time destructors because they get executed in nondeterministic order on Windows.

But in the GTK/WPE-specific code, we've found cases where it's unavoidable. Bug #166029 comes to mind. C++ best practice is to leak the object to avoid an exit time destructor, but GObject classes expect to be disposed and finalized and do important things besides free memory when disposed/finalized. In this case, that was to close database handles. It could also be to shut down network connections, etc. (We also have a couple manual exit handlers too, but this isn't one of those.)

So if we want to leak the default web context, we have to make sure it's safe to do so. That means checking its dispose/finalize, and the destructors of all objects in its priv struct, to make sure each and every one of them is safe to skip. And then predicting the future to ensure that no destructors are ever added that would be unsafe to skip. It's a really hard problem. I know for starters that we'll run into problems because the default WebProcessPool won't be destroyed, and that is leak-checked, so you'll get a big warning in debug builds, for instance.

So not a simple problem at all. Not at all....
Comment 4 Michael Catanzaro 2018-11-13 10:33:19 PST
BTW in this case the exit handler is coming from right here:

static gpointer createDefaultWebContext(gpointer)
{
    static GRefPtr<WebKitWebContext> webContext = adoptGRef(WEBKIT_WEB_CONTEXT(g_object_new(WEBKIT_TYPE_WEB_CONTEXT, nullptr)));
    return webContext.get();
}

To avoid, the GRefPtr would need to be removed. Then the default WebKitWebContext would be leaked. But, as explained above, we don't want to do that. Ideally we would try to find some way to ensure that pygobject and gjs shut down last of all. Normally the way to do that is to register your own exit handler first. pygobject and gjs should be able to win and run first, because they execute code before WebKit ever does.

P.S. I know the Bugzilla components are confusing, but we'll only see the bugs if you select the "WebKit Gtk" component. Sorry for taking three years to respond, but I just stumbled onto this by chance....
Comment 5 Philip Chimento 2018-11-13 15:53:03 PST
(In reply to Michael Catanzaro from comment #4)
> Ideally we would try to find some way to ensure that pygobject and
> gjs shut down last of all. Normally the way to do that is to register your
> own exit handler first. pygobject and gjs should be able to win and run
> first, because they execute code before WebKit ever does.

It's not an option for GJS to do that unless we allow WebKit's bad practices to leak into GJS, and it would also break the expectations of GJS library users that they should be able to destroy their JS context at a time of their choosing.

I understand the problem, but I think should be libraries' responsibility to at least make it possible to opt out of running weird code in static destructors or atexit() handlers.

How about API to unregister the URI scheme handlers early?

Alternatively we could advise PyGObject and GJS users to use their own WebContext rather than the default one. I seem to remember I tested that at the time and it crashed, but maybe that has been fixed in the meantime.