Bug 167003 – REGRESSION(r202615?): [GStreamer] ASSERTION FAILED: isMainThread() in WebCore::BuiltinResourceHandleConstructorMap& WebCore::builtinResourceHandleConstructorMap()

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Michael Catanzaro]

Seeing this error very frequently after with WebKit trunk from the last day or two. I'm pretty sure it's a very recent regression:

ASSERTION FAILED: isMainThread()
../../Source/WebCore/platform/network/ResourceHandle.cpp(51) : WebCore::BuiltinResourceHandleConstructorMap& WebCore::builtinResourceHandleConstructorMap()
1   0x7f5fe9ebcdc2 /home/mcatanzaro/Projects/GNOME/install/lib/libjavascriptcoregtk-4.0.so.18(WTFCrash+0x1e) [0x7f5fe9ebcdc2]
2   0x7f5ff3509a87 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(+0x669ba87) [0x7f5ff3509a87]
3   0x7f5ff3509dfe /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN7WebCore14ResourceHandle6createEPNS_17NetworkingContextERKNS_15ResourceRequestEPNS_20ResourceHandleClientEbb+0x82) [0x7f5ff3509dfe]
4   0x7f5ff3bb922d /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(+0x6d4b22d) [0x7f5ff3bb922d]
5   0x7f5ff3bbb9b8 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(+0x6d4d9b8) [0x7f5ff3bbb9b8]
6   0x7f5ff1da9f2e /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZNKSt8functionIFvvEEclEv+0x32) [0x7f5ff1da9f2e]
7   0x7f5fe9ede4cc /home/mcatanzaro/Projects/GNOME/install/lib/libjavascriptcoregtk-4.0.so.18(+0x24b84cc) [0x7f5fe9ede4cc]
8   0x7f5fe9f1d822 /home/mcatanzaro/Projects/GNOME/install/lib/libjavascriptcoregtk-4.0.so.18(+0x24f7822) [0x7f5fe9f1d822]
9   0x7f5fe57166ca /lib64/libpthread.so.0(+0x76ca) [0x7f5fe57166ca]
10  0x7f5fdeaaef7f /lib64/libc.so.6(clone+0x5f) [0x7f5fdeaaef7f]

Sorry I don't have a good backtrace. :/ ResourceHandle::create must only be called from the main thread, but it is clearly being called from a secondary thread. Unfortunately this crap backtrace does not show from where. :/

[Michael Catanzaro]

100% reproducible on http://www.riverfronttimes.com/

Will attach a full backtrace of all threads. The crashing thread is:

Thread 1 (Thread 0x7fc2f49fc700 (LWP 21128)):
#0  0x00007fc361490221 in WTFCrash ()
    at ../../Source/WTF/wtf/Assertions.cpp:323
No locals.
#1  0x00007fc36ab94311 in (anonymous namespace)::builtinResourceHandleConstructorMap () at ../../Source/WebCore/platform/network/ResourceHandle.cpp:51
        __PRETTY_FUNCTION__ = "WebCore::BuiltinResourceHandleConstructorMap& WebCore::builtinResourceHandleConstructorMap()"
        map = {m_storage = {__data = '\000' <repeats 39 times>, 
            __align = {<No data fields>}}}
#2  0x00007fc36ab94688 in (anonymous namespace)::ResourceHandle::create (
    context=0x0, request=..., client=0x7fc2f12d2310, defersLoading=true, 
    shouldContentSniff=false)
    at ../../Source/WebCore/platform/network/ResourceHandle.cpp:91
        constructor = 0x7fc3614af7f8 <WTF::RunLoop::current()+102>
        newHandle = {static isRef = <optimized out>, m_ptr = 0x7fc2f49fb9e0}
#3  0x00007fc36b247d11 in ResourceHandleStreamingClient::<lambda()>::operator()(void) const (__closure=0x43ceae0)
    at ../../Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:1074
        locker = {<WTF::AbstractLocker> = {<No data fields>}, 
          m_lockable = 0x7fc2f12d232c}
        request = {<(anonymous namespace)::ResourceRequestBase> = {m_url = {
              m_string = {m_impl = {static isRefPtr = <optimized out>, 
                  m_ptr = 0x7fc2f1316f00}}, m_isValid = true, 
              m_protocolIsInHTTPFamily = true, m_cannotBeABaseURL = false, 
              m_schemeEnd = 4, m_userStart = 7, m_userEnd = 7, 
              m_passwordEnd = 7, m_hostEnd = 33, m_portEnd = 33, 
              m_pathAfterLastSlash = 160, m_pathEnd = 171, m_queryEnd = 171, 
              m_fragmentEnd = 171}, m_timeoutInterval = 0, 
            m_firstPartyForCookies = {m_string = {m_impl = {
                  static isRefPtr = <optimized out>, m_ptr = 0x7fc2f1316f00}}, 
              m_isValid = true, m_protocolIsInHTTPFamily = true, 
              m_cannotBeABaseURL = false, m_schemeEnd = 4, m_userStart = 7, 
              m_userEnd = 7, m_passwordEnd = 7, m_hostEnd = 33, 
              m_portEnd = 33, m_pathAfterLastSlash = 160, m_pathEnd = 171, 
              m_queryEnd = 171, m_fragmentEnd = 171}, m_httpMethod = {
              m_impl = {static isRefPtr = <optimized out>, 
                m_ptr = 0x7fc2f6269b28}}, m_httpHeaderFields = {
              m_commonHeaders = {m_impl = {static m_maxLoad = <optimized out>, 
                  static m_minLoad = <optimized out>, 
                  m_table = 0x7fc2f1225200, m_tableSize = 8, 
                  m_tableSizeMask = 7, m_keyCount = 3, m_deletedCount = 0, 
                  m_iterators = 0x0, 
                  m_mutex = std::unique_ptr<WTF::Lock> containing 0x7fc2f1220020}}, m_uncommonHeaders = {m_impl = {static m_maxLoad = <optimized out>, 
                  static m_minLoad = <optimized out>, m_table = 0x0, 
                  m_tableSize = 0, m_tableSizeMask = 0, m_keyCount = 0, 
                  m_deletedCount = 0, m_iterators = 0x0, 
                  m_mutex = std::unique_ptr<WTF::Lock> containing 0x7fc2f1220028}}}, 
            m_responseContentDispositionEncodingFallbackArray = {<WTF::VectorBuffer<WTF::String, 0ul>> = {<WTF::VectorBufferBase<WTF::String>> = {m_buffer = 0x0, 
                  m_capacity = 0, 
                  m_size = 0}, <No data fields>}, <No data fields>}, 
            m_httpBody = {static isRefPtr = <optimized out>, m_ptr = 0x0}, 
            m_cachePolicy = (anonymous namespace)::UseProtocolCachePolicy, 
            m_allowCookies = true, m_resourceRequestUpdated = true, 
            m_platformRequestUpdated = false, 
            m_resourceRequestBodyUpdated = true, 
            m_platformRequestBodyUpdated = false, 
            m_reportUploadProgress = false, m_reportLoadTiming = false, 
            m_reportRawHeaders = false, m_hiddenFromInspector = false, 
            m_ignoreForRequestCount = false, 
            m_priority = (anonymous namespace)::ResourceLoadPriority::Low, 
            m_requester = (anonymous namespace)::ResourceRequestBase::Requester::Unspecified, m_initiatorIdentifier = {m_impl = {
                static isRefPtr = <optimized out>, m_ptr = 0x0}}, 
            static s_defaultTimeoutInterval = <optimized out>}, 
          m_acceptEncoding = true, m_soupFlags = (unknown: 0), 
          m_initiatingPageID = 0}
        this = 0x7fc2f12d2310
#4  0x00007fc36b24a49c in std::_Function_handler<void(), ResourceHandleStreamingClient::ResourceHandleStreamingClient(WebKitWebSrc*, WebCore::ResourceRequest&&)::<lambda()> >::_M_invoke(const std::_Any_data &) (__functor=...)
    at /usr/include/c++/6.3.1/functional:1731
No locals.
#5  0x00007fc3694251ee in std::function<void()>::operator()(void) const (
    this=0x7fc2f49fbb10) at /usr/include/c++/6.3.1/functional:2127
No locals.
#6  0x00007fc3614b18a4 in WTF::threadEntryPoint (contextData=0x7fc2f12a6000)
    at ../../Source/WTF/wtf/Threading.cpp:88
        context = 0x7fc2f12a6000
        entryPoint = {<std::_Maybe_unary_or_binary_function<void>> = {<No data fields>}, <std::_Function_base> = {static _M_max_size = 16, 
            static _M_max_align = 8, _M_functor = {_M_unused = {
                _M_object = 0x43ceae0, _M_const_object = 0x43ceae0, 
                _M_function_pointer = 0x43ceae0, 
                _M_member_pointer = (void (std::_Undefined_class::*)(std::_Undefined_class * const)) 0x43ceae0, this adjustment 140477374688804}, 
              _M_pod_data = "\340\352<\004\000\000\000\000$n\004p\303\177\000"}, 
            _M_manager = 0x7fc36b24a49f <std::_Function_base::_Base_manager<ResourceHandleStreamingClient::ResourceHandleStreamingClient(WebKitWebSrc*, WebCore::ResourceRequest&&)::<lambda()> >::_M_manager(std::_Any_data &, const std::_Any_data &, std::_Manager_operation)>}, 
          _M_invoker = 0x7fc36b24a47c <std::_Function_handler<void(), ResourceHandleStreamingClient::ResourceHandleStreamingClient(WebKitWebSrc*, WebCore::ResourceRequest&&)::<lambda()> >::_M_invoke(const std::_Any_data &)>}
#7  0x00007fc3614f0c40 in WTF::wtfThreadEntryPoint (param=0x7fc34e9e5510)
    at ../../Source/WTF/wtf/ThreadingPthreads.cpp:168
        invocation = std::unique_ptr<WTF::ThreadFunctionInvocation> containing 0x7fc34e9e5510
#8  0x00007fc35ccfc6ca in start_thread () from /lib64/libpthread.so.0
No symbol table info available.
#9  0x00007fc35608df7f in clone () from /lib64/libc.so.6
No symbol table info available.

[Michael Catanzaro]

Created attachment 300055 [details]
full backtrace

[Michael Catanzaro]

Carlos, I think you broke this when trying to fix HLS in r202615. In ResourceHandleStreamingClient::ResourceHandleStreamingClient, you create the ResourceHandleStreamingClient thread, in which you immediately call ResourceHandle::create. But this causes an immediate crash in debug builds because ResourceHandles must only be created on the main thread.

I am really confused why we did not notice this until now, because creating a ResourceHandle off the main thread has been an immediate assert for a long time. It's not some new restriction.

[Carlos Garcia Campos]

Created attachment 300089 [details]
Patch

[Michael Catanzaro]

Comment on attachment 300089 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=300089&action=review

> Source/WebCore/ChangeLog:13
> +        (ResourceHandleStreamingClient::ResourceHandleStreamingClient): Create a SoupNetworkSession and apss it to ResourceHandle::create().

apss -> pass

> Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:1086
> +#else
>              m_resource = ResourceHandle::create(nullptr /*context*/, request, this, true, false);
> +#endif

But this is always going to crash in the non-soup case.

Since there are zero non-soup ports using GStreamer (even Sony wants to avoid GStreamer), you should make it mandatory instead of leaving this broken code here for new ports to stumble upon. If you don't want to do that, then at the very least use RELEASE_ASSERT_NOT_REACHED() and add a FIXME.

[Carlos Garcia Campos]

(In reply to comment #5)
> Comment on attachment 300089 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=300089&action=review
> 
> > Source/WebCore/ChangeLog:13
> > +        (ResourceHandleStreamingClient::ResourceHandleStreamingClient): Create a SoupNetworkSession and apss it to ResourceHandle::create().
> 
> apss -> pass
> 
> > Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:1086
> > +#else
> >              m_resource = ResourceHandle::create(nullptr /*context*/, request, this, true, false);
> > +#endif
> 
> But this is always going to crash in the non-soup case.

Yes, downstreams will have to do their own thing I guess. I can't do more in upstream.

> Since there are zero non-soup ports using GStreamer (even Sony wants to
> avoid GStreamer), you should make it mandatory instead of leaving this
> broken code here for new ports to stumble upon. If you don't want to do
> that, then at the very least use RELEASE_ASSERT_NOT_REACHED() and add a
> FIXME.

I'll add an ASSERT, I don't see why crashing a release build because of that, it has always worked in release, I don't think this is fatal at all.

[Carlos Garcia Campos]

Committed r211372: <http://trac.webkit.org/changeset/211372>

[Michael Catanzaro]

(In reply to comment #6)
> I'll add an ASSERT, I don't see why crashing a release build because of
> that, it has always worked in release, I don't think this is fatal at all.

The code is super fragile and not suitable for use. It's only due to luck that we don't have a thread safety bug there. Anyway, I guess the FIXME is sufficient.

[Konstantin Tokarev]

(In reply to comment #5)
> Since there are zero non-soup ports using GStreamer

This is not exactly true, unless you mean upstream ports.

[Carlos Garcia Campos]

(In reply to comment #9)
> (In reply to comment #5)
> > Since there are zero non-soup ports using GStreamer
> 
> This is not exactly true, unless you mean upstream ports.

Of course, this is the upstream bugzilla

[Michael Catanzaro]

Since we decided to maintain the codepath upstream, I think it's reasonable to create an upstream bug to indicate that it's still broken, even though no ports build it. Otherwise, we should remove the non-soup path; that's a clear indication that some downstream work is required to make it work.