Closed Bug 1684838 Opened 2 years ago Closed 2 years ago

Crash in [@ mozilla::dom::cache::db::CacheKeys]


(Core :: Storage: Cache API, defect)




86 Branch
Tracking Status
firefox-esr78 --- unaffected
firefox84 --- unaffected
firefox85 --- fixed
firefox86 --- fixed


(Reporter: aryx, Assigned: sg)


(Blocks 1 open bug, Regression)


(Keywords: crash, regression)

Crash Data


(1 file)

New in 85.0 (only observed in betas, could be explained by low volume). The signature was known before but only affected <=1 installation per version.

Crash report:

Common crash sites:


Top 10 frames of crashing thread:

0 xul.dll mozilla::dom::cache::db::CacheKeys dom/cache/DBSchema.cpp:929
1 xul.dll mozilla::dom::cache::Manager::CacheKeysAction::RunSyncWithDBOnTarget dom/cache/Manager.cpp:1208
2 xul.dll mozilla::dom::cache::SyncDBAction::RunWithDBOnTarget dom/cache/DBAction.cpp:183
3 xul.dll mozilla::dom::cache::DBAction::RunOnTarget dom/cache/DBAction.cpp:124
4 xul.dll mozilla::dom::cache::Context::ActionRunnable::Run dom/cache/Context.cpp:647
5 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1200
6 xul.dll mozilla::ipc::MessagePumpForNonMainThreads::Run ipc/glue/MessagePump.cpp:332
7 xul.dll MessageLoop::RunHandler ipc/chromium/src/base/
8 xul.dll MessageLoop::Run ipc/chromium/src/base/
9 xul.dll static nsThread::ThreadFunc xpcom/threads/nsThread.cpp:441

From this report's dump it seems that we fail here with a null.

As savedRequest lives on the stack, the only remaining variable I see that might cause this is state, which somehow comes to live through the CACHE_TRY_INSPECT magic.

Assuming that bug 1678030 regressed this, as the first spike is right after bug 1678030 made it into beta.

Flags: needinfo?(sgiesecke)
Regressed by: 1678030
Has Regression Range: --- → yes

This was buggy before Bug 1678030. However, Bug 1678030 turned this into a nullptr crash.

Before Bug 1678030, we were assuming that hasMoreData was true after, but apparently this wasn't always the case. We should propagate this as an error instead.

Assignee: nobody → sgiesecke
Flags: needinfo?(sgiesecke)
Pushed by
Check result of CreateAndExecuteSingleStepStatement in ReadRequest. r=jstutte
Blocks: 1687305
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 86 Branch

Might be worth picking that up if we have a 85 dot release. If you agree can you request uplift to release?

Flags: needinfo?(sgiesecke)

Comment on attachment 9197737 [details]
Bug 1684838 - Check result of CreateAndExecuteSingleStepStatement in ReadRequest. r=#dom-workers-and-storage

Beta/Release Uplift Approval Request

  • User impact if declined: Exposure to parent process crashes in case of an unexpected Cache API state. The patch doesn't fix the root cause of this (Bug 1687305 tracks this), but ensures an error is propagated rather than crashing the parent process.
  • 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): The change essentially restores the behaviour before Bug 1678030.
  • String changes made/needed:
Flags: needinfo?(sgiesecke)
Attachment #9197737 - Flags: approval-mozilla-release?

Comment on attachment 9197737 [details]
Bug 1684838 - Check result of CreateAndExecuteSingleStepStatement in ReadRequest. r=#dom-workers-and-storage

crash fix, approved for 85.0.1

Attachment #9197737 - Flags: approval-mozilla-release? → approval-mozilla-release+

Adding to the 85.0.1 release notes:
"Fixed a browser crash in case of unexpected Cache API state (bug 1684838)."

You need to log in before you can comment on or make changes to this bug.