Closed Bug 1481164 Opened Last year Closed 8 months ago

Crash in InvalidArrayIndex_CRASH | mozilla::dom::indexedDB::(anonymous namespace)::ConnectionPool::NoteIdleDatabase

Categories

(Core :: Storage: IndexedDB, defect, P2, critical)

defect

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox-esr60 --- unaffected
firefox62 --- unaffected
firefox63 --- wontfix
firefox64 --- wontfix
firefox65 --- wontfix
firefox66 --- wontfix
firefox67 --- fixed
firefox68 --- fixed

People

(Reporter: gsvelto, Assigned: tt)

References

(Blocks 1 open bug)

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is
report bp-a3293fca-0511-435e-bc0e-103e40180805.
=============================================================

Top 10 frames of crashing thread:

0 libmozglue.dylib MOZ_CrashPrintf mfbt/Assertions.cpp
1 XUL InvalidArrayIndex_CRASH xpcom/ds/nsTArray.cpp:26
2 XUL mozilla::dom::indexedDB:: xpcom/ds/nsTArray.h:1100
3 XUL mozilla::dom::indexedDB:: dom/indexedDB/ActorsParent.cpp:13003
4 XUL nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1235
5 XUL NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:519
6 XUL mozilla::dom::indexedDB:: xpcom/threads/nsThreadUtils.h:324
7 XUL nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1235
8 XUL NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:519
9 XUL mozilla::ipc::MessagePumpForNonMainThreads::Run ipc/glue/MessagePump.cpp:364

=============================================================
Priority: -- → P2
:jan this is a relatively low freq crasher, on your radar
Flags: needinfo?(jvarga)
Crash Signature: [@ InvalidArrayIndex_CRASH | mozilla::dom::indexedDB::(anonymous namespace)::ConnectionPool::NoteIdleDatabase] → [@ InvalidArrayIndex_CRASH | mozilla::dom::indexedDB::(anonymous namespace)::ConnectionPool::NoteIdleDatabase] [@ InvalidArrayIndex_CRASH | void mozilla::dom::indexedDB::`anonymous namespace'::ConnectionPool::NoteIdleDatabase] [@ void mozilla::dom::inde…
OS: Mac OS X → All
Hardware: Unspecified → All
this issue is regressing in volume in firefox 63.
Removing NI
Flags: needinfo?(jvarga)
firefox63:wontfix → is that a good idea?

I may be seeing this crash: https://crash-stats.mozilla.com/report/index/777ee3fa-a1ea-4e5e-8d2c-55db40181120

In case this is the correct bug for this crash (of which I am not entirely sure) this is pretty critical as it prevents any productive work from happening in firefox on that computer. I have 63.0.3 on a second computer with no crash at all. Makes me wonder if the crash is related to the old hardware of the 2011 macbook pro they are happening on?

Can't specifically reproduce but the crash just happenes without me doing anything. I can open Firefox with one tab open (duckduckgo) and do nothing → crash. Just this morning it has crashed at least 7 times. Just now while writing this report.

This is indeed a regression in 63 and was not the case for previous firefox versions.

Any info that could help address this crash? I understand that even ~60 crashes/day are not a lot looking at the entire firefox userbase. for me this crash is so bad that I have to consider switching the browser until this is fixed as I can not be productive with the constant interruptions.
I found even more instances of this.
Crash Signature: namespace'::ConnectionPool::DatabaseInfo*)] [@ InvalidArrayIndex_CRASH | mozilla::dom::indexedDB::(anonymous namespace)::ConnectionPool::NoteIdleDatabase(mozilla::dom::indexedDB::(anonymous namespace)::ConnectionPool::DatabaseInfo*)] → namespace'::ConnectionPool::DatabaseInfo*)] [@ InvalidArrayIndex_CRASH | mozilla::dom::indexedDB::(anonymous namespace)::ConnectionPool::NoteIdleDatabase(mozilla::dom::indexedDB::(anonymous namespace)::ConnectionPool::DatabaseInfo*)] [@ _pthread_cond_w…

I've talked with the user that left comment 4 and he can easily reproduce the bug. If we could provide him with a build with the proper debugging facilities we might get to the bottom of this, Jan is there someone from the IndexedDB team who could help with this?

Flags: needinfo?(jvarga)

Added one more signature and sorted them so that it's clearer.

Crash Signature: [@ InvalidArrayIndex_CRASH | mozilla::dom::indexedDB::(anonymous namespace)::ConnectionPool::NoteIdleDatabase] [@ InvalidArrayIndex_CRASH | void mozilla::dom::indexedDB::`anonymous namespace'::ConnectionPool::NoteIdleDatabase] [@ void mozilla::dom::inde… → [@ InvalidArrayIndex_CRASH | mozilla::dom::indexedDB::(anonymous namespace)::ConnectionPool::NoteIdleDatabase] [@ InvalidArrayIndex_CRASH | mozilla::dom::indexedDB::(anonymous namespace)::ConnectionPool::NoteIdleDatabase(mozilla::dom::indexedDB::(anonymo…

... and it looks like there might be even more signatures:

https://crash-stats.mozilla.com/search/?signature=~NoteIdleDatabase#facet-signature

I don't have time to sift through all of them now but I will ASAP.

Crash Signature: mozilla::dom::indexedDB::`anonymous namespace'::ConnectionPool::DatabaseInfo*)] [@ InvalidArrayIndex_CRASH | void mozilla::dom::indexedDB::`anonymous namespace'::ConnectionPool::NoteIdleDatabase] [@ mozilla::dom::indexedDB::(anonymous namespace)::Conne… → mozilla::dom::indexedDB::`anonymous namespace'::ConnectionPool::DatabaseInfo*)] [@ InvalidArrayIndex_CRASH | nsTArray_Impl<T>::InsertElementAt<T> | mozilla::dom::indexedDB::`anonymous namespace'::ConnectionPool::NoteIdleDatabase] [@ InvalidArrayIndex_C…
Blocks: 1541370
Flags: needinfo?(jvarga)

Added an ESR signature.

Crash Signature: [@ InvalidArrayIndex_CRASH | mozilla::dom::indexedDB::(anonymous namespace)::ConnectionPool::NoteIdleDatabase] [@ InvalidArrayIndex_CRASH | mozilla::dom::indexedDB::(anonymous namespace)::ConnectionPool::NoteIdleDatabase(mozilla::dom::indexedDB::(anonymo… → [@ arena_t::RallocSmallOrLarge | moz_xrealloc | nsTArray_base<T>::EnsureCapacity<T> | nsTArray_Impl<T>::InsertElementAt<T> | mozilla::dom::indexedDB::`anonymous namespace'::ConnectionPool::NoteIdleDatabase] [@ InvalidArrayIndex_CRASH | mozilla::dom::inde…

It crashes at [1].

[1] https://searchfox.org/mozilla-central/rev/f46e2bf881d522a440b30cbf5cf8d76fc212eaf4/dom/indexedDB/ActorsParent.cpp#11381

From my initial impression, it seems there is a race for accessing the mIdleDatabases.
For instance, we insert a databaseInfo into it also on a connection thread (which can be different threads) [1-2]. At the same time, we remove an entry on the background thread [3].

Note that [2] and [3] have the same thread assertion (AssertIsOnOwningThread() for ConnectionPool), but the caller [4] of [3] is on the Background thread and the caller of [2] is on owning thread.

[1] https://crash-stats.mozilla.com/report/index/cd71d41e-7282-4874-a323-747800190422
[2] https://searchfox.org/mozilla-central/rev/ec489aa170b6486891cf3625717d6fa12bcd11c1/dom/indexedDB/ActorsParent.cpp#11380
[3] https://searchfox.org/mozilla-central/rev/ec489aa170b6486891cf3625717d6fa12bcd11c1/dom/indexedDB/ActorsParent.cpp#11558
[4] https://searchfox.org/mozilla-central/rev/ec489aa170b6486891cf3625717d6fa12bcd11c1/dom/indexedDB/ActorsParent.cpp#12692

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

It crashes at [1].

[1] https://searchfox.org/mozilla-central/rev/f46e2bf881d522a440b30cbf5cf8d76fc212eaf4/dom/indexedDB/ActorsParent.cpp#11381

From my initial impression, it seems there is a race for accessing the mIdleDatabases.
For instance, we insert a databaseInfo into it also on a connection thread (which can be different threads) [1-2]. At the same time, we remove an entry on the background thread [3].

Note that [2] and [3] have the same thread assertion (AssertIsOnOwningThread() for ConnectionPool), but the caller [4] of [3] is on the Background thread and the caller of [2] is on owning thread.

[1] https://crash-stats.mozilla.com/report/index/cd71d41e-7282-4874-a323-747800190422
[2] https://searchfox.org/mozilla-central/rev/ec489aa170b6486891cf3625717d6fa12bcd11c1/dom/indexedDB/ActorsParent.cpp#11380
[3] https://searchfox.org/mozilla-central/rev/ec489aa170b6486891cf3625717d6fa12bcd11c1/dom/indexedDB/ActorsParent.cpp#11558
[4] https://searchfox.org/mozilla-central/rev/ec489aa170b6486891cf3625717d6fa12bcd11c1/dom/indexedDB/ActorsParent.cpp#12692

On IRC discussion, Jan catches this issue. It's because the code doesn't make idleConnectionRunnable dispatch itself to the background thread when a connection of the databaseInfo is null. So that it's possible that we can read this variable on a connection thread (which shouldn't be) while there is a write on the Background thread.

Attachment #9060169 - Attachment description: Bug 1481164 - Dispatch to the Background thread even when the Connction is a nullptr in IdleConnectionRunnable::Run(); → Bug 1481164 - Dispatch to the Background thread even when the Connection is a nullptr in IdleConnectionRunnable::Run();
Pushed by shes050117@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/158510da415d
Dispatch to the Background thread even when the Connection is a nullptr in IdleConnectionRunnable::Run(); r=janv
Status: NEW → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Assignee: nobody → shes050117
Depends on: 1546892

should we consider uplifting the patch to 67 beta still at this point?

the [@ mozilla::dom::indexedDB::anonymous namespace'::ConnectionPool::NoteIdleDatabase(struct mozilla::dom::indexedDB::anonymous namespace'::ConnectionPool::DatabaseInfo*)] crash signature would account for 0.5% of browser crashes on 67.0b13 currently...

Comment on attachment 9060169 [details]
Bug 1481164 - Dispatch to the Background thread even when the Connection is a nullptr in IdleConnectionRunnable::Run();

Beta/Release Uplift Approval Request

  • User impact if declined: The patch fixes the place where breaks threading assumption for the case of the idle database if the connection is a nullptr. Thus, that might cause a race or another crash if declined.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This patch corrects the behavior and shouldn't be risky.
  • String changes made/needed:
Flags: needinfo?(shes050117)
Attachment #9060169 - Flags: approval-mozilla-beta?

Comment on attachment 9060169 [details]
Bug 1481164 - Dispatch to the Background thread even when the Connection is a nullptr in IdleConnectionRunnable::Run();

Fix for a medium volume crasher on beta, patch evaluated as low risk, uplift approved for 67 beta 15, thanks.

Attachment #9060169 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Blocks: 1546892
No longer depends on: 1546892
No longer blocks: 1546892
See Also: → 1565091
Duplicate of this bug: 1565091
You need to log in before you can comment on or make changes to this bug.