Closed Bug 711494 Opened 12 years ago Closed 12 years ago

close _dbConnection

Categories

(Toolkit :: General, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla11

People

(Reporter: espindola, Assigned: espindola)

Details

Attachments

(1 file)

      No description provided.
https://tbpl.mozilla.org/?tree=Try&rev=133092be6f37
Assignee: nobody → respindola
Status: NEW → ASSIGNED
Attachment #582287 - Flags: review?(mak77)
Comment on attachment 582287 [details] [diff] [review]
close _dbConnection

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

::: toolkit/components/contentprefs/nsContentPrefService.js
@@ +303,5 @@
>        this.__stmtUpdatePref.finalize();
>        this.__stmtUpdatePref = null;
>      }
>  
> +    this._dbConnection.asyncClose();

I don't see any createAsyncStatement call to this, should we rather use close()? do you see the asyncClose() assertion doing so or you suppose add-ons may use it asynchronously?
Component: Storage → General
QA Contact: storage → general
A variation using close is at
https://tbpl.mozilla.org/?tree=Try&rev=5dcfeabb3e75

Is it OK if the try is green? If not, is the original patch using asyncClose OK?
yes, it looks fine, r=me on that.
Using close failed:

************************************************************
* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [mozIStorageConnection.close]"  nsresult: "0x8000ffff (NS_ERROR_UNEXPECTED)"  location: "JS frame :: resource:///components/nsContentPrefService.js :: ContentPrefService__destroy :: line 307"  data: no]
************************************************************

I pushed the asyncClose version

https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=6e57fcb02e68
Using close failed:

************************************************************
* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [mozIStorageConnection.close]"  nsresult: "0x8000ffff (NS_ERROR_UNEXPECTED)"  location: "JS frame :: resource:///components/nsContentPrefService.js :: ContentPrefService__destroy :: line 307"  data: no]
************************************************************

I pushed the asyncClose version

https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=6e57fcb02e68
ah-ha, I was not looking for the right magic word, here we are:
http://mxr.mozilla.org/mozilla-central/source/toolkit/components/contentprefs/nsContentPrefService.js#1457

interesting that this component uses the same statement synchronously and asynchronously, it's a good recipe for thread contention
Attachment #582287 - Flags: review?(mak77) → review+
(In reply to Marco Bonardo [:mak] from comment #7)
> ah-ha, I was not looking for the right magic word, here we are:
> http://mxr.mozilla.org/mozilla-central/source/toolkit/components/
> contentprefs/nsContentPrefService.js#1457
> 
> interesting that this component uses the same statement synchronously and
> asynchronously, it's a good recipe for thread contention

Should I open a bug about it? More than happy to fix it since i am touching everything that open a sqllite connection....
I think adding a comment in bug 699859 may be enough, this component Storage part has to be mostly rewritten.
https://hg.mozilla.org/mozilla-central/rev/6e57fcb02e68
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
You need to log in before you can comment on or make changes to this bug.