Error dialog on exit

[Tom Marshall]

System is current dapper as of this writing. Thunderbird is configured with two accounts:

Home: IMAP4/SSL to Gentoo dovecot 1.0b3 server.
Work: IMAP4/SSL to Ubuntu dovecot 1.0b3 server.

On exit, Thunderbird gives this dialog:

The operation can not be completed because of an internal failure. A secure network communication has not been cleaned up correctly.

[Nbaca (nbaca)]

Branch build 2003-06-19: WinXP
Branch build 2003-06-20: Linux RH 8
haven't tried mac yet.

Using the 6/17 build on WinXP, I don't see the internal failure error. Using the
6/19 build on WinXP I do see the error each time I perform an LDAP search and
then exit. Why am I seeing this now and not in earlier builds? Will users who
have LDAP/SSL enabled always see this error upon exit?

[Esther (esther)]

I do see this error with LDAP SSL =ON with both the 6-17 and 6-19 builds on
winxp and mac osx after searching and finding an LDAP match.

[Sspitzer (sspitzer)]

accepting, I see this too.

this error strings is defined in
mozilla\security\manager\ssl\resources\locale\en-US\pipnss.properties

ProfileSwitchSocketsStillActive=The operation can not be completed because of an
internal failure. A secure network communication has not been cleaned up correctly.

the place to start debugging is
http://lxr.mozilla.org/mozilla/source/security/manager/ssl/src/nsNSSComponent.cpp#1708.

since people aren't seeing this with https, or IMAP over SSL, or NNTP over SSL,
I bet we're just doing something wrong in the addressbook (or mozilla/directory)
code.

[Sspitzer (sspitzer)]

note to self, the code that alerts us to know we've left open SSL sockets was
added on 06 Jan 2003 14:23, by kaie.

"<email address hidden> b=177260 Fix known leaks in PSM, track blocking PSM UI,
track open SSL sockets r=javi sr=darin"

[Sspitzer (sspitzer)]

ok, doing some debugging, and finding a couple issues.

while the alert is unique to LDAP over SSL, we have the same problem with
non-SSL LDAP connections. on shutdown, they aren't closed cleanly either. (I'm
sure the LDAP server logs are showing this, and this isn't nice of us.)

on shutdown (and probably profile switching), the observers of the
"profile-change-teardown" notification are notified.

the nss component is one one of those observers, and if it gets shutdown with
any active and open SSL sockets, it will alert.

why are there open LDAP connections?

it appears that we are leaking LDAP connections, my guess is that we are leaking
nsAbLDAPDirectory objects, which owns the connection.

we also might want to add code (not sure where yet, to nsLDAPConnection, or
nsLDAPConnectionLoop, for the open connections, or to nsMsgComposeService, for
the cache compose window, or to the addrbook code for LDAP queries), to clean up
any open connections on the notification that's common to profile switching and
shutdown.

I'll continue to investigate.

[Sspitzer (sspitzer)]

i'm working on sorting out the ownership model (for the addressbook, between
nsAbView, nsAbLDAPDirectory, nsAbQueryLDAPMessageListener and nsLDAPConnection).

[Hajek-systinet (hajek-systinet)]

Mozillas,

what is the status of this ticket? SSL LDAP search is working only once, than
I have to restart mozilla and see the alert window saying "The operation
cannot be completed because of an internal failure. A secure network
communication has not been cleaned up correctly."

Thanks,
David

[Mark-tranchant (mark-tranchant)]

This also affects Thunderbird 0.4 on Win2k.

Unlike David Hajek, I can perform multiple searches (to an OpenLDAP 2.1.22
server on Linux), but get the error message when I close Thunderbird down.

I have noticed that each search query opens a new TCP connection to the server,
so after three connections, the server has:

tcp 0 0 mauve.home:ldaps mauve.home:1034 ESTABLISHED
tcp 0 0 mauve.home:1034 mauve.home:ldaps ESTABLISHED
tcp 0 0 mauve.home:1035 mauve.home:ldaps ESTABLISHED
tcp 0 0 mauve.home:1036 mauve.home:ldaps ESTABLISHED
tcp 0 0 mauve.home:ldaps mauve.home:1035 ESTABLISHED
tcp 0 0 mauve.home:ldaps mauve.home:1036 ESTABLISHED

These don't seem to get cleared in any reasonable time - perhaps this is the
cause of the problem?

[Mark-tranchant (mark-tranchant)]

As some developers may not have access to an LDAP/SSL setup, I have created a
test directory.

Hostname: ldap.billericaybaptist.net
Port: 636
BaseDN: o=Mozilla Test,c=GB
BindDN: cn=John Smith,o=Mozilla Test,c=GB
Password: password

You'll need to accept the root certificate I've created. There are three entries
in the database.

You can then see the open connections at:

http://tranchant.plus.com/ldaps

which is the output of "netstat -al | grep ldaps".

[Mcs-pearlcrescent (mcs-pearlcrescent)]

Is this a duplicate of bug 176760? It sounds like it....

[Bmo (bmo)]

as noted in bug 176760 comment 5, the bugs are closely related. however, the
developer (sspitzer) asks that both bugs be left open.

[Ronny-adsetts (ronny-adsetts)]

*** Bug 228859 has been marked as a duplicate of this bug. ***

[Bienvenu (bienvenu)]

I'm working on this. The connections aren't closing because of memory leaks.

[Bienvenu (bienvenu)]

The core problem is that the ldap connection is only freed up in the destructor
for nsLDAPConnection(). So, any memory leak or failure to free up the
nsLDAPConnection will cause the connection to persist.

There are a couple causes of failure to free up nsLDAPConnections. The first,
recently introduced, is some memory leaks when using nsSupportsHashtable::Clone
- the destructors of this were recently made non-virtual, which breaks the
ref-counting of nsLDAPOperations, which causes leaks. See bug 229875

The second core cause is ownership cycles. The critical cycle that I think needs
to be broken is between nsLDAPOperation and its message listener, closure, and
ldap connection references. I'm trying some code to break this cycle when we
remove an operation from the pending queue in nsLDAPConnection, because the
operation has finished. The alternative is weak ref pointers, or simple
pointers, because we know the connection object strictly owns the ldap
operations. But I don't know about if the message listener or closure strong
references are required to keep some objects alive...

An additional problem is that we recycle compose windows, which prevents us from
releasing auto complete sessions, which holds onto an ldap connection. The auto
complete session is held onto by a global js var. I believe we need to null this
when the compose window is closed, even if it is recycled, and recreate it when
opened.

[Bienvenu (bienvenu)]

Created an attachment (id=141248)
proposed fix

[Bienvenu (bienvenu)]

(From update of attachment 141248)
Clearing the nsLDAPOperation only turns out to be needed when doing quick
search in the AB, not when doing auto complete...

[Dmose (dmose)]

(From update of attachment 141248)
A few tweaks needed as discussed over IM.

[Bienvenu (bienvenu)]

Created an attachment (id=141830)
fix addressing Dan's comments

the one thing I'm not sure is kosher is doing nsCOMPtr <nsLDAPOperation>
operation - it compiles and works, and there's some mailnews code that's always
been that way, but it doesn't seem kosher to me. It's convenient, however, to
handle the ref-counting issue - otherwise, we'd need to release the operation
pointer explicitly.

[Dmose (dmose)]

(From update of attachment 141830)
>+#ifdef PR_LOGGING
>+ if (rc != LDAP_SUCCESS) {
>+ PR_LOG(gLDAPLogModule, PR_LOG_WARNING,
>+ ("nsLDAPConnection::~nsLDAPConnection: %s\n",
>+ ldap_err2string(rc)));
>+ }
>+#endif

The above error message should now reference "nsLDAPConnection:Close" instead
of the destructor.

r=dmose with that change.

[Bienvenu (bienvenu)]

(From update of attachment 141830)
carrying forward mscott's sr.

[Bienvenu (bienvenu)]

fix checked in, with Dan's comment addressed.

[Bmo (bmo)]

*** Bug 176760 has been marked as a duplicate of this bug. ***

[Bmo (bmo)]

using the steps i provided in comment 0, this bug still occurs in the latest
nightly (20040407).

[Q0011 (q0011)]

Still buggy in TB 0.6+ (20041015)

[Jimd-gie (jimd-gie)]

Buggy in thunderbird 1.0

[Colin-harrison (colin-harrison)]

Still occurs on Thunderbird 1.0+ (20050714) from CVS.
Sometimes with a core dump on exit:-

/usr/local/thunderbird/run-mozilla.sh: line 131: 19395 Segmentation fault
(core dumped) "$prog" ${1+"$@"}
Built using:

export MOZILLA_OFFICIAL="1"
export BUILD_OFFICIAL="1"
export MOZ_THUNDERBIRD="1"
export MOZ_CO_LOCALES="en-GB"
export MOZ_PSM="1"
./configure --enable-default-toolkit=gtk2 --enable-ui-locale=en-GB \
            --with-system-zlib \
            --with-system-jpeg --with-system-png --with-system-mng \
            --enable-xft --disable-freetype2 --enable-crypto \
            --disable-accessibility --disable-profilesharing \
            "-enable-optimize=-march=pentium4 -mfpmath=sse,387 -O2" \
            --disable-tests --disable-debug --with-gssapi=/usr/local/lib \
            --disable-logging --enable-reorder --enable-xinerama \
            --enable-strip --disable-pedantic --disable-installer \
            --disable-mathml --disable-oji --disable-necko-disk-cache \
            --enable-necko-protocols=http,file,jar,viewsource,res,data \
            --enable-image-decoders=default,-xbm --enable-single-profile --
disable-plugins \
            --enable-cpp-rtti --enable-
extensions=wallet,spellcheck,xmlextras,webservices --enable-application=mail

I'll build with tests/debug/strip/optimize enable/enable/disable/disable and
see if I can glean more trace.

[Colin-harrison (colin-harrison)]

Better trace:-

Type Manifest File: /root/.thunderbird/glwfej41.default/xpti.dat
nsNativeComponentLoader: autoregistering begins.
nsNativeComponentLoader: autoregistering succeeded
nsNativeComponentLoader: registering deferred (0)
pldhash: for the table at address 0x80f5748, the given entrySize of 44 probably
favors chaining over double hashing.
GFX: dpi=75 t2p=0.0526316 p2t=19 depth=16
++WEBSHELL == 1
WARNING: NS_ENSURE_TRUE(aRequestingLocation) failed, file
nsMsgContentPolicy.cpp, line 180
++DOMWINDOW == 1
++WEBSHELL == 2
++DOMWINDOW == 2
WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed, file nsStringBundle.cpp, line
273
Note: styleverifytree is disabled
++WEBSHELL == 3
++DOMWINDOW == 3
++WEBSHELL == 4
++DOMWINDOW == 4
Note: frameverifytree is disabled
Note: verifyreflow is disabled
++WEBSHELL == 5
++DOMWINDOW == 5
++WEBSHELL == 6
++DOMWINDOW == 6
++WEBSHELL == 7
++DOMWINDOW == 7
WARNING: Deleting out of flow without tearing down placeholder relationship,
file nsFrame.cpp, line 641
WARNING: nsTimeout::Release() proceeding without context., file
nsGlobalWindow.cpp, line 5480
--WEBSHELL == 6
--DOMWINDOW == 6
WARNING: Deleting out of flow without tearing down placeholder relationship,
file nsFrame.cpp, line 641
--WEBSHELL == 5
--DOMWINDOW == 5
WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed, file nsMsgDatabase.cpp, line
160
WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed, file nsMsgDatabase.cpp, line
160
WARNING: Deleting out of flow without tearing down placeholder relationship,
file nsFrame.cpp, line 641
WARNING: Deleting out of flow without tearing down placeholder relationship,
file nsFrame.cpp, line 641
WARNING: Deleting out of flow without tearing down placeholder relationship,
file nsFrame.cpp, line 641
WARNING: requested removal of nonexistent window
, file nsWindowWatcher.cpp, line 980
--WEBSHELL == 4
GetPrimaryFrameFor() called while nsFrameManager is being destroyed!
WARNING: dependent window created without a parent, file nsAppStartup.cpp, line
438
++WEBSHELL == 5
++DOMWINDOW == 6
WARNING: NS_ENSURE_TRUE(aRequestingLocation) failed, file
nsMsgContentPolicy.cpp, line 180

(Gecko:12107): GLib-GObject-WARNING **: invalid uninstantiatable type `gint64'
in cast to `GtkContainer'

(Gecko:12107): Gtk-CRITICAL **: gtk_container_add: assertion `GTK_IS_CONTAINER
(container)' failed

...
loads of Gecko messages and then
...

(Gecko:12107): Gdk-CRITICAL **: gdk_gc_set_clip_rectangle: assertion `GDK_IS_GC
(gc)' failed
WARNING: Deleting out of flow without tearing down placeholder relationship,
file nsFrame.cpp, line 641

Program /usr/local/thunderbird/thunderbird-bin (pid = 12107) received signal 11.
Stack:
UNKNOWN [/usr/local/thunderbird/thunderbird-bin +0x0001EE33]
__kernel_sigreturn+0x00000000 [ +0x00000420]
UNKNOWN [/usr/local/thunderbird/libxpcom_core.so +0x00041B0E]
UNKNOWN [/usr/local/thunderbird/components/libtoolkitcomps.so +0x00002FAD]
UNKNOWN [/usr/local/thunderbird/components/libtoolkitcomps.so +0x00003C4D]
UNKNOWN [/usr/local/thunderbird/libxpcom_core.so +0x00041B0E]
UNKNOWN [/usr/local/thunderbird/components/libnsappshell.so +0x0000E5DD]
UNKNOWN [/usr/local/thunderbird/components/libnsappshell.so +0x000229A1]
UN...

[Dmose (dmose)]

Even though the pasted stack doesn't seem to have debugging symbols, just
looking at the DLLs that are implicated doesn't suggest to me that this is
actually triggered by an sort of LDAP/SSL problem. Perhaps both the crash and
that error are triggered by something else.

[Colin-harrison (colin-harrison)]

Ok, this may be related. Trace if I open Thunderbird, Address Book, Search
Addresses windows. Then close Address Book and Thunderbird leaving Search
Addresses open. Then lastly close Search Addresses:-

Type Manifest File: /root/.thunderbird/glwfej41.default/xpti.dat
nsNativeComponentLoader: autoregistering begins.
nsNativeComponentLoader: autoregistering succeeded
nsNativeComponentLoader: registering deferred (0)
pldhash: for the table at address 0x80f5798, the given entrySize of 44 probably
favors chaining over double hashing.
GFX: dpi=75 t2p=0.0526316 p2t=19 depth=16
++WEBSHELL == 1
WARNING: NS_ENSURE_TRUE(aRequestingLocation) failed, file
nsMsgContentPolicy.cpp, line 180
++DOMWINDOW == 1
++WEBSHELL == 2
++DOMWINDOW == 2
WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed, file nsStringBundle.cpp, line
273
Note: styleverifytree is disabled
++WEBSHELL == 3
++DOMWINDOW == 3
++WEBSHELL == 4
++DOMWINDOW == 4
Note: frameverifytree is disabled
Note: verifyreflow is disabled
++WEBSHELL == 5
++DOMWINDOW == 5
++WEBSHELL == 6
++DOMWINDOW == 6
WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed, file nsMsgDatabase.cpp, line
160
--DOMWINDOW == 5
--WEBSHELL == 5
--WEBSHELL == 4
--WEBSHELL == 3
--DOMWINDOW == 4
WARNING: requested removal of nonexistent window
, file nsWindowWatcher.cpp, line 980
--WEBSHELL == 2
###!!! ASSERTION: Rejecting event posted to uninitialized
sts: 'PR_GetCurrentThread() != gSocketThread', file nsSocketTranspor
tService2.cpp, line 129
Break: at file nsSocketTransportService2.cpp, line 129
###!!! ASSERTION: unable to post continuation event: 'Error', file
nsStreamUtils.cpp, line 442
Break: at file nsStreamUtils.cpp, line 442
###!!! ASSERTION: Rejecting event posted to uninitialized
sts: 'PR_GetCurrentThread() != gSocketThread', file nsSocketTranspor
tService2.cpp, line 129
Break: at file nsSocketTransportService2.cpp, line 129
###!!! ASSERTION: unable to post continuation event: 'Error', file
nsStreamUtils.cpp, line 442
Break: at file nsStreamUtils.cpp, line 442
--WEBSHELL == 1
--DOMWINDOW == 3
WARNING: nsExceptionService ignoring thread destruction after shutdown, file
nsExceptionService.cpp, line 191
--WEBSHELL == 0
--DOMWINDOW == 2
--DOMWINDOW == 1
--DOMWINDOW == 0
###!!! ASSERTION: Potential deadlock between Monitor@80a9e88 and
Lock@8799960: 'Error', file nsAutoLock.cpp, line 302
Break: at file nsAutoLock.cpp, line 302
JS engine warning: 6 atoms remain after destroying the JSRuntime.
                   These atoms may point to freed memory. Things reachable
                   through them have not been finalized.
GC Cache:
        hits: 4889 818 693 958 137 47 22 15 14 4
        hits: 7597, misses: 801, hit percent: 90.462021%
###!!! ASSERTION: Main thread being held past XPCOM shutdown.: 'cnt == 0', file
nsThread.cpp, line 471
Break: at file nsThread.cpp, line 471
nsStringStats
 => mAllocCount: 21719
 => mReallocCount: 2889
 => mFreeCount: 21718 -- LEAKED 1 !!!
 => mShareCount: 29116
 => mAdoptCount: 2951
 => mAdoptFreeCount: 2906 -- LEAKED 45 !!!

Looks suspect?

[Bienvenu (bienvenu)]

Created an attachment (id=206568)
fix ab quick search ldap leak

we had a reference cycle between the listener and the ldapdirectoryquery - since the ldapdirectoryquery strictly owns the listener, we don't need the add/release ref stuff in the listener

[Bienvenu (bienvenu)]

fixed on trunk.

[Bugzilla-standard8 (bugzilla-standard8)]

Go into the address book window and start a quick search (preferably returning lots of results) and then cancel it whilst the search is still running -> crash.

It crashes at this line:

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/mailnews/addrbook/src/nsAbLDAPDirectoryQuery.cpp&rev=1.33&mark=554#554

If I back out David's patch on bug 206018 this works fine (although may cause other problems looking at that bug) - its because mDirectoryQuery becomes a pointer to nowhere.

Investigating a little further it seems that this section:

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/mailnews/addrbook/src/nsAbLDAPDirectoryQuery.cpp&rev=1.33&mark=204-213#198

should be preventing the address book getting to the crash line but I think for some reason, this section either isn't being set up or something.

Hopefully we can come up with a better fix to sort both of these issues.

[Bugzilla-standard8 (bugzilla-standard8)]

Requesting blocking thunderbird 2 - we shouldn't ship with a crash like this and the fix should be reasonably simple.

[Bienvenu (bienvenu)]

>the fix should be reasonably simple

:-) That has not been my experience. My change in bug 206018 may have caused the problem, or just exposed it - without that change, nothing was ever getting freed, which would hide a multitude of sins. One thing to check is if the assertion I made in that bug is valid, to wit: ldapdirectoryquery strictly owns the listener.

I'll try to recreate this - I wish I had a working Purify again...

[Bienvenu (bienvenu)]

I was able to recreate the crash. nsAbDirSearchListener has a pointer, not a reference, to mSearchContext - the mSearchContext is the object that's been deleted. That object is an nsAbLDAPDirectory, if I'm reading the code right...that can't be right...

[Bienvenu (bienvenu)]

This code alway confuses me - iirc, there are multiple nsIAbdirectory objects/rdf resources - one for the directory itself, and one for the search query that's running. It's the latter that's getting deleted, before the nsAbQueryLDAPMessageListener has finished. I'm not sure what cancelling the quick search actually does, because I don't seem to be hitting the cancel code I would expect...

[Bienvenu (bienvenu)]

the first thing cancelling the search does is clear the mDirectory pointer in nsAbView, by replacing it with an mDirectory for the directory, not the search. That might be the last reference other than js refs that get gc'd away.

[Bienvenu (bienvenu)]

more info - we're never cancelling the ldap search in this case.
We bail out of nsAbLDAPDirectory::StartSearch here:
    if (!mIsQueryURI || mQueryString.IsEmpty())
        return NS_OK;

We fail that test because cancelling the quick search just does a "search" over the whole directory. So the ldap query is never cancelled, and eventually, sometimes crashes (depending on whether the js gc causes the directory object to get freed before the query finishes).

So I think the fix is to actually cancel the query. I'll try it out.

[Bienvenu (bienvenu)]

Created an attachment (id=215331)
proposed fix

[Bienvenu (bienvenu)]

the basic premise is that the AbView owns the directoryQuery at this point, and if it's going to create a new directoryQuery, it should cancel the previous directoryQuery so that it can clean up after itself.

[Bienvenu (bienvenu)]

Created an attachment (id=215712)
proposed fix

OK, I've basically backed out the change that caused this crash and fixed that bug an other way, by breaking the cycle with the listener, in the OnSearchFinished notification...

[Bienvenu (bienvenu)]

oops, ignore the nsAbView change - I'm removing that...

[Bugzilla-standard8 (bugzilla-standard8)]

(From update of attachment 215712)
r=me assuming you don't check in the nsAbView.cpp change.

This does appear to fix the crash, and I can't see any further regressions with this fix in place.

[Mscott-mozilla (mscott-mozilla)]

(From update of attachment 215712)
without the abview change of course.

[Bienvenu (bienvenu)]

After removing the abview change, I crash frequently with the remaining changes on the trunk - the ldap connection gets closed, which it shouldn't - we should be re-using the same connection. And when the ldap connection goes away, that causes strange crashes down the road, when the address book is closed. I'm going to see if cancelling the query (the abview change) makes the crashes and extra connections go away, which would be a bit counter-intuitive, but I didn't see those before.

[Bugzilla-standard8 (bugzilla-standard8)]

*** Bug 332601 has been marked as a duplicate of this bug. ***

[Bugzilla-standard8 (bugzilla-standard8)]

*** Bug 274097 has been marked as a duplicate of this bug. ***

[Pete-mozdevgroup (pete-mozdevgroup)]

This patch was approved for the 1.8 branch. Is it going to land there?

[Kai Engert (kaie)]

Scott, should we check in this patch to the 1.8 branch?

[Kai Engert (kaie)]

(In reply to comment #35)
> Scott, should we check in this patch to the 1.8 branch?

Please ignore my question!!!

Pete, according to bonsai.mozilla.org, the patch has been checked in to the MOZILLA_1_8_BRANCH branch 3 months ago. While no comment has been made in this bug, the keyword fixed1.8.1 in this bug indicates that it has been checked in to the 1.8 branch.

[Bugzilla-standard8 (bugzilla-standard8)]

*** Bug 219087 has been marked as a duplicate of this bug. ***

[Tom Marshall (tommy-home)]

System is current dapper as of this writing. Thunderbird is configured with two accounts:

Home: IMAP4/SSL to Gentoo dovecot 1.0b3 server.
Work: IMAP4/SSL to Ubuntu dovecot 1.0b3 server.

On exit, Thunderbird gives this dialog:

The operation can not be completed because of an internal failure. A secure network communication has not been cleaned up correctly.

[Massey-stlouis-shopper (massey-stlouis-shopper)]

Is there any chance of this being fixed on the 1.8.0 branch? The patch in attachment 206568 does not seem to have been checked in on the 1.8.0 branch. If I apply the patch in attachment 206568 to the 1.8.0 branch the 'unhandled secure connection' alert and crash on shutdown are fixed.

[Massey-stlouis-shopper (massey-stlouis-shopper)]

Created an attachment (id=221813)
ldap-leak-1.8.0.patch

patch against the 1.8.0 branch and is basicaly the same as patch in attachment 206568 except that patch did apply cleanly against the 1.8.0 branch. Fixes the 'unhandled secure connection' and crash on shutdown problem.

[Timr-mozilla (timr-mozilla)]

(From update of attachment 221813)
We are already past code freeze for 1.8.0.4. Can I bump the patch approval request to 1.8.0.5?

[Kai Engert (kaie)]

(From update of attachment 221813)
thanks for making me aware

[Pascal De Vuyst (pascal-devuyst)]

This is a known issue upstream (see bug watch).
It will be resolved in a future version of Thunderbird.
Marking as "Rejected" since there is no task in Ubuntu.

[Dveditz (dveditz)]

Why didn't attachment 206568 apply to 1.8.0, is there some unallowed API skew between the two?

[Massey-stlouis-shopper (massey-stlouis-shopper)]

Attachment 206568 would not apply because nsAbLDAPDirectoryQuery.cpp has gone thru some rev's, had changed just enough. Why it was not checked in on 1.8.0. branch I don't know. But I do know it stops the alert/crash when addressbook is started from tbird. The alert/crash still happens sometimes when addressbook is started like thunderbird -addressbook.

[Bugzilla-standard8 (bugzilla-standard8)]

*** Bug 341402 has been marked as a duplicate of this bug. ***

[Dveditz (dveditz)]

(From update of attachment 221813)
approved for 1.8.0 branch, a=dveditz for drivers

[Kai Engert (kaie)]

let me know if you want me to check 1.8.0 branch patch in

[Bienvenu (bienvenu)]

checking this in will cause https://bugzilla.mozilla.org/show_bug.cgi?id=330507 - I don't think we should be landing this on the branch.

[Dveditz (dveditz)]

(From update of attachment 221813)
never mind, minusing for 1.8.0.x until regressions get worked out.

[Bienvenu (bienvenu)]

I'm not seeing the strange crash I was seeing before, but I am seeing connections getting closed. Time to dive into this again.

[Bienvenu (bienvenu)]

we were still closing connections w/o this patch, so I think I'm going to go with this patch for now on trunk and 1.8 branch

[Ria-klaassen (ria-klaassen)]

Could this have broken Super DragAndGo (search function)?
I'm using userChrome.js extension with the Drag'n'go code snippet.
http://forums.mozillazine.org/viewtopic.php?t=397735&postdays=0&postorder=asc&postsperpage=15&

When I select a word, drag it away and release it, it only throws and error instead of doing a search:

Error: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIStringBundle.GetStringFromName]
Source file: XStringBundle
Line: 17

Some patch checked in between 1.9a1_2006062921 and 1.9a1_2006063014 broke it.
I'm just trying to find out what patch caused this.

[Bugzilla-standard8 (bugzilla-standard8)]

*** Bug 346800 has been marked as a duplicate of this bug. ***

[Bienvenu (bienvenu)]

this change should have nothing to do with the super drag and drop error.

[Bienvenu (bienvenu)]

OK, the checked in fix has issues. It's now possible to get multiple threads going against the same nsLDAPConnection object, which causes a crash, if you do quick search in the ab view. By nulling out mListener in OnSearchFinished, we cause nsAbLDAPDirectoryQuery::DoQuery to re-init the nsLDAPConnection object, which makes it spin up a new thread. I'll have to think about how to fix this.

[Bienvenu (bienvenu)]

Created an attachment (id=243490)
proposed fix

instead of clearing listener to break cycle at end of search, just don't hold a reference to the ldap connection, so there's no cycle to begin with.

[Bienvenu (bienvenu)]

with this patch, I verified that we're still cleaning up ldap connections when the address book is closed...

[Bienvenu (bienvenu)]

(From update of attachment 243490)
clearing request - this introduces a crash if you close the address book while an ldap search is going on. The directory the mListener points to gets deleted. mListener might want a weak ref to the directory...or else we should somehow clear the directory pointer from the listener when the directory gets closed...a weak ref is probably the way to go...

[Bienvenu (bienvenu)]

also, I should say - for 2.0, the problem is that we never close the ldap connections - it doesn't have this patch, per se.

[Bienvenu (bienvenu)]

Created an attachment (id=244127)
fix for leaking connections that doesn't crash

[Bienvenu (bienvenu)]

that patch makes nsAbLDAPDirectoryQuery null out the nsAbQueryLDAPMessageListener's mDirectoryQuery pointer when the nsAbLDAPDirectoryQuery gets destroyed. And it makes nsAbQueryLDAPMessageListener check if the pointer is null in a few places, to avoid a crash.

[Bienvenu (bienvenu)]

Standard8, that patch is for the 2.0 branch.

[Bienvenu (bienvenu)]

(From update of attachment 244127)
for the trunk, this fixes the crash closing the ab view while a search is still going on.

[Bienvenu (bienvenu)]

https://bugzilla.mozilla.org/attachment.cgi?id=244127 is for the 2.0 branch - on the 2.0 branch, we still leak connections. What I meant to say was that a very similar change on the trunk will fix the crash when you close the ab window while an ldap query is going on, i.e., part of that patch.

[Bienvenu (bienvenu)]

Created an attachment (id=244265)
fix one more case

Mark, this might fix the crash you saw - we were getting into js gc which was clearing out the directory query out from under the listener. If I null out the search context as well, and check for that, that fixes one last crash. This patch is also for 2.0, though it will need to go on the trunk as well.

[Bugzilla-standard8 (bugzilla-standard8)]

(From update of attachment 244265)
Ok, this seems to work fine now. The code looks reasonable as well.

[Bienvenu (bienvenu)]

the real fix for this is in bug 330507 - this fix was bad

[Bienvenu (bienvenu)]

fixed on branch

[Bienvenu (bienvenu)]

fixed on trunk.

[Bugzilla-standard8 (bugzilla-standard8)]

*** Bug 351642 has been marked as a duplicate of this bug. ***

[Jbecerra-mozilla (jbecerra-mozilla)]

Thunderbird 2.0.0.0; version 2.0.0.0 (20070326) does not crash when canceling an address book search, or when closing the address book window. Adding verified1.8.1.3 keyworkd.

[Erich-ebremer (erich-ebremer)]

I am getting this problem with Thunderbird 2.0.0.22. "The operation cannot be completed because of an internal failure. A secure network communication has not been cleaned up correctly." The LDAP searches work, just gives the error upon exiting Thunderbird.

[Davidsmerrill (davidsmerrill)]

I'm getting same error as in #51 with version 2.0.0.23 (20090812) on WinXP SP3.

[Raphael-fairise-bugs (raphael-fairise-bugs)]

This issue is not fixed. An "alert" window is still displayed when Thunderbird is closed (tested on Windows and Linux, version 2.0.0.23 and 3.0.1).

Happens after an address book search or an auto-completion in compose window.

Steps to Reproduce in address book:
- open address book
- search something in a LDAPS directory
- close address book before or just after all the responses are received (in the next 5 seconds)
- close Thunderbird immediately
(alert window is displayed every time)

Steps to Reproduce in compose window:
- open compose window
- type something in a recipient field with auto-completion from a LDAPS directory
- close the compose window without saving
- close Thunderbird immediately
(alert window is displayed sometimes)

Shouldn't we disable this alert window? (or convert it to a console message)
I don't think it's a big issue if some network connections are not cleaned up correctly when the software is closed too quickly.

[Dmose (dmose)]

Adding qawanted for verification help. Please set this bug to NEW and do any nominations you feel are appropriate or else re-close it. Thanks!

[Dmose (dmose)]

After discussion with Standard8, it seems that this particular bug is probably not the right one to reopen. I'll re-resolve this, and clone a separate bug for those interested with Comment 53 included.

[Just4fun73de (just4fun73de)]

(In reply to comment #55)
I'll re-resolve this, and clone a separate bug
> for those interested with Comment 53 included.

Where can I find the cloned bug report ?

[Raphael-fairise-bugs (raphael-fairise-bugs)]

(In reply to comment #56)
> Where can I find the cloned bug report ?

You already found it: bug 544939

[Vseerror (vseerror)]

*** Bug 129107 has been marked as a duplicate of this bug. ***