1500116 - Assertion failure: !mPendingRequestCount, at /builds/worker/workspace/build/src/dom/indexedDB/IDBTransaction.cpp:102

Status: RESOLVED FIXED

(Core :: Storage: IndexedDB, defect, P3)

Description

[Jason Kratzer [:jkratzer]]

Testcase found while fuzzing mozilla-central rev 8f709fd4aa46.  Please note that the testcase must be served via a local webserver in order to reproduce.

Assertion failure: !mPendingRequestCount, at /builds/worker/workspace/build/src/dom/indexedDB/IDBTransaction.cpp:102

rax = 0x0000000000000000   rdx = 0x0000000000000000
rcx = 0x0000000000000b40   rbx = 0x00007f8b2f1c8890
rsi = 0x00007f8b46e6b8b0   rdi = 0x00007f8b46e6a680
rbp = 0x00007ffe47437fa0   rsp = 0x00007ffe47437f90
r8 = 0x00007f8b46e6b8b0    r9 = 0x00007f8b47fdc740
r10 = 0x0000000000000000   r11 = 0x0000000000000000
r12 = 0x00007ffe47438068   r13 = 0x00007ffe47437fd8
r14 = 0x00007f8b2f1c8890   r15 = 0x00007ffe47438058
rip = 0x00007f8b371db65b
OS|Linux|0.0.0 Linux 4.15.0-36-generic #39-Ubuntu SMP Mon Sep 24 16:19:09 UTC 2018 x86_64
CPU|amd64|family 6 model 78 stepping 3|1
GPU|||
Crash|SIGSEGV /SEGV_MAPERR|0x0|0
0|0|libxul.so|mozilla::dom::IDBTransaction::~IDBTransaction()|hg:hg.mozilla.org/mozilla-central:dom/indexedDB/IDBTransaction.cpp:8f709fd4aa463ecfc38deda95ac9cc68b5095356|103|0x18
0|1|libxul.so|mozilla::dom::IDBTransaction::~IDBTransaction()|hg:hg.mozilla.org/mozilla-central:dom/indexedDB/IDBTransaction.cpp:8f709fd4aa463ecfc38deda95ac9cc68b5095356|133|0x5
0|2|libxul.so|SnowWhiteKiller::MaybeKillObject(SnowWhiteKiller::SnowWhiteObject&)|hg:hg.mozilla.org/mozilla-central:xpcom/base/nsCycleCollector.cpp:8f709fd4aa463ecfc38deda95ac9cc68b5095356|2754|0xd
0|3|libxul.so|SnowWhiteKiller::~SnowWhiteKiller()|hg:hg.mozilla.org/mozilla-central:xpcom/base/nsCycleCollector.cpp:8f709fd4aa463ecfc38deda95ac9cc68b5095356|2740|0xb
0|4|libxul.so|nsCycleCollector::FreeSnowWhite(bool)|hg:hg.mozilla.org/mozilla-central:xpcom/base/nsCycleCollector.cpp:8f709fd4aa463ecfc38deda95ac9cc68b5095356|2956|0x5
0|5|libxul.so|nsCycleCollector::BeginCollection(ccType, nsICycleCollectorListener*)|hg:hg.mozilla.org/mozilla-central:xpcom/base/nsCycleCollector.cpp:8f709fd4aa463ecfc38deda95ac9cc68b5095356|3996|0xd
0|6|libxul.so|nsCycleCollector::Collect(ccType, js::SliceBudget&, nsICycleCollectorListener*, bool)|hg:hg.mozilla.org/mozilla-central:xpcom/base/nsCycleCollector.cpp:8f709fd4aa463ecfc38deda95ac9cc68b5095356|3817|0xf
0|7|libxul.so|nsCycleCollector::ShutdownCollect()|hg:hg.mozilla.org/mozilla-central:xpcom/base/nsCycleCollector.cpp:8f709fd4aa463ecfc38deda95ac9cc68b5095356|3757|0x15
0|8|libxul.so|nsCycleCollector::Shutdown(bool)|hg:hg.mozilla.org/mozilla-central:xpcom/base/nsCycleCollector.cpp:8f709fd4aa463ecfc38deda95ac9cc68b5095356|4061|0x8
0|9|libxul.so|nsCycleCollector_shutdown(bool)|hg:hg.mozilla.org/mozilla-central:xpcom/base/nsCycleCollector.cpp:8f709fd4aa463ecfc38deda95ac9cc68b5095356|4466|0x14
0|10|libxul.so|mozilla::ShutdownXPCOM(nsIServiceManager*)|hg:hg.mozilla.org/mozilla-central:xpcom/build/XPCOMInit.cpp:8f709fd4aa463ecfc38deda95ac9cc68b5095356|1012|0xa
0|11|libxul.so|XRE_TermEmbedding()|hg:hg.mozilla.org/mozilla-central:toolkit/xre/nsEmbedFunctions.cpp:8f709fd4aa463ecfc38deda95ac9cc68b5095356|227|0x7
0|12|libxul.so|mozilla::ipc::ScopedXREEmbed::Stop()|hg:hg.mozilla.org/mozilla-central:ipc/glue/ScopedXREEmbed.cpp:8f709fd4aa463ecfc38deda95ac9cc68b5095356|108|0x5
0|13|libxul.so|XRE_InitChildProcess(int, char**, XREChildData const*)|hg:hg.mozilla.org/mozilla-central:toolkit/xre/nsEmbedFunctions.cpp:8f709fd4aa463ecfc38deda95ac9cc68b5095356|769|0x11
0|14|firefox-bin|content_process_main(mozilla::Bootstrap*, int, char**)|hg:hg.mozilla.org/mozilla-central:ipc/contentproc/plugin-container.cpp:8f709fd4aa463ecfc38deda95ac9cc68b5095356|50|0x14
0|15|firefox-bin|main|hg:hg.mozilla.org/mozilla-central:browser/app/nsBrowserApp.cpp:8f709fd4aa463ecfc38deda95ac9cc68b5095356|287|0x11
0|16|libc-2.27.so||||0x21b97
0|17|firefox-bin|MOZ_ReportAssertionFailure|hg:hg.mozilla.org/mozilla-central:mfbt/Assertions.h:8f709fd4aa463ecfc38deda95ac9cc68b5095356|164|0x5

Comment 1

[Yaron Tausky [:ytausky]]

Jan, could you please assess this?

Comment 2

[Jason Kratzer [:jkratzer]]

I realized that I never actually attached a testcase here. The original testcase I had for this issue no longer reproduces however, we are still seeing this issue. I'm working on getting a reproducible testcase and will attach it here once I do.

Comment 3

[Jason Kratzer [:jkratzer]]

A pernosco session for this issue can be found at the following URL:
https://pernos.co/debug/ob3Pey3nza1ibwaDuowVGg/index.html

Comment 4

[Tom Tung [:tt, :ttung]]

There are two alerts in this session:

MOZ_ReportAssertionFailure (aStr@0x7fc2549798d9="mValue == Initial", aFilename@0x7fc254606453="/home/forb1dden/source/mozilla-central/objdir-ff-debug-pernosco/dist/include/mozilla/dom/FlippedOnce.h", aLine=30)

MOZ_ReportAssertionFailure (aStr@0x7fdbe01ef00f="!mPendingRequestCount", aFilename@0x7fdbe0525056="/home/forb1dden/source/mozilla-central/dom/indexedDB/IDBTransaction.cpp", aLine=134)

For the first one, the alert was raised because DatabaseOperationBase::NoteActorDestroyed got called twice. (The assertion is used to ensure mActorDestroyed gets flipped once.)
The first call was initiated by Cursor<CursorType>::ActorDestroy.
And the second call was initiated by Cursor<CursorType>::CursorOpBase::Cleanup.

I guess the second alert was caused by the crash at FlippedOnce.h on the parent process. So that there is no corresponding IDBTransaction::OnRequestFinished arrives at the content process. (And thus, causing the inequality pair of OnNewRequest and OnRequestFinished. Such that mPendingRequestCount was 1 rather than expected 0 when destroying the IDBTransaction)

BTW, when I searched the callsites for DatabaseOperationBase::NoteActorDestroyed on the serachfox, I didn't find the callsite for Cursor<CursorType>::CursorOpBase::Cleanup. I guess it's probably an issue on searchfox.

Comment 5

[Simon Giesecke [:sg] [he/him]]

(In reply to Tom Tung [:tt, :ttung] from comment #4)

BTW, when I searched the callsites for DatabaseOperationBase::NoteActorDestroyed on the serachfox, I didn't find the callsite for Cursor<CursorType>::CursorOpBase::Cleanup. I guess it's probably an issue on searchfox.

I think the (only) call is from https://searchfox.org/mozilla-central/rev/1a47a74bd5ba89f2474aa27c40bd478e853f3276/dom/indexedDB/ActorsParent.cpp#17675. It's a virtual member function.

Comment 6

[Tom Tung [:tt, :ttung]]

(In reply to Simon Giesecke [:sg] [he/him] from comment #5)

(In reply to Tom Tung [:tt, :ttung] from comment #4)

BTW, when I searched the callsites for DatabaseOperationBase::NoteActorDestroyed on the serachfox, I didn't find the callsite for Cursor<CursorType>::CursorOpBase::Cleanup. I guess it's probably an issue on searchfox.

I think the (only) call is from https://searchfox.org/mozilla-central/rev/1a47a74bd5ba89f2474aa27c40bd478e853f3276/dom/indexedDB/ActorsParent.cpp#17675. It's a virtual member function.

Hmm, I think I wanted to say that I expected to see:
20660 NoteActorDestroyed(); // found in mozilla::dom::indexedDB::(anonymous namespace)::Cursor<CursorType>::CursorOpBase::Cleanup in the result for searching the uses of DatabaseOperationBase::NoteActorDestroyed.
However, it actually presents in the result so never mind. :)

Anyway, after checking the original code in D56014, I think mActorDestroyed is not originally designed to prevent NoteActorDestroyed from being called more than once. It was used to ensure NoteActorDestroyed is called before when destructing DatabaseOperationBase(). Such that I will submit a patch to update that.

Comment 7

[Simon Giesecke [:sg] [he/him]]

Hm, it's just that this bug was opened in October 2018, which was way before D56014 landed. The code has changed massively since October 2018, so it's possible that the current issue is different from the original one.

Comment 8

[Tom Tung [:tt, :ttung]]

Attachment: Bug 1500116 - Allow mActorDestroyed to be set to true more than one time;

Comment 9

[Tom Tung [:tt, :ttung]]

(In reply to Simon Giesecke [:sg] [he/him] from comment #7)

Hm, it's just that this bug was opened in October 2018, which was way before D56014 landed. The code has changed massively since October 2018, so it's possible that the current issue is different from the original one.

True, it's possible that the current issue is different from the original one. We will see if this issue (Assertion failure: !mPendingRequestCount...) still exist after patch gets landed

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/47b068651034
Allow mActorDestroyed to be set to true more than one time; r=dom-storage-reviewers,sg

Comment 11

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/47b068651034