Bug 209499 – REGRESSION: ASSERTION FAILED: m_wrapper on storage/indexeddb/modern/abort-requests tests

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Jason Lawrence]

Created attachment 394414 [details]
abort-requests-cancelled-private-crash-log

storage/indexeddb/modern/abort-requests-cancelled.html
storage/indexeddb/modern/abort-requests-cancelled-private.html

Description:
These tests are flaky crashing on Catalina wk1. The crashes appear in the visible history on 03/12/2020.

History:
https://results.webkit.org/?flavor=wk1&version_name=Catalina&style=debug&suite=layout-tests&suite=layout-tests&test=storage%2Findexeddb%2Fmodern%2Fblocked-open-db-requests-private.html&test=storage%2Findexeddb%2Fodd-strings-private.html

Crash logs attached;
Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.JavaScriptCore      	0x000000010a15687e WTFCrash + 14 (Assertions.cpp:309)
1   com.apple.WebCore             	0x000000012319e64b WTFCrashWithInfo(int, char const*, char const*, int) + 27
2   com.apple.WebCore             	0x00000001254d3cb8 WebCore::JSEventListener::jsFunction(WebCore::ScriptExecutionContext&) const + 616 (JSEventListener.h:123)
3   com.apple.WebCore             	0x00000001254d2fdb WebCore::JSEventListener::handleEvent(WebCore::ScriptExecutionContext&, WebCore::Event&) + 219 (JSEventListener.cpp:115)
4   com.apple.WebCore             	0x0000000125b8e5cc WebCore::EventTarget::innerInvokeEventListeners(WebCore::Event&, WTF::Vector<WTF::RefPtr<WebCore::RegisteredEventListener, WTF::DumbPtrTraits<WebCore::RegisteredEventListener> >, 1ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>, WebCore::EventTarget::EventInvokePhase) + 956 (EventTarget.cpp:320)
5   com.apple.WebCore             	0x0000000125b8a802 WebCore::EventTarget::fireEventListeners(WebCore::Event&, WebCore::EventTarget::EventInvokePhase) + 354 (EventTarget.cpp:257)
6   com.apple.WebCore             	0x0000000125b75ef4 WebCore::EventContext::handleLocalEvents(WebCore::Event&, WebCore::EventTarget::EventInvokePhase) const + 228 (EventContext.cpp:58)
7   com.apple.WebCore             	0x0000000125b769eb WebCore::dispatchEventInDOM(WebCore::Event&, WebCore::EventPath const&) + 379 (EventDispatcher.cpp:101)
8   com.apple.WebCore             	0x0000000125b76df2 void WebCore::dispatchEventWithType<WebCore::EventTarget>(WTF::Vector<WebCore::EventTarget*, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&, WebCore::Event&) + 338 (EventDispatcher.cpp:186)

[Radar WebKit Bug Importer]

<rdar://problem/60842165>

[Jason Lawrence]

I have marked these tests as crashing while this issue is investigated.
https://trac.webkit.org/changeset/258939/webkit

[Alexey Proskuryakov]

Correct history link:

https://results.webkit.org/?flavor=wk1&version_name=Catalina&style=debug&suite=layout-tests&suite=layout-tests&test=storage%2Findexeddb%2Fmodern%2Fabort-requests-cancelled.html&test=storage%2Findexeddb%2Fmodern%2Fabort-requests-cancelled-private.html

[Chris Dumez]

Created attachment 394641 [details]
Patch

[EWS]

Committed r259068: <https://trac.webkit.org/changeset/259068>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 394641 [details].

[Sihui Liu]

Wonder whether or not we need this change for IDBDatabase and IDBDatabaseRequest, which are also not consulting ActiveDOMObject::hasPendingActivity()

[Sihui Liu]

(In reply to Sihui Liu from comment #6)
> Wonder whether or not we need this change for IDBDatabase and
> IDBDatabaseRequest, which are also not consulting
> ActiveDOMObject::hasPendingActivity()

*IDBRequest*

[Geoffrey Garen]

(In reply to Sihui Liu from comment #6)
> Wonder whether or not we need this change for IDBDatabase and
> IDBDatabaseRequest, which are also not consulting
> ActiveDOMObject::hasPendingActivity()

Yes, I believe we do. (Both should call through to IDBActiveDOMObject.)

[Chris Dumez]

(In reply to Geoffrey Garen from comment #8)
> (In reply to Sihui Liu from comment #6)
> > Wonder whether or not we need this change for IDBDatabase and
> > IDBDatabaseRequest, which are also not consulting
> > ActiveDOMObject::hasPendingActivity()
> 
> Yes, I believe we do. (Both should call through to IDBActiveDOMObject.)

Every ActiveDOMObject::hasPendingActivity() override *should* check the base ActiveDOMObject::hasPendingActivity(). It may or may have a behavior change, depending on whether or not makePendingActivity() is used. Either way though, the safe thing to do is to always query the base ActiveDOMObject::hasPendingActivity().

[Chris Dumez]

(In reply to Chris Dumez from comment #9)
> (In reply to Geoffrey Garen from comment #8)
> > (In reply to Sihui Liu from comment #6)
> > > Wonder whether or not we need this change for IDBDatabase and
> > > IDBDatabaseRequest, which are also not consulting
> > > ActiveDOMObject::hasPendingActivity()
> > 
> > Yes, I believe we do. (Both should call through to IDBActiveDOMObject.)
> 
> Every ActiveDOMObject::hasPendingActivity() override *should* check the base
> ActiveDOMObject::hasPendingActivity(). It may or may have a behavior change,
> depending on whether or not makePendingActivity() is used. Either way
> though, the safe thing to do is to always query the base
> ActiveDOMObject::hasPendingActivity().

I think I will check ALL ActiveDOMObject::hasPendingActivity() overrides and fix them all up at once.

[Chris Dumez]

*** Bug 209744 has been marked as a duplicate of this bug. ***