Closed Bug 1090157 Opened 10 years ago Closed 10 years ago

History.remove should block async shutdown

Categories

(Toolkit :: Places, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: Yoric, Assigned: Yoric)

References

Details

Attachments

(1 file, 1 obsolete file)

      No description provided.
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.
Attached patch Implementation (obsolete) — Splinter Review
Attachment #8512589 - Flags: review?(mak77)
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+
(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.
Attachment #8512589 - Attachment is obsolete: true
Attachment #8525310 - Flags: review+
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.