81692 - Crash Backspacing in Addressing Widget of Compose Window

Status: VERIFIED FIXED

(MailNews Core :: Composition, defect, P1)

Description

[sol]

I crash when I (a) send a web page via email from Navigator *and* (b) backspace 
in the addressing widget of the compose window.

Running 2001-05-17-04 (commercial) on WinNT.

I can reproduce this about 75% of the time. It seems that following a particular 
order of steps is very important in reproducing this bug.

Steps to reproduce.

1. Launch MoJo.

2. Launch Mail (Tasks | Mail). Authenticate to email server when prompted. I'm 
using IMAP.

3. Go back to Navigator. Browse to a site - say, http://www.mozilla.org/

4. Send the web page via email (File | Send Page ...).

5. *As soon as you see the cursor* in the addressing widget in the email compose 
window, begin typing, and then - after you've typed about 5 characters - quickly 
start hitting the backspace key (like you made a mistake in your typing and want 
to correct it right away). 

Note - I have configured email addressing to run against my LDAP server at work.

Expected result: The text you have typed in the addressing widget is deleted, 
and the cursor backspaces.

Actual result: CRASH!!

Stack trace:

Trigger Type:  Program Crash 

Trigger Reason:  Access violation 

Thread ID: 203  

Call Stack:    (Signature = nsLDAPAutoCompleteSession::FinishAutoCompleteLookup 
8ed50941) 
     
nsLDAPAutoCompleteSession::FinishAutoCompleteLookup

[d:\builds\seamonkey\mozilla\xpfe\components\autocomplete\src\nsLDAPAutoComplete
Session.cpp, line 923]
     
nsLDAPAutoCompleteSession::OnLDAPSearchResult 

[d:\builds\seamonkey\mozilla\xpfe\components\autocomplete\src\nsLDAPAutoComplete
Session.cpp, line 601]
     
nsLDAPAutoCompleteSession::OnLDAPMessage 

[d:\builds\seamonkey\mozilla\xpfe\components\autocomplete\src\nsLDAPAutoComplete
Session.cpp, line 327]

Notes:

Seems to be crashing in LDAP code.

For more information, see Talkback incidents: 30630928 and 30631144 (identical 
stack traces).

Comment 1

[Jean-Francois Ducarroz]

Looks like a LDAP problem, reassign to dmose

Comment 2

[chris hofmann]

are the options 1) fix the crash  or 2) take out the feature??

Comment 3

[Dan Mosedale (:dmosedale, :dmose)]

chofmann: a third option would be to release note it.  My impression is that
this crash isn't all that common, and the easy workaround is to disable LDAP in
the prefs.

I've been able to reproduce this maybe twice (out of about 10 attempts) running
on Linux with the release build.  Unfortunately, I've never seen it under debug
builds at all (let alone while running under the debugger) after about 30
attempts, so it's a very timing sensitive thing.  Haven't been able to catch it
under the debugger on Mac either; Srilatha's had similar experiences on windows;
she's going to try over a slower connection once she gets home this evening.

However, from reading the code, I have a guess as to what could be happening. 
I'm gonna go grovel through the code some more, and will post my thoughts in a bit.

Comment 4

[Dan Mosedale (:dmosedale, :dmose)]

OK, after grovelling through the source for a while (and thanks to talkback) I
see what could cause this crash.  With luck I'll have a patch later tonight.

Comment 5

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: minimal paper-over-the-problem patch

Comment 6

[Dan Mosedale (:dmosedale, :dmose)]

I've just attached a minimal paper-over-the-problem fix.  With just this patch,
the race still exists, but if we lose the race, the result will just be a
confused autocomplete session rather than a crash.  Worst case, I'd suggest
shipping with this patch.

However, I expect that with a few more hours of work tommorrow (Weds), I'll have
a better patch that includes this paper-over fallback, but also eliminates the
race entirely.

Comment 7

[Dan Mosedale (:dmosedale, :dmose)]

OK, I've got the real fix.  A side-effect of this fix is that the race described
in 70422 is fixed also.

Comment 8

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: patch v2

Comment 9

[Dan Mosedale (:dmosedale, :dmose)]

Actually, the side effect is that 77452 is fixed, NOT 70422.

Comment 10

[Frank Tang]

who should be the reviewer?

Comment 11

[Dan Mosedale (:dmosedale, :dmose)]

ducarroz, hewitt, or chuang.  mscott would make a fine sr=.  :-)

Comment 12

[Jean-Francois Ducarroz]

the patch looks good. R=ducarroz

Comment 13

[Dan Mosedale (:dmosedale, :dmose)]

This has been built and tested on all three platforms.

Comment 14

[Scott MacGregor]

sr=mscott

Comment 15

[Asa Dotzler [:asa]]

a= [e-mail] for checkin to 0.9.1

Comment 16

[Dan Mosedale (:dmosedale, :dmose)]

Fix checked in.

Comment 17

[esther]

Using build 2001-05-31 on mac, linux and windows this is fixed.  Verified.
I added the LDAP directory and went back to a 5-23 build to see the crash.  I 
then ran sol's test on all three platforms and did not crash.