Closed
Bug 1329668
Opened 8 years ago
Closed 8 years ago
Crash in mozilla::dom::cache::CacheStorage::Delete
Categories
(Core :: DOM: Workers, defect)
Tracking
()
RESOLVED
FIXED
mozilla53
People
(Reporter: bkelly, Assigned: bkelly)
References
Details
(Keywords: crash)
Crash Data
Attachments
(1 file)
(deleted),
patch
|
baku
:
review+
gchang
:
approval-mozilla-aurora+
gchang
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
This bug was filed from the Socorro interface and is
report bp-53d9068a-3025-4bdc-9769-2d2422170109.
=============================================================
Fair number of crashes in the worker ScriptLoader at address 0x2c when using CacheStorage. Guessing this is a ScriptLoader problem and not a CacheStorage problem.
Assignee | ||
Comment 1•8 years ago
|
||
This is happening because we try to delete the Cache when we cancel, but we may not be fully initialized if the cancel is initiated here:
https://dxr.mozilla.org/mozilla-central/source/dom/workers/ScriptLoader.cpp#601
Assignee: nobody → bkelly
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•8 years ago
|
||
Andrea, this handles the case where we cancel worker script loading here:
https://dxr.mozilla.org/mozilla-central/source/dom/workers/ScriptLoader.cpp#601
Depending on when RunInternal() returns an error we may not have initialized the mCacheStorage yet. So we cannot unconditionally use it in cancellation.
Attachment #8825095 -
Flags: review?(amarchesini)
Assignee | ||
Comment 3•8 years ago
|
||
BTW, this accounts for 70% of the crashes related to dom::cache between Dec 1 and Jan 9. About 500 crashes.
Assignee | ||
Updated•8 years ago
|
status-firefox50:
--- → wontfix
status-firefox51:
--- → affected
status-firefox52:
--- → affected
status-firefox53:
--- → affected
status-firefox-esr45:
--- → wontfix
Updated•8 years ago
|
Attachment #8825095 -
Flags: review?(amarchesini) → review+
Pushed by bkelly@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8bb63d1c87d7
Handle partial initialized when ScriptLoader is canceled. r=baku
Assignee | ||
Comment 5•8 years ago
|
||
Comment on attachment 8825095 [details] [diff] [review]
Handle partial initialized when ScriptLoader is canceled. r=baku
Approval Request Comment
[Feature/Bug causing the regression]: Service workers
[User impact if declined]: About 200 crashes per week in release channel.
[Is this code covered by automated tests?]: Yes, but it does not trigger this corner case.
[Has the fix been verified in Nightly?]: Its very racy, so we have not been able to reproduce manually.
[Needs manual test from QE? If yes, steps to reproduce]: None
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Its simply an extra nullptr check. Its very low risk.
[String changes made/needed]: None
Attachment #8825095 -
Flags: approval-mozilla-beta?
Attachment #8825095 -
Flags: approval-mozilla-aurora?
Comment 6•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Comment 7•8 years ago
|
||
Comment on attachment 8825095 [details] [diff] [review]
Handle partial initialized when ScriptLoader is canceled. r=baku
Fix a crash. Beta51+ & Aurora52+. Should be in 51 beta 14.
Attachment #8825095 -
Flags: approval-mozilla-beta?
Attachment #8825095 -
Flags: approval-mozilla-beta+
Attachment #8825095 -
Flags: approval-mozilla-aurora?
Attachment #8825095 -
Flags: approval-mozilla-aurora+
Assignee | ||
Comment 8•8 years ago
|
||
Assignee | ||
Comment 9•8 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•