397744 - Can't access to the third account and the local mail [@ nsXULTreeBuilder::RemoveMatchesFor(nsTreeRows::Subtree&)]

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Stéphane Grégoire (yamo)]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; rv:1.9a9pre) Gecko/2007092601 SeaMonkey/2.0a1pre
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; rv:1.9a9pre) Gecko/2007092601 SeaMonkey/2.0a1pre

The third account who is a pop3 account don't have a "+" icon so I can't open it.

And  the old bug ( https://bugzilla.mozilla.org/show_bug.cgi?id=396740 ) don't have this, the directories "Dossiers locaux" is also afected by this bug.

And  after some clicks seamonkey crash.

Reproducible: Always

Steps to Reproduce:
1.open mails
2.click on the buggie account some times
3.
Actual Results:  
crash (whith the  Seamonkey2.0a1 20070912 there's no bug)

Expected Results:  
Account with a "+" icon and can view the account.



Reproducible: Always

Steps to Reproduce:
1.open mails
2.click on the buggie account some times
3.
 

Actual Results:  
crash when I try to open a mail dir whom as the "-" icon and not the "+" icon (whith the  Seamonkey2.0a1 20070912 there's no bug)



Expected Results:  
Account with a "+" icon and can view the account.

Comment 1

[Stéphane Grégoire (yamo)]

Attachment: crashreporter

Comment 3

[Stéphane Grégoire (yamo)]

The security is not normal I think it's critical I missed this checkbox the first time 

And it's for the trunk 

Comment 4

[Frank Wein [:mcsmurf]]

You need to mention the Breakpad ID you get or does the Crashreporter not pop
up when you crash? Attaching the crashreporter app is not the correct way I
think ;-). Or are the instruction under
http://kb.mozillazine.org/Breakpad#Linux_2 not clear for you? I do not use
Linux, so I cannot tell if they are correct (but I think they are).

Comment 5

[Stéphane Grégoire (yamo)]

Attachment: crashreporter when click on local mail directory

the  #282525 was when clicking on the third pop3 account

Comment 6

[Frank Wein [:mcsmurf]]

I don't know what files you are attaching there, but the Crashreporter id files are text files, roughly ~1 KB in size and have a file name like "bp-[Universally Unique Identifier value with 32 letters]" :).

Comment 7

[Stéphane Grégoire (yamo)]

The file that I files is the one on http://kb.mozillazine.org/Breakpad#Linux_2  :
/<directory>/<where>/<seamonkey2a>/<is/<installed>/crashreporter


And I also can't view it like a backtrace binary file!

The LastCrash is  1190886513


on linux to find it I had to type on a console :

 find ~ -name "Crash Reports" -type d 


I have 16 crash files

 
I hope it's help you.

Best Regards.

Comment 8

[Frank Wein [:mcsmurf]]

I think you need to look under /home/<username>/.mozilla.org/seamonkey/Crash Reports/submitted. There open the file that was created/changed last and paste the text in the bug here.

Comment 9

[Stéphane Grégoire (yamo)]

Attachment: I think it's the good one!

these file is at the directory 
~/.mozilla.org/seamonkey/Crash Reports/submitted

I have 17 files in this directory.

Comment 10

[Frank Wein [:mcsmurf]]

Attachment: Stack trace from Breakpad

Comment 11

[Frank Wein [:mcsmurf]]

Possibly dupe of Bug 356025.

Comment 12

[Robert Kaiser]

BTW, just posting the ID from the text file into a comment nicely links the full report on the crash report server: bp-d382ead7-6cde-11dc-b161-001a4bd43e5c

Comment 13

[Stéphane Grégoire (yamo)]

Hi, 

I've read your article (
 http://wiki.mozilla.org/SeaMonkey:Moving_Profiles_from_mozilla.org_to_Mozilla ), the script doesn't work on my system (failed to parse profiles.ini) but the migration is very simple. 

with  Mozilla/5.0 (X11; U; Linux i686; rv:1.9a9pre) Gecko/2007101702 SeaMonkey/2.0a1pre this bug has disappeared! :)

Should I mark this bug as fixed?

Comment 14

[Stéphane Grégoire (yamo)]

I don't understand.
After a reboot, the bug is back :(

Comment 15

[Stéphane Grégoire (yamo)]

Hi, 
On Seamonkey 2.0 a2 this bug seems to be fixed.

Comment 16

[Frank Wein [:mcsmurf]]

Glad to hear. Resolving wfm.

Comment 17

[Wayne Mery (:wsmwk)]

Stéphane, have you seen this since your last comment?

Comment 18

[Stéphane Grégoire (yamo)]

Since building 2.0.1 on Ubuntu9.04 64bit I didn't seen it.

I've seen it many times on 2.0.0

Happy new year!

Comment 19

[Stéphane Grégoire (yamo)]

I have this bug on a folder when running Seamonkey 2.0.2 for a long time (36 hours).

You may reproduce this bug when having a lot a folder overlapped (I wanted to say in French "imbriqués").

Comment 20

[Wayne Mery (:wsmwk)]

can you attach a screen shot?

Comment 21

[Stéphane Grégoire (yamo)]

Attachment: directory Ubuntu cannot be open

The directory Ubuntu cannot be open and inside this directory there's a directory "securité" where there's a unread mail

Comment 22

[Stéphane Grégoire (yamo)]

Attachment: After a restart

Comment 23

[Stéphane Grégoire (yamo)]

Hi,

Today I had this bug on some subdirectories with SeaMonkey/2.0.5 rc build1

Mozilla/5.0 (X11; U; Linux x86_64; fr; rv:1.9.1.10) Gecko/20100504 Lightning/1.0b2pre Mnenhy/0.8.2 SeaMonkey/2.0.5 not Firefox/3.5

Comment 24

[Wayne Mery (:wsmwk)]

Stéphane, thanks for updating the bug. no doubt is nsXULTreeBuilder::RemoveMatchesFor(nsTreeRows::Subtree&) 

Is XUL a victim here of some js badness?

#4 crash for Seamonkey 2.0.4 (up from #7 in 2.0.2) and #76 crash for Thunderbird 3.0.4.  (almost non-existent for firefox)

crash goes back at least as far as TB 3.0a1. and exists on trunk as of 20100428 with SM crash bp-051152c9-d588-49bf-840f-c8f9a2100428

All Seamonkey crashes on crash-stats cite bookmarks
bp-3ed6cb10-15ac-4d82-a335-1e5802100424 (BorkZork)
bp-8bb55e1f-f23d-47bf-98a6-d8f142100428
bp-c7f14a7d-0fb9-4570-bd13-b07dc2100507 (jfca)


bp-051152c9-d588-49bf-840f-c8f9a2100428 SM 2.1a1pre, no comment, a few extensions
0	seamonkey.exe	nsXULTreeBuilder::RemoveMatchesFor	 content/xul/templates/src/nsXULTreeBuilder.cpp:1736
1	seamonkey.exe	nsXULTreeBuilder::CloseContainer	content/xul/templates/src/nsXULTreeBuilder.cpp:1713
2	seamonkey.exe	nsXULTreeBuilder::ToggleOpenState	content/xul/templates/src/nsXULTreeBuilder.cpp:869
3	xpcom_core.dll	NS_InvokeByIndex_P	xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:102
4	seamonkey.exe	XPCWrappedNative::CallMethod	js/src/xpconnect/src/xpcwrappednative.cpp:2282
5	seamonkey.exe	XPC_WN_CallMethod	js/src/xpconnect/src/xpcwrappednativejsops.cpp:1770
6	mozjs.dll	js_Invoke	js/src/jsinterp.cpp:834
7	mozjs.dll	js_Interpret	js/src/jsops.cpp:2244
8	mozjs.dll	js_Invoke	js/src/jsinterp.cpp:842
9	mozjs.dll	js_InternalInvoke	js/src/jsinterp.cpp:899
10	mozjs.dll	JS_CallFunctionValue	js/src/jsapi.cpp:4945
11	seamonkey.exe	nsJSContext::CallEventHandler	dom/base/nsJSEnvironment.cpp:2163
12	seamonkey.exe	nsJSEventListener::HandleEvent	dom/src/events/nsJSEventListener.cpp:228
13	seamonkey.exe	nsEventListenerManager::HandleEventSubType	content/events/src/nsEventListenerManager.cpp:1082
14	seamonkey.exe	nsEventListenerManager::HandleEventInternal	content/events/src/nsEventListenerManager.cpp:1178
15	seamonkey.exe	nsEventListenerManager::HandleEvent	content/events/src/nsEventListenerManager.h:144
16	seamonkey.exe	nsEventTargetChainItem::HandleEvent	content/events/src/nsEventDispatcher.cpp:211
17	seamonkey.exe	nsEventTargetChainItem::HandleEventTargetChain	content/events/src/nsEventDispatcher.cpp:363
18	seamonkey.exe	nsEventDispatcher::Dispatch	content/events/src/nsEventDispatcher.cpp:620
19	seamonkey.exe	PresShell::HandleEventInternal	layout/base/nsPresShell.cpp:6431
20	seamonkey.exe	PresShell::HandleEventWithTarget	layout/base/nsPresShell.cpp:6287 


But Stéphane's report aligns more with Thunderbird crashes
bp-1d69eb9d-c79b-4899-8972-037172100412 trying to revive an old account and my password will not work
bp-2f0bf6a1-bbb2-4b1d-9a13-2244b2100415 Searching a folder, I needed to change the folder, but T crashed before I could. 
bp-bb9b2f2e-4ef8-421a-84d6-7255a2100425 data (both header and content) in some messages disappear until restarting Tbird, or until leaving and re-entering the folder. (mchodrow)

Comment 26

[Robert Kaiser]

Being in CloseContainer, I wonder if we are closing a container that somehow ended up in a weird state regarding its children, esp. as the crashing line seems to be |for (PRInt32 i = subtree.Count() - 1; i >= 0; --i) {| which makes me think subtree.Count() is somehow returning something bad (at least my non-C++-trained eyes and brain get to that conclusion).

I believe we shouldn't crash even if weirdness happened - still, there must be some bug that caused it in the first place.

Comment 27

[timeless]

1703 nsXULTreeBuilder::CloseContainer(PRInt32 aIndex)
1709   nsTreeRows::iterator iter = mRows[aIndex];
array reference, if mRows is null, we crash here, we didn't, so mRows is not null.
1711   nsTreeRows::Subtree& subtree = *(iter->mSubtree);
pointer dereference on iter, if iter was null, we'd crash here, we didn't so iter is not null.
because subtree is a Substree&, the * dereference here is deferred to the callee, and thus it can be null even though you'd expect to have crashed here if it were.
1713 RemoveMatchesFor(subtree); 

1734 nsXULTreeBuilder::RemoveMatchesFor(nsTreeRows::Subtree& subtree)
1736   for (PRInt32 i = subtree.Count() - 1; i >= 0; --i) { 

Even though it looks like subtree is a local object, it's in fact our friendly maybe null object pointer from 1711. And in fact, we're crashing at 0x4, which means we have a null pointer.

108         Subtree* mParent;
This is +0x0
113         PRInt32 mCount;
This is +0x4

131     public:
135         Subtree(Subtree* aParent)
140               mRows(nsnull) {}
142         ~Subtree();
147         PRInt32 Count() const { return mCount; }

Count() is inlined, so we're more interested in mCount. So we have a null this pointer dereference for mCount.

Comment 28

[timeless]

Attachment: patch

Comment 29

[[e-mail]]

Comment on attachment 444306 [details] [diff] [review]
patch

>+    if (iter->mSubtree) {
>+        RemoveMatchesFor(*iter->mSubtree);
Given that other users of mSubtree null-check it too (well one checks the result of a user that null-checks it), I agree that this is the way to go.

>+        // Update the view
>+        iter = mRows[aIndex];
This statement appears to have no effect (indeed operator[] goes as far as to cache the result just in case!) but you could ask Enn if you want to be sure.

Comment 30

[timeless]

Comment on attachment 444306 [details] [diff] [review]
patch

enn: my impression is that mRows can be reallocated or mutated in interesting ways which is why this code doesn't cache iter. Am I wrong?

Comment 31

[Neil Deakin]

Comment on attachment 444306 [details] [diff] [review]
patch

>-    nsTreeRows::Subtree& subtree = *(iter->mSubtree);
>+    if (iter->mSubtree) {
>+        RemoveMatchesFor(*iter->mSubtree);
> 
>-    RemoveMatchesFor(subtree);
>-
>-    // Update the view
>-    iter = mRows[aIndex];
>+        // Update the view
>+        iter = mRows[aIndex];
>+    }

RemoveMatchesFor doesn't change the rows, so regetting the iterator shouldn't be needed. (The rows are removed in RemoveSubtreeFor called afterwards.)

Comment 32

[timeless]

Attachment: patch

Comment 33

[[e-mail]]

Comment on attachment 444420 [details] [diff] [review]
patch

> 
>-    RemoveMatchesFor(subtree);
>-
>-    // Update the view
>-    iter = mRows[aIndex];
> 
Won't this result in two consecutive blank lines? ;-)

Comment 34

[timeless]

http://hg.mozilla.org/mozilla-central/rev/f827953eb7f8

oh well, we can zap that blank line later. i'm sure i had it fixed in my tree, but i was using someone else's try series.