Last Comment Bug 711554 - finalize statements and close connection
: finalize statements and close connection
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Search (show other bugs)
: unspecified
: x86_64 Linux
: -- normal (vote)
: Firefox 12
Assigned To: Rafael Ávila de Espíndola (:espindola) (not reading bugmail)
:
: Florian Quèze [:florian] [:flo]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-12-16 11:26 PST by Rafael Ávila de Espíndola (:espindola) (not reading bugmail)
Modified: 2011-12-28 11:14 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
finalize statements and close connection (1.64 KB, patch)
2011-12-16 11:30 PST, Rafael Ávila de Espíndola (:espindola) (not reading bugmail)
mak77: review-
Details | Diff | Splinter Review
finalize statements and close connection (1.88 KB, patch)
2011-12-20 09:57 PST, Rafael Ávila de Espíndola (:espindola) (not reading bugmail)
mak77: review-
Details | Diff | Splinter Review
finalize statements and close connection (1.81 KB, patch)
2011-12-21 10:09 PST, Rafael Ávila de Espíndola (:espindola) (not reading bugmail)
no flags Details | Diff | Splinter Review
finalize statements and close connection (1.81 KB, patch)
2011-12-21 11:17 PST, Rafael Ávila de Espíndola (:espindola) (not reading bugmail)
mak77: review+
Details | Diff | Splinter Review

Description Rafael Ávila de Espíndola (:espindola) (not reading bugmail) 2011-12-16 11:26:55 PST

    
Comment 1 Rafael Ávila de Espíndola (:espindola) (not reading bugmail) 2011-12-16 11:30:00 PST
Created attachment 582336 [details] [diff] [review]
finalize statements and close connection

https://tbpl.mozilla.org/?tree=Try&rev=7e534723652b
Comment 2 Marco Bonardo [::mak] 2011-12-16 13:12:12 PST
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.
Comment 3 Rafael Ávila de Espíndola (:espindola) (not reading bugmail) 2011-12-20 09:57:47 PST
Created attachment 583207 [details] [diff] [review]
finalize statements and close connection

https://tbpl.mozilla.org/?tree=Try&rev=fbf33829a451
Comment 4 Marco Bonardo [::mak] 2011-12-21 04:55:20 PST
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.
Comment 5 Rafael Ávila de Espíndola (:espindola) (not reading bugmail) 2011-12-21 10:09:00 PST
Created attachment 583544 [details] [diff] [review]
finalize statements and close connection

https://tbpl.mozilla.org/?tree=Try&rev=98457fae3a7d
Comment 6 Rafael Ávila de Espíndola (:espindola) (not reading bugmail) 2011-12-21 11:17:30 PST
Created attachment 583561 [details] [diff] [review]
finalize statements and close connection

https://tbpl.mozilla.org/?tree=Try&rev=1f8b2d452ff8
Comment 7 Marco Bonardo [::mak] 2011-12-28 05:18:37 PST
Comment on attachment 583561 [details] [diff] [review]
finalize statements and close connection

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

looks good
Comment 8 Rafael Ávila de Espíndola (:espindola) (not reading bugmail) 2011-12-28 05:40:49 PST
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=c0b49e61e2fd
Comment 9 Matt Brubeck (:mbrubeck) 2011-12-28 11:14:59 PST
https://hg.mozilla.org/mozilla-central/rev/c0b49e61e2fd

Note You need to log in before you can comment on or make changes to this bug.