Closed
Bug 1156611
Opened 10 years ago
Closed 10 years ago
Add Nuwa idle detection hooks to IndexedDB threads
Categories
(Core :: Storage: IndexedDB, defect)
Core
Storage: IndexedDB
Tracking
()
Tracking | Status | |
---|---|---|
firefox40 | --- | fixed |
People
(Reporter: bent.mozilla, Assigned: bent.mozilla)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
khuey
:
review+
|
Details | Diff | Splinter Review |
+++ This bug was initially created as a clone of Bug #1156484 +++
I think this properly sets idle/working state for Nuwa's purposes. Idle threads aren't enough - we have to wait for transactions to finish.
Patrick, does this look about right?
Attachment #8595128 -
Flags: review?(kk1fff)
Attachment #8595128 -
Flags: review?(khuey)
Assignee | ||
Comment 1•10 years ago
|
||
We will need a different patch for 2.2
Comment 2•10 years ago
|
||
Comment on attachment 8595128 [details] [diff] [review]
Patch for trunk, v1
Review of attachment 8595128 [details] [diff] [review]:
-----------------------------------------------------------------
Thanks, Ben!
Attachment #8595128 -
Flags: review?(kk1fff) → review+
Comment 3•10 years ago
|
||
(In reply to Ben Turner [:bent] (use the needinfo flag!) from comment #1)
> We will need a different patch for 2.2
Actually I've inserted some code to report the thread status in TranactionThreadPool.cpp[1]. They are still in 2.2 branch. If they were inserted at correct place, 2.2 should be free from being affected.
[1] https://github.com/mozilla/gecko-dev/blob/b2g37_v2_2/dom/indexedDB/TransactionThreadPool.cpp#L880
Assignee | ||
Comment 4•10 years ago
|
||
(In reply to Patrick Wang (Chih-Kai Wang) (:kk1fff) from comment #3)
> Actually I've inserted some code to report the thread status in
> TranactionThreadPool.cpp[1]. They are still in 2.2 branch. If they were
> inserted at correct place, 2.2 should be free from being affected.
The hooks you've added in 2.2 only detect when a transaction thread becomes idle. As I have said in bug 1156484, idle thread detection is not sufficient here (and this is likely the case with other APIs). Nuwa could freeze while an IndexedDB transaction is in progress but temporarily idle. If that happens all further attempts to access that database will hang forever.
Comment 5•10 years ago
|
||
Is this something I need to do for the Cache API as well? Or is IDB doing something special which requires the custom hooks?
Flags: needinfo?(bent.mozilla)
Assignee | ||
Comment 6•10 years ago
|
||
(In reply to Ben Kelly [:bkelly] from comment #5)
> Is this something I need to do for the Cache API as well? Or is IDB doing
> something special which requires the custom hooks?
The Cache API is lock-like, right? I imagine you'll need to do something special too.
Flags: needinfo?(bent.mozilla)
Comment 7•10 years ago
|
||
(In reply to Ben Turner [:bent] (use the needinfo flag!) from comment #6)
> The Cache API is lock-like, right? I imagine you'll need to do something
> special too.
I spoke with Ben and I don't think the situation is as bad for Cache. If NUWA freezes with a Cache object open and another process does caches.delete() for that Cache, then the backing storage could be leaked until next start up. It shouldn't block other processes from working, though.
Comment on attachment 8595128 [details] [diff] [review]
Patch for trunk, v1
Review of attachment 8595128 [details] [diff] [review]:
-----------------------------------------------------------------
ugh lambdas
Attachment #8595128 -
Flags: review?(khuey) → review+
Assignee | ||
Comment 9•10 years ago
|
||
We can't let 2.2 ship without fixing this there too.
blocking-b2g: --- → 2.2?
Assignee | ||
Comment 10•10 years ago
|
||
Keywords: checkin-needed
Assignee | ||
Comment 11•10 years ago
|
||
Keywords: checkin-needed
Updated•10 years ago
|
blocking-b2g: 2.2? → 2.2+
Updated•10 years ago
|
Assignee: nobody → bent.mozilla
Comment 12•10 years ago
|
||
Hi Ben,
What does it mean for comment 11? Is it the problem of treeherder?
Flags: needinfo?(bent.mozilla)
Assignee | ||
Comment 13•10 years ago
|
||
(In reply to Josh Cheng [:josh] from comment #12)
> Hi Ben,
> What does it mean for comment 11? Is it the problem of treeherder?
No, my patch had a problem.
Assignee | ||
Comment 14•10 years ago
|
||
Same patch, just added an additional state twiddle to ConnectionPool::CloseDatabase.
Attachment #8595128 -
Attachment is obsolete: true
Flags: needinfo?(bent.mozilla)
Attachment #8602827 -
Flags: review?(khuey)
Assignee | ||
Updated•10 years ago
|
Attachment #8602827 -
Attachment description: changes.patch → Patch, v1.1
Attachment #8602827 -
Flags: review?(khuey) → review+
Assignee | ||
Comment 15•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Comment 17•10 years ago
|
||
Backout: https://hg.mozilla.org/integration/b2g-inbound/rev/78b19f9f8da8
due to backout of bug 970307.
Assignee | ||
Updated•10 years ago
|
Resolution: FIXED → WONTFIX
Comment 18•10 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•