We seem to be creating cycles with XPConnect and mozStorageService. We should just release the reference we hold at xpcom-shutdown in storage. Locally, this makes browser chrome leaks go to zero. yay!

Comment on attachment 357257 [details] [diff] [review] v1.0 >+ "Did not properlyshut down mozStorageService!"); Nit: missing space.

Comment on attachment 357257 [details] [diff] [review] v1.0 As per our discussion let's just free the cached xpconnect on xpcom-shutdown and leave the sqlite shutdown order alone. Also fix the xpconnect() getter to handle this case so you can't leak xpconnect if one of your wrappers is still alive.

Drop the leak threshold to 0 as well if the follow up patch works as well as the existing one does.

Like so? This clears the leaks from browser-chrome on my machine, haven't tested on other platforms though. Only one thought is whether Init really needs to return a failure if we fail to get and use the observer service, the only consequence is a shutdown leak. But I guess if that happens things are pretty screwed anyway.

Comment on attachment 357969 [details] [diff] [review] simpler patch The problem with this patch is that it's perfectly legal for someone to try and use xpconnect during xpcom-shutdown. bent and I discussed how to get around this yesterday though, so I'll post a new patch.

And leaks are at zero locally.

Oh, drat, I wasn't CC'd.

Comment on attachment 357988 [details] [diff] [review] v3.0 >+already_AddRefed<nsIXPConnect> > mozStorageService::XPConnect() I think you could make this much simpler by using nsCOMPtr's forget method (this case is what it was esigned for): nsCOMPtr<nsIXPConnect> retval; if (sXPConnect) { // normal retval = sXPConnect; } else { // shutdown retval = do_GetService(nsIXPConnect::GetCID()); } return retval.forget(); >+ // We chache XPConnect for our language helpers. >+ (void)CallGetService(nsIXPConnect::GetCID(), &sXPConnect); You don't check the return here so if that call failed you'd crash on shutdown in Shutdown's NS_RELEASE... I think you should reverse the order of the calls in Init to do the observer service stuff first, then cache xpconnect, and use NS_IF_RELEASE in shutdown. That way you're guaranteed not to leak or crash on shutdown. Mossop is right, though, that if any of that fails we're screwed anyway. Meh.

Oh, and 'chache' is not spelled correctly in that comment i pasted above :)

Comment on attachment 358020 [details] [diff] [review] v3.1 r=me.

Leak threshold was different for 1.9.1, so I had to manually fix that, but no other changes were needed: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/f7c35e9f75cb

V.Fixed, per bug 465952.

I had to commit http://hg.mozilla.org/mozilla-central/rev/64ff884c9127 to get my build to start properly.

(In reply to comment #14) > Leak threshold was different for 1.9.1, so I had to manually fix that, but no > other changes were needed: But linux seems fine, so I suspect we are fine across the board with it.

My recent builds from 1.9.1 branch have no bookmarks in the menu or sidebar with f7c35e9f75cb applied. I have to back it out or apply 64ff884c9127 to restore bookmarks. Can we get this fix checked into the branch?