Add Nuwa idle detection hooks to IndexedDB threads

RESOLVED WONTFIX

Status

()

defect
RESOLVED WONTFIX
4 years ago
4 years ago

People

(Reporter: bent.mozilla, Assigned: bent.mozilla)

Tracking

Trunk
mozilla40
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.2+, firefox40 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Posted patch Patch for trunk, v1 (obsolete) — 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)
We will need a different patch for 2.2
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+
(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
(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.
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)
(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)
(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.
Blocks: 1151672
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+
We can't let 2.2 ship without fixing this there too.
blocking-b2g: --- → 2.2?
blocking-b2g: 2.2? → 2.2+
Assignee: nobody → bent.mozilla
Hi Ben,
What does it mean for comment 11? Is it the problem of treeherder?
Flags: needinfo?(bent.mozilla)
(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.
Posted patch Patch, v1.1Splinter Review
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)
Attachment #8602827 - Attachment description: changes.patch → Patch, v1.1
Blocks: 1162535
https://hg.mozilla.org/mozilla-central/rev/3f62597c7e66
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Resolution: FIXED → WONTFIX
You need to log in before you can comment on or make changes to this bug.