Closed
Bug 1090157
Opened 10 years ago
Closed 10 years ago
History.remove should block async shutdown
Categories
(Toolkit :: Places, defect)
Toolkit
Places
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: Yoric, Assigned: Yoric)
References
Details
Attachments
(1 file, 1 obsolete file)
5.83 KB,
patch
|
Yoric
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•10 years ago
|
||
Since we are going to use History.* to clean some privacy-related stuff, possibly during shutdown, we need to make sure that this blocks shutdown.
Assignee | ||
Comment 2•10 years ago
|
||
Attachment #8512589 -
Flags: review?(mak77)
Comment 3•10 years ago
|
||
Comment on attachment 8512589 [details] [diff] [review] Implementation Review of attachment 8512589 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/components/places/History.jsm @@ +89,2 @@ > */ > +XPCOMUtils.defineLazyGetter(this, "operations", () => can we find a more self-documenting name? something like closeConnectionBarrier could be nicer... @@ +101,5 @@ > try { > Sqlite.shutdown.addBlocker( > "Places History.jsm: Closing database wrapper", > Task.async(function*() { > + yield operations.wait(); we'll have to solve the fact this connection should close when the wrapped connection does, that will make this a little bit less clear. I guess we might not care, and just say we want to complete removals before the connection goes away, then the code should basically be the same, apart this won't be a shutdown blocker. though, at that point the underlying connection might not respect this wait request... the only thing it respects so far is the fact any async query issued before closing the connection will be executed. I guess we'll leave this problem to bug 1091851... @@ +129,5 @@ > +let gIsClosed = false; > + > +function ensureModuleIsOpen() { > + if (gIsClosed) { > + throw new Error("History.jsm is closed"); s/is closed/has been shutdown/
Attachment #8512589 -
Flags: review?(mak77) → review+
Assignee | ||
Comment 4•10 years ago
|
||
(In reply to Marco Bonardo [::mak] (needinfo? me) from comment #3) > I guess we'll leave this problem to bug 1091851... Yes, I think that's another issue that needs to be solved.
Assignee | ||
Comment 5•10 years ago
|
||
Attachment #8512589 -
Attachment is obsolete: true
Attachment #8525310 -
Flags: review+
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 6•10 years ago
|
||
Try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=71c6110695ea
Comment 7•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/b055078c05a1
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/b055078c05a1
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla36
You need to log in
before you can comment on or make changes to this bug.
Description
•