Bug 301146 – REGRESSION(301701@main): Web Inspector: crash in EventFiredCallback

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Yury Semikhatsky]

After https://commits.webkit.org/301701@main we see the following crash in one of the playwright tests (https://github.com/microsoft/playwright/blob/3978aab64105a351c112221f1a244b15d1c04c2d/tests/library/video.spec.ts#L206-L234). The test opens a link in a new tab and quickly closes it. Looks like the change itself just revealed a problem that was introduced in https://commits.webkit.org/205406@main. `EventFiredCallback` added as a listener to a DOM Node in InspectorDOMAgent can be kept alive by references from the JS heap even after the DOM agent has been destroyed. No events are actually fired after the DOM agent has been destroyed, so it didn't crash before, but after 301701@main the code fails the assertion that the reference to the InspectorDOMObjected is still alive. 

```
* thread #1, name = 'WebKitWebProces', stop reason = signal SIGABRT
  * frame #0: 0x00007ac471c969fc libc.so.6`__GI___pthread_kill [inlined] __pthread_kill_implementation(no_tid=0, signo=6, threadid=134984076405632) at pthread_kill.c:44:76
    frame #1: 0x00007ac471c969b0 libc.so.6`__GI___pthread_kill [inlined] __pthread_kill_internal(signo=6, threadid=134984076405632) at pthread_kill.c:78:10
    frame #2: 0x00007ac471c969b0 libc.so.6`__GI___pthread_kill(threadid=134984076405632, signo=6) at pthread_kill.c:89:10
    frame #3: 0x00007ac471c42476 libc.so.6`__GI_raise(sig=6) at raise.c:26:13
    frame #4: 0x00007ac471c287f3 libc.so.6`__GI_abort at abort.c:79:7
    frame #5: 0x00007ac47837668a libwebkitgtk-6.0.so.4`WTFCrashWithInfo(int, char const*, char const*, int) + 26
    frame #6: 0x00007ac47a826de9 libwebkitgtk-6.0.so.4`WebCore::EventFiredCallback::~EventFiredCallback() + 153
    frame #7: 0x00007ac478c1c105 libwebkitgtk-6.0.so.4`WTF::Vector<WTF::RefPtr<WebCore::RegisteredEventListener, WTF::RawPtrTraits<WebCore::RegisteredEventListener>, WTF::DefaultRefDerefTraits<WebCore::RegisteredEventListener>>, 1ul, WTF::CrashOnOverflow, 2ul, WTF::FastMalloc>::~Vector() + 133
    frame #8: 0x00007ac47a36b842 libwebkitgtk-6.0.so.4`WebCore::EventListenerMap::clear() + 194
    frame #9: 0x00007ac47a37402f libwebkitgtk-6.0.so.4`WebCore::EventTarget::removeAllEventListeners() + 111
    frame #10: 0x00007ac47a2cbbbc libwebkitgtk-6.0.so.4`WebCore::Document::removeAllEventListeners() + 12
    frame #11: 0x00007ac47a30aa67 libwebkitgtk-6.0.so.4`WebCore::Document::~Document() + 647
    frame #12: 0x00007ac47a56e489 libwebkitgtk-6.0.so.4`WebCore::HTMLDocument::~HTMLDocument() + 9
    frame #13: 0x00007ac47a2cc8a8 libwebkitgtk-6.0.so.4`WebCore::Document::removedLastRef() + 1320
    frame #14: 0x00007ac475411698 libjavascriptcoregtk-6.0.so.1`JSC::MarkedSpace::sweepPreciseAllocations() + 168
    frame #15: 0x00007ac4753d630c libjavascriptcoregtk-6.0.so.1`JSC::Heap::finalize() + 92
    frame #16: 0x00007ac4753d5e82 libjavascriptcoregtk-6.0.so.1`JSC::Heap::handleNeedFinalize(unsigned int) + 50
    frame #17: 0x00007ac4753d1b4f libjavascriptcoregtk-6.0.so.1`JSC::Heap::finishChangingPhase(JSC::GCConductor) + 143
    frame #18: 0x00007ac4753d365e libjavascriptcoregtk-6.0.so.1`JSC::Heap::runEndPhase(JSC::GCConductor) + 2558
    frame #19: 0x00007ac4753d19c2 libjavascriptcoregtk-6.0.so.1`JSC::Heap::runCurrentPhase(JSC::GCConductor, JSC::CurrentThreadState*) + 290
    frame #20: 0x00007ac4753f47fd libjavascriptcoregtk-6.0.so.1`WTF::ScopedLambdaFunctor<void (JSC::CurrentThreadState&), JSC::Heap::collectInMutatorThread()::$_0>::implFunction(void*, JSC::CurrentThreadState&) + 29
    frame #21: 0x00007ac475409937 libjavascriptcoregtk-6.0.so.1`JSC::callWithCurrentThreadState(WTF::ScopedLambda<void (JSC::CurrentThreadState&)> const&) + 119
    frame #22: 0x00007ac4753d5f3e libjavascriptcoregtk-6.0.so.1`JSC::Heap::collectInMutatorThread() + 94
    frame #23: 0x00007ac4753d16ab libjavascriptcoregtk-6.0.so.1`JSC::Heap::waitForCollection(unsigned long) + 315
    frame #24: 0x00007ac4753d0cbf libjavascriptcoregtk-6.0.so.1`JSC::Heap::collect(JSC::Synchronousness, JSC::GCRequest) + 255
    frame #25: 0x00007ac4753c4a70 libjavascriptcoregtk-6.0.so.1`JSC::EdenGCActivityCallback::doCollection(JSC::VM&) + 48
    frame #26: 0x00007ac47aa92ffb libwebkitgtk-6.0.so.4`WTF::Detail::CallableWrapper<WebCore::OpportunisticTaskScheduler::EdenGCActivityCallback::EdenGCActivityCallback(JSC::Heap&)::$_0, void>::call() + 43
    frame #27: 0x00007ac47ac1829d libwebkitgtk-6.0.so.4`WTF::Detail::CallableWrapper<WebCore::RunLoopObserver::schedule(void*, WTF::OptionSet<WTF::RunLoop::Activity, (WTF::ConcurrencyTag)0>)::$_0, WTF::ActivityObserver::ContinueObservation>::call() + 13
    frame #28: 0x00007ac476179a34 libjavascriptcoregtk-6.0.so.1`WTF::RunLoop::notifyActivity(WTF::RunLoop::Activity) + 436
    frame #29: 0x00007ac47617980e libjavascriptcoregtk-6.0.so.1`WTF::RunLoop::runGLibMainLoopIteration(WTF::RunLoop::MayBlock) + 606
    frame #30: 0x00007ac476179d17 libjavascriptcoregtk-6.0.so.1`WTF::RunLoop::run() + 135
    frame #31: 0x00007ac478ebeb20 libwebkitgtk-6.0.so.4`WebKit::WebProcessMain(int, char**) + 144
    frame #32: 0x00007ac471c29d90 libc.so.6`__libc_start_call_main(main=(WebKitWebProcess`main), argc=3, argv=0x00007ffd91056fa8) at libc_start_call_main.h:58:16
    frame #33: 0x00007ac471c29e40 libc.so.6`__libc_start_main_impl(main=(WebKitWebProcess`main), argc=3, argv=0x00007ffd91056fa8, init=<unavailable>, fini=<unavailable>, rtld_fini=<unavailable>, stack_end=0x00007ffd91056f98) at libc-start.c:392:3
    frame #34: 0x000063cb65c2b075 WebKitWebProcess`_start + 37
```

[Yury Semikhatsky]

I'm going to send a PR that replaces the CheckedRef with a WeakPtr, but long term it seems that the code can be improved as the listeners added by the inspector should be removed if inspector front-end disconnects and if the DOM agent gets destroyed. Currently if front-end reconnects the listeners are added second time.

[Yury Semikhatsky]

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

[Yury Semikhatsky]

The implementation can probably be changed to use `InspectorInstrumentation` instead of adding the listeners that have to be removed later, but it gets more complicated because HTMLMediaElement schedules the events asynchronously (https://github.com/WebKit/WebKit/blob/bcfcdfe504cfe9bac80b153a2a6e22338b6c50a0/Source/WebCore/html/HTMLMediaElement.cpp#L1269-L1272)

[EWS]

Committed 301844@main (5e36299743f8): <https://commits.webkit.org/301844@main>

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

[Radar WebKit Bug Importer]

<rdar://problem/163084352>