Closed Bug 1110585 Opened 5 years ago Closed 5 years ago

Inconsistent timestamp in storage metadata file

Categories

(Core :: DOM: IndexedDB, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla37
Tracking Status
firefox36 --- fixed
firefox37 --- fixed

People

(Reporter: hectorz, Assigned: janv)

References

Details

Attachments

(1 file)

Timestamp in storage metadata comes from two sources: PR_Now for new directory, GetLastModifiedTime for existing directory. But value from PR_Now is in microsecond while that from GetLastModifiedTime is in millisecond.
During B2G startup:
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 1781.1857]
0xb4f09a32 in mozilla::dom::quota::QuotaManager::EnsureOriginIsInitialized (this=this@entry=0xb6a98980, aPersistenceType=aPersistenceType@entry=mozilla::dom::quota::PERSISTENCE_TYPE_PERSISTENT, aGroup=..., aOrigin=..., 
    aIsApp=aIsApp@entry=false, aHasUnlimStoragePerm=aHasUnlimStoragePerm@entry=false, aDirectory=0xa9ec1990) at ../../../dom/quota/QuotaManager.cpp:2379
2379	      MOZ_ASSERT(timestamp <= PR_Now());
Attached patch patchSplinter Review
Assignee: nobody → Jan.Varga
Status: NEW → ASSIGNED
Attachment #8545185 - Flags: review?(bent.mozilla)
Comment on attachment 8545185 [details] [diff] [review]
patch

Review of attachment 8545185 [details] [diff] [review]:
-----------------------------------------------------------------

Can we do this the other way and convert PR_Now to milliseconds instead? I don't think we need microsecond resolution in quota stuff...
(In reply to ben turner [:bent] (use the needinfo? flag!) from comment #3)
> Comment on attachment 8545185 [details] [diff] [review]
> patch
> 
> Review of attachment 8545185 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Can we do this the other way and convert PR_Now to milliseconds instead? I
> don't think we need microsecond resolution in quota stuff...

Yeah, but we already have metadata files on the disk with origin access time stored in it and the value is in milliseconds.
(In reply to Jan Varga [:janv] from comment #4)
> (In reply to ben turner [:bent] (use the needinfo? flag!) from comment #3)
> > Comment on attachment 8545185 [details] [diff] [review]
> > patch
> > 
> > Review of attachment 8545185 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > Can we do this the other way and convert PR_Now to milliseconds instead? I
> > don't think we need microsecond resolution in quota stuff...
> 
> Yeah, but we already have metadata files on the disk with origin access time
> stored in it and the value is in milliseconds.

sorry, I meant *microseconds*
Comment on attachment 8545185 [details] [diff] [review]
patch

I'm ok with this but please assert that we don't overflow (e.g. (INT64_MAX / PR_USEC_PER_MSEC) > timestamp))
Attachment #8545185 - Flags: review?(bent.mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/20a7f674d668
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Comment on attachment 8545185 [details] [diff] [review]
patch

Approval Request Comment

[Feature/regressing bug #]: This is a followup fix for bug 1083927.

[User impact if declined]: I'm not aware of any big problems, but this fix is definitely needed for correctness

[Describe test coverage new/current, TBPL]: We have an assertion for this, bug 1110010 proves it.

[Risks and why]: The patch is very simple.

[String/UUID change made/needed]: No string/UUID changes.
Attachment #8545185 - Flags: approval-mozilla-aurora?
Attachment #8545185 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.