289503 - TB102 [@ nsQueryElementAt::operator][@ nsMsgLocalMailFolder::OnStopRunningUrl] TB102 [@ nsMsgMailSession::GetTopmostMsgWindow] infinite recursion when opening mail folders

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Patrick Reiter Horn]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; chrome://navigator/locale/navigator.properties; rv:1.7.5) Gecko/20041107 Firefox/1.0
Build Identifier: Thunderbird 1.0.2 (20050403) [Compiled from source]

A crash occurs happens when I went to a local imported folder, and after bug
269554 occurred (although the patch I applied to 1.0.2 made it not crash.)


Reproducible: Couldn't Reproduce

Steps to Reproduce:
1. Browse around when trying to check a lot of folders
2. Browsing around to an old imported folder from Netscape 4 causes it to crash.

Actual Results:  
Thunderbird crashes after a slight pause (due to recursion).
Everything was fine after restarting

Expected Results:  
Opened folders like it did every other time and like it did after restarting
thunderbird.


I will attach a complete gdb backtrace for the problem.

The infinite recursion happens here:
#12 0xb7504493 in nsMsgLocalMailFolder::OnStopRunningUrl (this=0x8779298, 
    aUrl=0x904274c, aExitCode=0) at nsLocalMailFolder.cpp:3293
3293        mReparseListener->OnStopRunningUrl(aUrl, aExitCode);

when mReparseListener is the same is this.

The solution might be to check mReparseListener==this before calling it recursively.


Build Config:
gcc:
 gcc version 3.3.5 (Debian 1:3.3.5-8)  	-Wall -W -Wno-unused -Wpointer-arith
-Wcast-align -Wno-long-long -pedantic -pthread -pipe
c++:
 gcc version 3.3.5 (Debian 1:3.3.5-8)
 -fno-rtti -fno-exceptions -Wall -Wconversion -Wpointer-arith -Wcast-align
-Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor
-Wno-long-long -pedantic -fshort-wchar -pthread -pipe   -I/usr/X11R6/include

Configure arguments
 --enable-crypto --disable-mathml --disable-activex --disable-activex-scripting
--disable-tests --disable-oji --disable-plugins --disable-necko-disk-cache
--enable-single-profile --disable-profilesharing
--enable-extensions=wallet,spellcheck,xmlextras,webservices
--enable-necko-protocols=http,file,jar,viewsource,res,data
--enable-image-decoders=default,-xbm

Comment 1

[Patrick Reiter Horn]

Attachment: A GDB backtrace of this bug,with output and a few variables.

The recursion happens in mailnews/local/src/nsLocalmailFolder.cpp, line 3293.

One simple solution would be to check if this==mReparseListener before calling
the function on itself.

Another, possibly more correct solution would be to save mReparseListener to a
temporary variable, and set it to NULL before doing recursion, instead of
after.

Comment 2

[sairuh (rarely reading bugmail)]

perhaps a dup of bug 290078?

*** This bug has been marked as a duplicate of 290078 ***

Comment 3

[sairuh (rarely reading bugmail)]

not sure if this is a dup --when I looked more closely at the talkback data for
bug 290078, all of the crashes were (so far) only on Windows machines.

Comment 4

[Patrick Reiter Horn]

(In reply to comment #3)
> not sure if this is a dup --when I looked more closely at the talkback data for
> bug 290078, all of the crashes were (so far) only on Windows machines.

I'm pretty sure that this is the same bug: the top of the stack is in
nsMsgLocalMailFolder::OnStopRunningUrl, when it calls a virtual function
referencing itself.

Checking for that condition might fix this bug:
Save it in temporary variable, set to NULL, and then call OnStopRunningUrl on
line 3292 of nsLocalMailFolder.cpp.

That fix can definitely not hurt anything (a function should never be able to
call itself recursively with the same conditions), but it won't solve the bigger
problem that causes this condition. And this may only make it happen somewhere else.

I got it to reproduce consistently (I tried 6 times: The four where I clicked
away and back got it to crash)

I just decided to try out the breakpoint at #19411 in my callback, and I am able
to get it to consistently happen on Linux in a GDB session:
nsMailboxProtocol::OnStopRequest (the function at the bottom of bactrace that
calls OnStopRunningURL is called only after generating MSF summary files.

1. Delete the MSF, and browse to the local folder with the bug (in my case,
Communicator 4.x Mail\Inbox)
2. While it is generating the summary, browse to another folder and then make
sure to go back to the original folder: it does not happen if you do not leve
and come back.  I assume that it sets the active folder as the summarising
folder's callback when you change (so Inbox becomes its own callback class).
3. When it is done generating the summary, it gets to my breakpoint in
nsMailboxProtocol::OnStopRequest (mailnews/local/src/nsMailboxProtocol.cpp:257)
4. Going down to the bottom of the function (nsMailboxProtocol.cpp:391) where it
calls OnStopRequest() goes to the recursive function.

Now that I can reproduce his bug, the next step is finding the exact place where
the callback gets set and then fixing the source.

The backtrace at OnStopRequest() is:
#0  nsMailboxProtocol::OnStopRequest (this=0x8609968, request=0x8ce4ec8, 
    ctxt=0x8c38d80, aStatus=0) at nsMailboxProtocol.cpp:260
#1  0xb7458f24 in nsInputStreamPump::OnStateStop (this=0x8ce4ec8)
    at nsInputStreamPump.cpp:498
#2  0xb7458928 in nsInputStreamPump::OnInputStreamReady (this=0x8ce4ec8, 
    stream=0x8c1a404) at nsInputStreamPump.cpp:339
#3  0xb7c024df in nsInputStreamReadyEvent::EventHandler (plevent=0x8c4e7c4)
    at nsStreamUtils.cpp:118
#4  0xb7c263ff in PL_HandleEvent (self=0x8c4e7c4) at plevent.c:673
#5  0xb7c262b4 in PL_ProcessPendingEvents (self=0x80f09b8) at plevent.c:608
#6  0xb7c295ae in nsEventQueueImpl::ProcessPendingEvents (this=0x8112180)
    at nsEventQueue.cpp:398
#7  0xb766d1a0 in event_processor_callback (data=0x8112180, source=7, 
    condition=GDK_INPUT_READ) at nsAppShell.cpp:186
#8  0xb766cb4d in our_gdk_io_invoke (source=0x82527e8, condition=G_IO_IN, 
    data=0x822c238) at nsAppShell.cpp:71
#9  0xb7d7fa56 in g_io_add_watch () from /usr/lib/libglib-1.2.so.0
#10 0xb7d8103d in g_get_current_time () from /usr/lib/libglib-1.2.so.0
#11 0xb7d814f4 in g_get_current_time () from /usr/lib/libglib-1.2.so.0
#12 0xb7d81724 in g_main_run () from /usr/lib/libglib-1.2.so.0
#13 0xb7e6d25f in gtk_main () from /usr/lib/libgtk-1.2.so.0
#14 0xb766d5ea in nsAppShell::Run (this=0x80f5bc8) at nsAppShell.cpp:317
#15 0xb774186e in nsAppShellService::Run (this=0x80f4b18)
    at nsAppShellService.cpp:494
#16 0x0805e69f in xre_main (argc=1, argv=0xbfffe654, aAppData=0x807500c)
    at nsAppRunner.cpp:1907
#17 0x080585ac in main (argc=1, argv=0xbfffe654) at nsMailApp.cpp:58
(gd
Full backtrace of top few functions:
gdb) 
(gdb) bt full
#0  nsMailboxProtocol::OnStopRequest (this=0x8609968, request=0x8ce4ec8, 
    ctxt=0x8c38d80, aStatus=0) at nsMailboxProtocol.cpp:260
        rv = 3074777681
        stopped = -1073750696
#1  0xb7458f24 in nsInputStreamPump::OnStateStop (this=0x8ce4ec8)
    at nsInputStreamPump.cpp:498
No locals.
#2  0xb7458928 in nsInputStreamPump::OnInputStreamReady (this=0x8ce4ec8, 
    stream=0x8c1a404) at nsInputStreamPump.cpp:339
        nextState = 3
#3  0xb7c024df in nsInputStreamReadyEvent::EventHandler (plevent=0x8c4e7c4)
    at nsStreamUtils.cpp:118
        ev = (nsInputStreamReadyEvent *) 0x8c4e7c0

Should I post this on bug 290078?

Comment 5

[David :Bienvenu]

Patrick, thanks for the steps to reproduce this. I'll try this today. Yes, I
believe these are all the same problems...

Comment 6

[David :Bienvenu]

Attachment: proposed fix

avoid infinite recursion...

Comment 7

[(not reading, please use [e-mail] instead)]

Comment on attachment 180748 [details] [diff] [review]
proposed fix

r/sr=sspitzer

Comment 8

[(not reading, please use [e-mail] instead)]

david, please consider adding what you wrote over aim as a comment in the code:

// if mReparseListener is null, then we won't call OnStopRunningUrl on it the
next time we get called
// we were passing on the notification to ourselves and recursing infinitely
// so I saved the listener in a temp var first...

Comment 9

[David :Bienvenu]

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

Comment 10

[David :Bienvenu]

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

Comment 11

[(not reading, please use [e-mail] instead)]

Comment on attachment 180748 [details] [diff] [review]
proposed fix

a=sspitzer for tbird 1.1a

Comment 12

[Hermann Schwab]

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