Bug 282384 – Crash in WebKit::WebPageProxy::sendWheelEvent because connection is null

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: WebKit2

[Michael Catanzaro]

(gdb) bt
#0  __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at pthread_kill.c:44
#1  0x00007f344273be03 in __pthread_kill_internal (threadid=<optimized out>, signo=6) at pthread_kill.c:78
#2  0x00007f34426e308e in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
#3  0x00007f34426ca882 in __GI_abort () at abort.c:79
#4  0x00007f343e0e8ddf in WTFCrashWithInfo () at WTF/Headers/wtf/Assertions.h:864
#5  0x00007f343e5329d6 in WebKit::AuxiliaryProcessProxy::connection (this=0x0)
    at /buildstream/gnome/sdk/webkitgtk-6.0.bst/Source/WebKit/UIProcess/AuxiliaryProcessProxy.h:131
#6  WebKit::WebPageProxy::sendWheelEvent
    (this=0x7f342570dd80, frameID=..., event=..., processingSteps=..., rubberBandableEdges=..., willStartSwipe=std::optional [no contained value], wasHandledForScrolling=<optimized out>) at /buildstream/gnome/sdk/webkitgtk-6.0.bst/Source/WebKit/UIProcess/WebPageProxy.cpp:3807
#7  0x00007f343e5322d8 in WebKit::WebPageProxy::continueWheelEventHandling
    (this=0x7f342570dd80, wheelEvent=..., result=..., willStartSwipe=std::optional [no contained value])
    at /buildstream/gnome/sdk/webkitgtk-6.0.bst/Source/WebKit/UIProcess/WebPageProxy.cpp:3798
#8  0x00007f343e532096 in WebKit::WebPageProxy::handleWheelEvent (this=0x7f342570dd80, wheelEvent=...)
    at /buildstream/gnome/sdk/webkitgtk-6.0.bst/Source/WebKit/UIProcess/WebPageProxy.cpp:3767
#9  WebKit::WebPageProxy::handleNativeWheelEvent (this=0x7f342570dd80, nativeWheelEvent=<optimized out>)
    at /buildstream/gnome/sdk/webkitgtk-6.0.bst/Source/WebKit/UIProcess/WebPageProxy.cpp:3758
#10 0x00007f343e63e9ae in handleScroll
    (webViewBase=0x55cd50dc5200 [EphyWebView], deltaX=<error reading variable: That operation is not available on integers of more than 8 bytes.>, deltaY=<error reading variable: That operation is not available on integers of more than 8 bytes.>, isEnd=false, eventController=<optimized out>)
    at /buildstream/gnome/sdk/webkitgtk-6.0.bst/Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp:1650
#15 0x00007f34436d41b3 in <emit signal 'scroll' on instance 0x55cd52594f00 [GtkEventControllerScroll]>
    (instance=instance@entry=0x55cd52594f00, signal_id=<optimized out>, detail=detail@entry=0) at ../gobject/gsignal.c:3582

Crash is here:

void WebPageProxy::sendWheelEvent(WebCore::FrameIdentifier frameID, const WebWheelEvent& event, OptionSet<WheelEventProcessingSteps> processingSteps, RectEdges<bool> rubberBandableEdges, std::optional<bool> willStartSwipe, bool wasHandledForScrolling)
{
#if HAVE(DISPLAY_LINK)
    internals().wheelEventActivityHysteresis.impulse();
#endif

    Ref connection = m_legacyMainFrameProcess->connection();

AuxiliaryProcessProxy::connection returns a RefPtr, but we incorrectly assume that it's not nul and assign it directly to a Ref.

[Michael Catanzaro]

This happened with WebKitGTK 2.46.1 and, as usual, I unfortunately don't have a reproducer.

(In reply to Michael Catanzaro from comment #0)
> AuxiliaryProcessProxy::connection returns a RefPtr, but we incorrectly
> assume that it's not nul and assign it directly to a Ref.

Um, I have no clue what I was looking at this morning, but it definitely was not AuxiliaryProcessProxy::connection, which returns a normal C++ reference, IPC::Connection&, and does RELEASE_ASSERT(m_connection) to ensure it is not nullptr first. That assert is what is failing here. This can happen in two situations:

 * AuxiliaryProcessProxy::didFinishLaunching hasn't executed yet
 * AuxiliaryProcessProxy::shutDownProcess has already been executed

[Radar WebKit Bug Importer]

<rdar://problem/139429454>

[Chris Dumez]

Might be related to https://commits.webkit.org/282353@main

[Ryosuke Niwa]

Is this a crash inside sendWheelEventScrollingAccelerationCurveIfNecessary??

[Michael Catanzaro]

Hm, I think I misanalyzed this twice, in both my first comment, and then again in comment #1. The actual problem here is surely that m_legacyMainFrameProcess is nullptr. That's not expected because it's a Ref, not a RefPtr.

(In reply to Ryosuke Niwa from comment #4)
> Is this a crash inside sendWheelEventScrollingAccelerationCurveIfNecessary??

Definitely not. It crashes before then:

void WebPageProxy::sendWheelEvent(WebCore::FrameIdentifier frameID, const WebWheelEvent& event, OptionSet<WheelEventProcessingSteps> processingSteps, RectEdges<bool> rubberBandableEdges, std::optional<bool> willStartSwipe, bool wasHandledForScrolling)
{
#if HAVE(DISPLAY_LINK)
    internals().wheelEventActivityHysteresis.impulse();
#endif

    Ref connection = m_legacyMainFrameProcess->connection(); // <-- crash is here
    if (drawingArea()->shouldSendWheelEventsToEventDispatcher()) {
        sendWheelEventScrollingAccelerationCurveIfNecessary(event);

[Ryosuke Niwa]

Oh, that makes sense because I added a release assert in connection() if you have that code. However, the latest codebase doesn't have that connection() call:
https://github.com/WebKit/WebKit/blob/c21bff762cc616d7b5905b8a19dc69c054a6a4bf/Source/WebKit/UIProcess/WebPageProxy.cpp#L4032

[Ryosuke Niwa]

Looks like this was changed recently in 5ebdb0cd30e4e. Perhaps you don't have that change yet?

[Michael Catanzaro]

Right. This was reported against WebKitGTK 2.46.1. The code on this tag is: https://github.com/WebKit/WebKit/blob/626653c49d366743a82476e84c4fb23fb222406c/Source/WebKit/UIProcess/WebPageProxy.cpp#L3801

Unfortunately if m_legacyMainFrameProcess is invalid as I suspect, then it's already too late and the other changes to protect the connection don't matter.

[Charlie Wolfe]

Pull request: https://github.com/WebKit/WebKit/pull/39137

[EWS]

Committed 289017@main (81ac6d828ecd): <https://commits.webkit.org/289017@main>

Reviewed commits have been landed. Closing PR #39137 and removing active labels.

[Michael Catanzaro]

*** This bug has been marked as a duplicate of bug 283546 ***

[Michael Catanzaro]

We've now had two failed attempts to fix this (289017@main and 302030@main). Let's continue in bug #283546.