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

Status: RESOLVED FIXED

(Core :: Storage: Quota Manager, defect)

Description

[Boris Zbarsky [:bzbarsky]]

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

Comment 1

[Boris Zbarsky [:bzbarsky]]

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?

Comment 2

[Boris Zbarsky [:bzbarsky]]

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

[Andrew Sutherland [:asuth] (he/him)]

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.

Comment 4

[Boris Zbarsky [:bzbarsky]]

> 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.  :(

Comment 5

[Jan Varga [:janv]]

Oh sorry, sure, drop the assert. I have an appointment right now, so feel free to fix it yourself.

Comment 6

[Andrew Sutherland [:asuth] (he/him)]

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.

Comment 7

[Jan Varga [:janv]]

Attachment: patch

Comment 8

[Pulsebot]

Pushed by [e-mail]:
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

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c6f5538e50a8