Bug 168375 – REGRESSION(r212345): [GTK] ASSERT(cookieChangeCallbackMap().contains(jar)) failed in WebCore::stopObservingCookieChanges:54

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Michael Catanzaro]

I hit this crash in when closing Epiphany. I'm not sure, but I presume it's a regression from r212283 "CookieManager only works with the default session" as I've updated recently and never seen this before.

#0  0x00007f5801621fac in WTFCrash ()
    at ../../Source/WTF/wtf/Assertions.cpp:323
No locals.
#1  0x00007f580b2384aa in (anonymous namespace)::stopObservingCookieChanges (
    storageSession=...)
    at ../../Source/WebCore/platform/network/soup/CookieStorageSoup.cpp:54
        jar = 0x2197540
        __PRETTY_FUNCTION__ = "void WebCore::stopObservingCookieChanges(const WebCore::NetworkStorageSession&)"
#2  0x00007f580978a12b in (anonymous namespace)::WebCookieManager::stopObservingCookieChanges (this=0x2181820, sessionID=...)
    at ../../Source/WebKit2/WebProcess/Cookies/WebCookieManager.cpp:105
        storageSession = 0x7f57eebae820
#3  0x00007f5809c10873 in IPC::callMemberFunctionImpl<WebKit::WebCookieManager, void (WebKit::WebCookieManager::*)(WebCore::SessionID), std::tuple<WebCore::SessionID>, 0ul>((anonymous namespace)::WebCookieManager *, void ((anonymous namespace)::WebCookieManager::*)((anonymous namespace)::WebCookieManager * const, (anonymous namespace)::SessionID), <unknown type in /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37, CU 0x0, DIE 0x1bc02>, std::index_sequence)
    (object=0x2181820, function=
    (void ((anonymous namespace)::WebCookieManager::*)((anonymous namespace)::WebCookieManager * const, (anonymous namespace)::SessionID)) 0x7f580978a0f8 <(anonymous namespace)::WebCookieManager::stopObservingCookieChanges((anonymous namespace)::SessionID)>, 
    args=<unknown type in /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37, CU 0x0, DIE 0x1bc02>)
    at ../../Source/WebKit2/Platform/IPC/HandleMessage.h:40
No locals.
#4  0x00007f5809c1030e in IPC::callMemberFunction<WebKit::WebCookieManager, void (WebKit::WebCookieManager::*)(WebCore::SessionID), std::tuple<WebCore::SessionID> >(<unknown type in /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37, CU 0x0, DIE 0x1bc02>, (anonymous namespace)::WebCookieManager *, void ((anonymous namespace)::WebCookieManager::*)((anonymous namespace)::WebCookieManager * const, (anonymous namespace)::SessionID)) (
    args=<unknown type in /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37, CU 0x0, DIE 0x1bc02>, object=0x2181820, function=
    (void ((anonymous namespace)::WebCookieManager::*)((anonymous namespace)::WebCookieManager * const, (anonymous namespace)::SessionID)) 0x7f580978a0f8 <(anonymous namespace)::WebCookieManager::stopObservingCookieChanges((anonymous namespace)::SessionID)>) at ../../Source/WebKit2/Platform/IPC/HandleMessage.h:46
No locals.
#5  0x00007f5809c1010a in IPC::handleMessage<Messages::WebCookieManager::StopObservingCookieChanges, WebKit::WebCookieManager, void (WebKit::WebCookieManager::*)(WebCore::SessionID)> (decoder=..., object=0x2181820, function=
    (void ((anonymous namespace)::WebCookieManager::*)((anonymous namespace)::WebCookieManager * const, (anonymous namespace)::SessionID)) 0x7f580978a0f8 <(anonymous namespace)::WebCookieManager::stopObservingCookieChanges((anonymous namespace)::SessionID)>) at ../../Source/WebKit2/Platform/IPC/HandleMessage.h:126
        __PRETTY_FUNCTION__ = "void IPC::handleMessage(IPC::Decoder&, C*, MF) [with T = Messages::WebCookieManager::StopObservingCookieChanges; C = WebKit::WebCookieManager; MF = void (WebKit::WebCookieManager::*)(WebCore::SessionI"...
        arguments = std::tuple containing = {[1] = {m_sessionID = 1}}
#6  0x00007f5809c0f730 in (anonymous namespace)::WebCookieManager::didReceiveMessage (this=0x2181820, connection=..., decoder=...)
    at DerivedSources/WebKit2/WebCookieManagerMessageReceiver.cpp:76
        __PRETTY_FUNCTION__ = "virtual void WebKit::WebCookieManager::didReceiveMessage(IPC::Connection&, IPC::Decoder&)"
#7  0x00007f58094fc765 in IPC::MessageReceiverMap::dispatchMessage (
    this=0x7f580fd166d0 <WebKit::NetworkProcess::singleton()::networkProcess+112>, connection=..., decoder=...)
    at ../../Source/WebKit2/Platform/IPC/MessageReceiverMap.cpp:118
        messageReceiver = 0x2181830
        __PRETTY_FUNCTION__ = "bool IPC::MessageReceiverMap::dispatchMessage(IPC::Connection&, IPC::Decoder&)"
#8  0x00007f5809908f30 in (anonymous namespace)::NetworkProcess::didReceiveMessage (this=0x7f580fd16660 <WebKit::NetworkProcess::singleton()::networkProcess>, 
    connection=..., decoder=...)
    at ../../Source/WebKit2/NetworkProcess/NetworkProcess.cpp:149
No locals.
#9  0x00007f58094e3052 in IPC::Connection::dispatchMessage (
    this=0x7f57eebf6000, decoder=...)
    at ../../Source/WebKit2/Platform/IPC/Connection.cpp:897
No locals.
#10 0x00007f58094e31bc in IPC::Connection::dispatchMessage (
    this=0x7f57eebf6000, 
    message=std::unique_ptr<IPC::Decoder> containing 0x7f57eebec060)
    at ../../Source/WebKit2/Platform/IPC/Connection.cpp:924
        oldDidReceiveInvalidMessage = false
#11 0x00007f58094e33ae in IPC::Connection::dispatchOneMessage (
    this=0x7f57eebf6000)
    at ../../Source/WebKit2/Platform/IPC/Connection.cpp:955
        message = std::unique_ptr<IPC::Decoder> containing 0x0
#12 0x00007f58094e2f38 in IPC::Connection::<lambda()>::operator()(void) (
    __closure=0x7f57eebed028)
    at ../../Source/WebKit2/Platform/IPC/Connection.cpp:891
        protectedThis = {static isRef = <optimized out>, 
          m_ptr = 0x7f57eebf6000}
#13 0x00007f58094e8f94 in WTF::Function<void()>::CallableWrapper<IPC::Connection::enqueueIncomingMessage(std::unique_ptr<IPC::Decoder>)::<lambda()> >::call(void) (this=0x7f57eebed020) at ../../Source/WTF/wtf/Function.h:89
No locals.
#14 0x00007f58094af141 in WTF::Function<void()>::operator()(void) const (
    this=0x7ffc03efa420) at ../../Source/WTF/wtf/Function.h:50
No locals.
#15 0x00007f58016855f6 in WTF::RunLoop::performWork (this=0x7f57eebf9180)
    at ../../Source/WTF/wtf/RunLoop.cpp:105
        function = {
          m_callableWrapper = std::unique_ptr<WTF::Function<void()>::CallableWrapperBase> containing 0x7f57eebed020}
        functionsToHandle = 3
#16 0x00007f5801682bce in WTF::RunLoop::<lambda(gpointer)>::operator()(gpointer) const (__closure=0x0, userData=0x7f57eebf9180)
    at ../../Source/WTF/wtf/glib/RunLoopGLib.cpp:66
No locals.
#17 0x00007f5801682bf2 in WTF::RunLoop::<lambda(gpointer)>::_FUN(gpointer) ()
    at ../../Source/WTF/wtf/glib/RunLoopGLib.cpp:68
No locals.
#18 0x00007f5801682b6e in WTF::<lambda(GSource*, GSourceFunc, gpointer)>::operator()(GSource *, GSourceFunc, gpointer) const (__closure=0x0, source=0x2181600, 
    callback=0x7f5801682bd5 <WTF::RunLoop::<lambda(gpointer)>::_FUN(gpointer)>, userData=0x7f57eebf9180) at ../../Source/WTF/wtf/glib/RunLoopGLib.cpp:44
No locals.
#19 0x00007f5801682b9d in WTF::<lambda(GSource*, GSourceFunc, gpointer)>::_FUN(GSource *, GSourceFunc, gpointer) ()
    at ../../Source/WTF/wtf/glib/RunLoopGLib.cpp:45
No locals.
#20 0x00007f57f9a1633e in g_main_dispatch (context=0x2180c80)
    at /home/mcatanzaro/Projects/GNOME/glib/glib/gmain.c:3203
        dispatch = 0x7f5801682b70 <WTF::<lambda(GSource*, GSourceFunc, gpointer)>::_FUN(GSource *, GSourceFunc, gpointer)>
        prev_source = 0x0
        was_in_call = 0
        user_data = 0x7f57eebf9180
        callback = 0x7f5801682bd5 <WTF::RunLoop::<lambda(gpointer)>::_FUN(gpointer)>
        cb_funcs = 0x7f57f9ceca40 <g_source_callback_funcs>
        cb_data = 0x2180dd0
        need_destroy = 0
        source = 0x2181600
        current = 0x216a730
        i = 0
        __func__ = "g_main_dispatch"
#21 0x00007f57f9a171c0 in g_main_context_dispatch (context=0x2180c80)
    at /home/mcatanzaro/Projects/GNOME/glib/glib/gmain.c:3856
No locals.
#22 0x00007f57f9a173a4 in g_main_context_iterate (context=0x2180c80, block=1, 
    dispatch=1, self=0x2184600)
    at /home/mcatanzaro/Projects/GNOME/glib/glib/gmain.c:3929
        max_priority = 0
        timeout = 0
        some_ready = 1
        nfds = 2
        allocated_nfds = 2
        fds = 0x29bf020
#23 0x00007f57f9a177ca in g_main_loop_run (loop=0x2180d90)
    at /home/mcatanzaro/Projects/GNOME/glib/glib/gmain.c:4125
        self = 0x2184600
        __func__ = "g_main_loop_run"
#24 0x00007f580168309d in WTF::RunLoop::run ()
    at ../../Source/WTF/wtf/glib/RunLoopGLib.cpp:94
        runLoop = 
    @0x7f57eebf9180: {<WTF::FunctionDispatcher> = {<WTF::ThreadSafeRefCounted<WTF::FunctionDispatcher>> = {<WTF::ThreadSafeRefCountedBase> = {
                m_refCount = {<std::__atomic_base<unsigned int>> = {
                    static _S_alignment = 4, 
                    _M_i = 1}, <No data fields>}}, <No data fields>}, 
            _vptr.FunctionDispatcher = 0x7f5802612ac8 <vtable for WTF::RunLoop+16>}, m_functionQueueLock = {m_mutex = {__data = {__lock = 0, __count = 0, 
                __owner = 0, __nusers = 0, __kind = 512, __spins = 0, 
                __elision = 0, __list = {__prev = 0x0, __next = 0x0}}, 
              __size = '\000' <repeats 17 times>, "\002", '\000' <repeats 21 times>, __align = 0}}, m_functionQueue = {m_start = 2, m_end = 4, 
            m_buffer = {<WTF::VectorBufferBase<WTF::Function<void()> >> = {
                m_buffer = 0x7f57eebf1180, m_capacity = 16, 
                m_size = 0}, <No data fields>}, m_iterators = 0x0}, 
          m_mainContext = {m_ptr = 0x2180c80}, 
          m_mainLoops = {<WTF::VectorBuffer<WTF::GRefPtr<_GMainLoop>, 0ul>> = {<WTF::VectorBufferBase<WTF::GRefPtr<_GMainLoop> >> = {
                m_buffer = 0x7f57eebf9200, m_capacity = 16, 
                m_size = 1}, <No data fields>}, <No data fields>}, m_source = {
            m_ptr = 0x2181600}}
        mainContext = 0x2180c80
        __PRETTY_FUNCTION__ = "static void WTF::RunLoop::run()"
        innermostLoop = 0x2180d90
        nestedMainLoop = 0x0
#25 0x00007f580999862e in (anonymous namespace)::ChildProcessMain<WebKit::NetworkProcess, WebKit::ChildProcessMainBase> (argc=2, argv=0x7ffc03efa828)
    at ../../Source/WebKit2/Shared/unix/ChildProcessMain.h:61
        childMain = <incomplete type>
#26 0x00007f58099985a5 in (anonymous namespace)::NetworkProcessMainUnix (
    argc=2, argv=0x7ffc03efa828)
    at ../../Source/WebKit2/NetworkProcess/soup/NetworkProcessMainSoup.cpp:37
No locals.
#27 0x0000000000400bea in main (argc=2, argv=0x7ffc03efa828)
    at ../../Source/WebKit2/NetworkProcess/EntryPoint/unix/NetworkProcessMain.cpp:44
No locals.

[Carlos Garcia Campos]

I don't think it's r212283, there's absolutely no change in behavior in that revision. If any, it would be r212345.

[Carlos Garcia Campos]

hmm, I was thinking in the UI process, in the web process I updated the cookies observer, there shouldn't be any change in behavior, but it could be... I still think it's more likely r212345, though.

[Michael Catanzaro]

Ah, this is a debug assertion:

void startObservingCookieChanges(const NetworkStorageSession& storageSession, std::function<void ()>&& callback)
{
    auto* jar = storageSession.cookieStorage();
    ASSERT(!cookieChangeCallbackMap().contains(jar));
    cookieChangeCallbackMap().add(jar, WTFMove(callback));
    g_signal_connect(jar, "changed", G_CALLBACK(soupCookiesChanged), nullptr);
}

[Michael Catanzaro]

Er sorry, it's stop not start. Crash is on shutdown:

void stopObservingCookieChanges(const NetworkStorageSession& storageSession)
{
    auto* jar = storageSession.cookieStorage();
    ASSERT(cookieChangeCallbackMap().contains(jar));
    cookieChangeCallbackMap().remove(jar);
    g_signal_handlers_disconnect_by_func(jar, reinterpret_cast<void*>(soupCookiesChanged), nullptr);
}

So I guess it's not properly paired with a call to startObservingCookieChanges().

[Carlos Garcia Campos]

I can't reproduce the issue, but I think there are several things with the session management that could leave to problems. This crash could be because because cookie manager sends the stop observing cookie changes relaunching the network process, so if we call this after the network process finishes, we launch a new network process for which start observing changes was not called. But there are more situations this can fail in one way or another. The main problem is that the website data store creates the session ID, but it's web page on creation the one ensuring the session in the web and network processes. So, if you setup your cookie manager before the first web page is created, the start message will not work for ephemeral sessions, because it doesn't exist yet in the network process. But it will exist when we call stop. Were you using a private session when it crashed?

[Carlos Garcia Campos]

In the GTK+ port we always want to observe cookie changes, so we could probably get rid of the start/stop messages and all the inconsistencies.

[Michael Catanzaro]

(In reply to comment #5)
> I can't reproduce the issue, but I think there are several things with the
> session management that could leave to problems. This crash could be because
> because cookie manager sends the stop observing cookie changes relaunching
> the network process, so if we call this after the network process finishes,
> we launch a new network process for which start observing changes was not
> called. 

That could definitely be it. I likely closed Epiphany with Ctrl+C, which would have caused network process termination to race with this shutdown message.

> But there are more situations this can fail in one way or another.
> The main problem is that the website data store creates the session ID, but
> it's web page on creation the one ensuring the session in the web and
> network processes. So, if you setup your cookie manager before the first web
> page is created, the start message will not work for ephemeral sessions,
> because it doesn't exist yet in the network process. But it will exist when
> we call stop. Were you using a private session when it crashed?

No, I was not using a private session, but it sounds like we need to fix that too.

[Carlos Garcia Campos]

Created attachment 301898 [details]
Patch

[WebKit Commit Bot]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[WebKit Commit Bot]

Attachment 301898 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/network/soup/NetworkStorageSessionSoup.cpp:128:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/WebCookieManagerProxy.cpp:146:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/platform/network/NetworkStorageSession.h:78:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/platform/network/NetworkStorageSession.h:99:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/WebCookieManagerProxy.h:80:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/platform/network/soup/CookieStorageSoup.cpp:26:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 6 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Michael Catanzaro]

r=me on the soup and GTK changes. Also needs an owner.

[Carlos Garcia Campos]

Alex?

[Carlos Garcia Campos]

Committed r212626: <http://trac.webkit.org/changeset/212626>