Closed Bug 1684838 Opened 4 months ago Closed 3 months ago

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

Categories

(Core :: Storage: Cache API, defect)

defect

Tracking

()

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

People

(Reporter: aryx, Assigned: sg)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: crash, regression)

Crash Data

Attachments

(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: https://crash-stats.mozilla.org/report/index/9f542882-fcd6-4c9f-853b-55d900210104

Common crash sites:

Reason: EXCEPTION_ACCESS_VIOLATION_READ

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/message_loop.cc:327
8 xul.dll MessageLoop::Run ipc/chromium/src/base/message_loop.cc:309
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

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 https://searchfox.org/mozilla-central/rev/1ff0e248eb784915f7838f2afcdf0078d4884630/dom/cache/DBSchema.cpp#2396-2400, but apparently this wasn't always the case. We should propagate this as an error instead.

Assignee: nobody → sgiesecke
Status: NEW → ASSIGNED
Flags: needinfo?(sgiesecke)
Pushed by sgiesecke@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a8625d790fe8
Check result of CreateAndExecuteSingleStepStatement in ReadRequest. r=jstutte
Blocks: 1687305
Status: ASSIGNED → RESOLVED
Closed: 3 months 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.