1.0 to 2.0 upgrade kills off the process when starting a debug build

RESOLVED FIXED in Firefox 55

Status

()

Core
DOM: Quota Manager
--
blocker
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: bz, Assigned: janv)

Tracking

Trunk
mozilla55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(1 attachment)

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)
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?
Is this just a copy/paste from QuotaManager::GetDirectoryMetadata2 error?  aTimestamp is a _pointer_ there, not a reference, so asserting it makes some sense...
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.
> 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

a year ago
Oh sorry, sure, drop the assert. I have an appointment right now, so feel free to fix it yourself.
Flags: needinfo?(jvarga)
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

a year ago
Created attachment 8844552 [details] [diff] [review]
patch
Assignee: nobody → jvarga
Status: NEW → ASSIGNED
Attachment #8844552 - Flags: review+
(Assignee)

Updated

a year ago
Keywords: checkin-needed

Comment 8

a year ago
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
(Assignee)

Updated

a year ago
See Also: → bug 1246615
(Assignee)

Updated

a year ago
See Also: bug 1246615

Comment 9

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c6f5538e50a8
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.