finalize statements and close connection

RESOLVED FIXED in Firefox 12

Status

()

Firefox
Search
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: espindola, Assigned: espindola)

Tracking

unspecified
Firefox 12
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

Comment hidden (empty)
Created attachment 582336 [details] [diff] [review]
finalize statements and close connection

https://tbpl.mozilla.org/?tree=Try&rev=7e534723652b
Assignee: nobody → respindola
Status: NEW → ASSIGNED
Attachment #582336 - Flags: review?(mak77)
Comment on attachment 582336 [details] [diff] [review]
finalize statements and close connection

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

::: toolkit/components/search/nsSearchService.js
@@ +3622,5 @@
> +  closeDB: function epsCloseDB() {
> +    this.mInsertData.finalize();
> +    this.mDeleteData.finalize();
> +    this.mGetData.finalize();
> +    this.mDB.close();

all of these are getters, you may be initing them there ideally. Before finalizing or closing you should ensure if they are still getters or not with Object.getOwnPropertyDescriptor(this, "property").value !== undefined
it's probably simpler to put the properties names in a temp array and loop it.
Attachment #582336 - Flags: review?(mak77) → review-
Created attachment 583207 [details] [diff] [review]
finalize statements and close connection

https://tbpl.mozilla.org/?tree=Try&rev=fbf33829a451
Attachment #582336 - Attachment is obsolete: true
Attachment #583207 - Flags: review?(mak77)
Comment on attachment 583207 [details] [diff] [review]
finalize statements and close connection

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

::: toolkit/components/search/nsSearchService.js
@@ +3624,5 @@
> +      this.mInsertData.finalize();
> +    if (this.getOwnPropertyDescriptor(this, "mDeleteData").value !== undefined)
> +      this.mDeleteData.finalize();
> +    if (this.getOwnPropertyDescriptor(this, "mGetData").value !== undefined)
> +      this.mGetData.finalize();

something like this may work

["mInsertData", "mDeleteData", "mGetData"].forEach(function(aStmt) {
  if (Object.getOwnPropertyDescriptor(this, aStmt).value !== undefined)
    this[aStmt].finalize();
}, this);

@@ +3625,5 @@
> +    if (this.getOwnPropertyDescriptor(this, "mDeleteData").value !== undefined)
> +      this.mDeleteData.finalize();
> +    if (this.getOwnPropertyDescriptor(this, "mGetData").value !== undefined)
> +      this.mGetData.finalize();
> +    this.mDB.close();

also mDB is a lazy getter, it should be handled similarly as the statements.
Attachment #583207 - Flags: review?(mak77) → review-
Created attachment 583544 [details] [diff] [review]
finalize statements and close connection

https://tbpl.mozilla.org/?tree=Try&rev=98457fae3a7d
Attachment #583207 - Attachment is obsolete: true
Attachment #583544 - Flags: review?(mak77)
Created attachment 583561 [details] [diff] [review]
finalize statements and close connection

https://tbpl.mozilla.org/?tree=Try&rev=1f8b2d452ff8
Attachment #583544 - Attachment is obsolete: true
Attachment #583544 - Flags: review?(mak77)
Attachment #583561 - Flags: review?(mak77)
Comment on attachment 583561 [details] [diff] [review]
finalize statements and close connection

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

looks good
Attachment #583561 - Flags: review?(mak77) → review+
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=c0b49e61e2fd
https://hg.mozilla.org/mozilla-central/rev/c0b49e61e2fd
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 12
You need to log in before you can comment on or make changes to this bug.