In reviewing another patch I've just spotted this. If you do any sort of LDAP query via autocomplete or the address book quick search, then the connection stays alive until shutdown. This is per-compose window or per a quick search. It is a regression from bug 343332 as that one registered nsLDAPConnection with the observer service as a strong reference, but that reference doesn't get removed until shutdown, hence connections are stuck open, even though we don't re-use them at the moment. As nsLDAPConnection already implements weak references, then we can just change the observer service hold of the connection to a weak reference, and add a remove observal call to tidy up. This stops us keeping nsLDAPConnection in memory, so it gets freed at an appropriate time, e.g. a garbage collection.

Comment on attachment 543750 [details] [diff] [review] The fix I'm not sure that you're allowed to pass yourself to RemoveObserver in your own destructor...

Why not? How else am I meant to say, I'm going away now so there's no point in keeping a reference to me... The other obvious place would be the Close() function, but that still gets called anyway. Also see: http://mxr.mozilla.org/comm-central/source/mozilla/modules/libpref/src/nsPrefBranch.cpp#118

Comment on attachment 543750 [details] [diff] [review] The fix I can't run xpcshell tests at the moment, but it's hard to see how this could affect any of our tests. Testing by hand seems to work fine; the connections are destroyed at gc time.

Well unfortunately we don't have xpcshell-tests covering LDAP connections at the moment anyway.

(In reply to comment #4) > Well unfortunately we don't have xpcshell-tests covering LDAP connections at > the moment anyway. Do we have a fake LDAP server ?

Checked in: http://hg.mozilla.org/comm-central/rev/79ea291ad0c0 No we don't have a fake server, otherwise we would have something covering LDAP connections ;-)

Comment on attachment 543750 [details] [diff] [review] The fix Also want this for TB 6 as it is a regression in TB 5 that we should fix asap.

Checked into aurora: http://hg.mozilla.org/releases/comm-aurora/rev/0d7f09a3495b

(In reply to comment #2) > Why not? How else am I meant to say, I'm going away now so there's no point > in keeping a reference to me... But you've *already* gone by the time your destructor runs... > Also see: > http://mxr.mozilla.org/comm-central/source/mozilla/modules/libpref/src/ > nsPrefBranch.cpp#118 Interesting... I'll ask about that.