i don't know if this should be a storage bug since to activate it i have to use nsPlacesDBFlush, so i suppose that's involved in the leak. This test activates the leak, changing the param assignment to bintInt64Param deactivate the leak.

ps: final reset() should be finalize(), hwv that does not change results.

Where did you place this test case to run it?

toolkit/places/tests/unit

ops i lost a /components/

Found it with bent's help. Totally a places bug...

Comment on attachment 353890 [details] [diff] [review] v1.0 [Checkin: Comment 10 & 17] talked over irc, r=me

OK - here's what was happening: Since we had the getter defined on the prototype, the database would be held onto as long as xpconnect was alive (assuming it was accessed at least once). The problem showed up when we used the mozStorageStatementJSHelper, which ends up making mozStorageService hold not XPConnect until it goes away. This ended up creating a cycle though: XPConnect held a reference to the places database connection The places database connection held a reference to mozStorageService mozStorageService held a reference to XPConnect Fun times...

Comment on attachment 353890 [details] [diff] [review] v1.0 [Checkin: Comment 10 & 17] http://hg.mozilla.org/mozilla-central/rev/62902ec9424a

(In reply to comment #10) [Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.2a1pre) Gecko/20081220 Minefield/3.2a1pre] (home, optim default) (W2Ksp4) (http://hg.mozilla.org/mozilla-central/rev/e45938aee309) Iirc, Shawn said this patch fixed the testcase, but not the actual tests: *Bug 470455 has a patch to fix its code... :-) *Bug 469062 has been worked around for the time being. :-| *Bug is still there ! :-(

(In reply to comment #11) > Iirc, Shawn said this patch fixed the testcase, but not the actual tests: So, I'm going to keep this bug closed, since we've fixed this leak (the one that the testcase exposed). Ideally, we'll get a new bug with another reduced test case, but I just verified that the testcase attached to this bug has been fixed (I had previously reduced it further, and hadn't verified that the original test case didn't leak). This bug is closed as it stands, and I guess we'll try to tackle the other ones one at a time. *sigh*

(In reply to comment #11) > *Bug is still there ! :-( Bug 465952 ! (In reply to comment #12) Agreed... Did this fix either of the following bugs ? *Bug 469062 without its workaround. *Bug 452899 with its currently attached patch.

(In reply to comment #13) > Did this fix either of the following bugs ? > *Bug 469062 without its workaround. yes > *Bug 452899 with its currently attached patch. yes

(In reply to comment #14) > > *Bug 452899 with its currently attached patch. > > yes ops sorry. i thought this was the embed bug, nevermind me.

however embed test is good to check this worked, because the actual patch in bug 470455 would not be enough to fix it completely without this.