Closed
Bug 1345120
Opened 8 years ago
Closed 8 years ago
1.0 to 2.0 upgrade kills off the process when starting a debug build
Categories
(Core :: Storage: Quota Manager, defect)
Core
Storage: Quota Manager
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: bzbarsky, Assigned: janv)
References
Details
Attachments
(1 file)
1.10 KB,
patch
|
janv
:
review+
|
Details | Diff | Splinter Review |
Whenever I try to start my debug build, I get:
Assertion failure: aTimestamp, at /Users/bzbarsky/mozilla/layout-work/mozilla/dom/quota/ActorsParent.cpp:6967
with this stack:
frame #0: 0x0000000105c3c0b8 XUL`mozilla::dom::quota::(anonymous namespace)::StorageDirectoryHelper::GetDirectoryMetadata2(this=0x00000001949efce0, aDirectory=0x0000000194bab600, aTimestamp=0x0000700004774da8, aSuffix=0x0000700004774d68, aGroup=0x0000700004774d98, aOrigin=0x0000700004774d88, aIsApp=0x0000700004774d80) + 216 at ActorsParent.cpp:6967
* frame #1: 0x0000000105c12342 XUL`mozilla::dom::quota::(anonymous namespace)::UpgradeStorageFrom1_0To2_0Helper::DoUpgrade(this=0x00000001949efce0) + 2354 at ActorsParent.cpp:8161
frame #2: 0x0000000105c116c4 XUL`mozilla::dom::quota::QuotaManager::UpgradeStorageFrom1_0To2_0(this=0x0000000194360b20, aConnection=0x0000000194ac64c0) + 692 at ActorsParent.cpp:4400
frame #3: 0x0000000105c132fc XUL`mozilla::dom::quota::QuotaManager::EnsureStorageIsInitialized(this=0x0000000194360b20) + 3180 at ActorsParent.cpp:4581
which is not terribly helpful: it means I can't debug anything. Looks to me like a regression from bug
Flags: needinfo?(jvarga)
Reporter | ||
Comment 1•8 years ago
|
||
Why is this assert even there? StorageDirectoryHelper::GetDirectoryMetadata2 never reads aTimestamp; it only writes to it. Why does it care what the value is on entry to the function?
Reporter | ||
Comment 2•8 years ago
|
||
Is this just a copy/paste from QuotaManager::GetDirectoryMetadata2 error? aTimestamp is a _pointer_ there, not a reference, so asserting it makes some sense...
Comment 3•8 years ago
|
||
I missed the signature change colliding with the assert in review. re: IRC comments, there is test coverage for the upgrade logic, but the issue is of course that the int64_t timestamp being passed in is allocated on the stack and its value is undefined.
Pre-emptive r=asuth for :janv for dropping the assert.
Reporter | ||
Comment 4•8 years ago
|
||
> but the issue is of course that the int64_t timestamp being passed in is allocated on the stack and its value
> is undefined.
Only if StorageDirectoryHelper::GetDirectoryMetadata errored out, right? I can't check whether it did that, because I disabled the assert locally and now my profile has gotten migrated. :(
Assignee | ||
Comment 5•8 years ago
|
||
Oh sorry, sure, drop the assert. I have an appointment right now, so feel free to fix it yourself.
Flags: needinfo?(jvarga)
Comment 6•8 years ago
|
||
Right you are. Either you had a ".metadata-v2" file in an origin directory but not a ".metadata" and the stack value was initialized to zero, or you had a ".metadata" file with a zero timestamp. The latter might have been possible via a previous upgrade path; the stack of patches cleaned up how timestamp is initialized so I don't think is possible now.
Assignee | ||
Comment 7•8 years ago
|
||
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c6f5538e50a8
1.0 to 2.0 upgrade kills off the process when starting a debug build. r=asuth
Keywords: checkin-needed
Comment 9•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in
before you can comment on or make changes to this bug.
Description
•