711554 - finalize statements and close connection

Status: RESOLVED FIXED

(Firefox :: Search, defect)

Comment 1

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

Attachment: finalize statements and close connection

https://tbpl.mozilla.org/?tree=Try&rev=7e534723652b

Comment 2

[Marco Bonardo [:mak]]

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)]

Attachment: finalize statements and close connection

https://tbpl.mozilla.org/?tree=Try&rev=fbf33829a451

Comment 4

[Marco Bonardo [:mak]]

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)]

Attachment: finalize statements and close connection

https://tbpl.mozilla.org/?tree=Try&rev=98457fae3a7d

Comment 6

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

Attachment: finalize statements and close connection

https://tbpl.mozilla.org/?tree=Try&rev=1f8b2d452ff8

Comment 7

[Marco Bonardo [:mak]]

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)]

https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=c0b49e61e2fd

Comment 9

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/mozilla-central/rev/c0b49e61e2fd