Closed Bug 220126 Opened 21 years ago Closed 21 years ago

crash in the Mailnews Filter Dialog [@ PresShell::PostReflowCallback]

Categories

(Core :: Layout, defect, P2)

defect

Tracking

()

VERIFIED FIXED

People

(Reporter: Matti, Assigned: MatsPalmgren_bugz)

References

Details

(Keywords: crash, regression)

Crash Data

Attachments

(3 files, 3 obsolete files)

win2k build 20030923..

1) select Theme : modern
2) open Mailnews
3) select Tools/Message Filters
4) select an existing entry and use "Edit"
5( add a rule with "MORE" and select customize and add your special header
6) select label as "work" as action
7) use the scrollbar to scroll the actions list up&down
8) select "OK" or click on the "X"

You should now seen a crash between 6) and 8)

I could reproduce it 5x in a row
Attached file stack trace #2
works in 1.4 and I'm unable to change my message filter with a trunk optimized
and/or debug build
Keywords: regression
*** Bug 220133 has been marked as a duplicate of this bug. ***
Happens in classic too - Talkback ID TB23846475M
Doesn't crash in BuildID 2003091704 on WinXP SP1 but does in BuildID 2003092304
Minimised reproducable case:
1) Start mail
2) Select Tools, Message Filters
3) Hightlight existing filter and click Edit
4) Click ok/cancel/x
5) CRASH!
And for those that haven't got a message filter already set up:
1) Start mailnews
2) select Tools/Message Filters
3) create a new filter by cliking on new eg Subject contains abcxyz and tell it
to move it the trash folder
4) ok the new filter
5) select edit in the list of filters
6) Click on ok/cancel/x and watch it CRASH!
Setting OS->All as dupe is on Linux and this one is on 2k/XP
OS: Windows 2000 → All
Purify output is likely to show the problem quite clearly.  Valgrind might. 
Otherwise finding the problem may require spending a good bit of time in the
debugger.

Can anyone try this under purify?
regression between linux trunk 2003091822 and 2003092022
I post results from valgind whenever it actually manages to come up.
All I get with valgrind is:
Jump to the invalid address stated on the next line
 0x0: ???
 0x44114A4C: PresShell::ResizeReflow() (nsPresShell.cpp:3053)
 0x4411D776: PresShell::ResizeReflow() (nsPresShell.cpp:6299)
 0x444806B5: nsViewManager::SetWindowDimensions() (nsViewManager.cpp:588)
 0x44483754: nsViewManager::DispatchEvent() (nsViewManager.cpp:1851)
 0x4447BA6B: HandleEvent() (nsView.cpp:76)
 0x45DC8573: nsWidget::DispatchEvent() (nsWidget.cpp:1506)
 0x45DC82B4: nsWidget::DispatchWindowEvent() (nsWidget.cpp:1394)

Mozilla's stack walker sees the same.  gdb can't see past the signal handler.
*** Bug 220374 has been marked as a duplicate of this bug. ***
*** Bug 220374 has been marked as a duplicate of this bug. ***
backing out bug 217201 fixed this bug for me
==> roc
Assignee: other → roc
*** Bug 220662 has been marked as a duplicate of this bug. ***
*** Bug 220722 has been marked as a duplicate of this bug. ***
If I comment out the call to FreeFrame( ) on line 5129, the bug
goes away.

This call does look a little early, but I may on the wrong lines, in saying
this; I think that I am right in suggesting that at some point we
are  calling through bogus pointers.
I'm not sure if bug 220629 is related to this. I've noticed it in my linux
builds of tbird over the same timeframe, and that it always crashes if you
highlight a message in the inbox and select "Create filter from message". 

I can also duplicate/confirm the crashes in this bug on my builds.


I've sent in three talkbacks on this problem; two just now, with
  Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6a) Gecko/20030930

Updating product/component; no wonder it was hard to find this bug.
Component: Layout → Filters
Product: Browser → MailNews
-> Layout
Assignee: roc → other
Component: Filters → Layout
Product: MailNews → Browser
um....why?  Please help us understand the process of categorizing bugs to proper
areas.
I can confirm this immediate and repeatable crash on today's (20031001) nightly. 

G. Beker
Because this crash stack is in Layout, not Mail/News.
*** Bug 220947 has been marked as a duplicate of this bug. ***
*** Bug 221179 has been marked as a duplicate of this bug. ***
Blocks: 220629
*** Bug 219540 has been marked as a duplicate of this bug. ***
*** Bug 221214 has been marked as a duplicate of this bug. ***
It looks like a reentrancy problem in PresShell::HandlePostedReflowCallbacks().
Here is some trace output of the events (without calling FreeFrame so we
should not see two events with the same address):

0x883a918 PresShell::PostReflowCallback - add to empty list 0x88bafc0
0x883a918 PresShell::PostReflowCallback - append 0x88bafc8 as next on 0x88bafc0
0x883a918 IN PresShell::HandlePostedReflowCallbacks 28
      28:  mFirstCallbackEventRequest 0x88bafc0
      28:  mLastCallbackEventRequest 0x88bafc8
      28:  toFree=0x88bafc0
0x883a918 IN PresShell::HandlePostedReflowCallbacks 29
      29:  mFirstCallbackEventRequest 0x88bafc8
      29:  mLastCallbackEventRequest 0x88bafc8
      29:  toFree=0x88bafc8  <------------------------------------------------+
0x883a918 PresShell::PostReflowCallback - add to empty list 0x88a499c	      |
0x883a918 IN PresShell::HandlePostedReflowCallbacks 30			      |
      30:  mFirstCallbackEventRequest 0x88a499c				      |
      30:  mLastCallbackEventRequest 0x88a499c				      |
      30:  toFree=0x88a499c						      |
0x883a918 PresShell::PostReflowCallback - add to empty list 0x88a4b38	      |
0x883a918 IN PresShell::HandlePostedReflowCallbacks 31		            BAD
      31:  mFirstCallbackEventRequest 0x88a4b38				      |
      31:  mLastCallbackEventRequest 0x88a4b38				      |
      31:  toFree=0x88a4b38						      |
0x883a918 OUT PresShell::HandlePostedReflowCallbacks 31			      |
0x883a918 OUT PresShell::HandlePostedReflowCallbacks 30			      |
0x883a918 OUT PresShell::HandlePostedReflowCallbacks 29			      |
      28:  toFree=0x88bafc8  <------------------------------------------------+
0x883a918 PresShell::PostReflowCallback - add to empty list 0x8892294
0x883a918 PresShell::PostReflowCallback - append 0x88924e0 as next on 0x8892294
0x883a918 IN PresShell::HandlePostedReflowCallbacks 32
      32:  mFirstCallbackEventRequest 0x8892294
      32:  mLastCallbackEventRequest 0x88924e0
      32:  toFree=0x8892294
      32:  toFree=0x88924e0
0x883a918 PresShell::PostReflowCallback - add to empty list 0x889529c
0x883a918 IN PresShell::HandlePostedReflowCallbacks 33
      33:  mFirstCallbackEventRequest 0x889529c
      33:  mLastCallbackEventRequest 0x889529c
      33:  toFree=0x889529c
0x883a918 OUT PresShell::HandlePostedReflowCallbacks 33
0x883a918 OUT PresShell::HandlePostedReflowCallbacks 32
0x883a918 OUT PresShell::HandlePostedReflowCallbacks 28
Attached patch Patch rev. 1 (obsolete) — Splinter Review
o  Add a missing update of mLastCallbackEventRequest in CancelReflowCallback()
   in the case that the last event in the list is cancelled.
   (is not strictly necessary to fix this bug though)

o In HandlePostedReflowCallbacks():
  old code: walk the list doing callbacks, set mLast/FirstCallbackEventRequest
	    pointers to NULL only after the loop is done.
  new code: unlink the first event from the list and update the pointers
	    immediately, then do the callback.
Summary: crash in the Mailnews Filter Dialog → crash in the Mailnews Filter Dialog [@ PresShell::PostReflowCallback]
*** Bug 221341 has been marked as a duplicate of this bug. ***
Just to make sure, 1.6a does not miss it ... blocking1.6a?

pi
Flags: blocking1.6a?
Blocks: 221676
My 0.000000000000002 cents :

I applied the patch (Pacth Rev.1) on my current CVS based Thunderbird sources,
and no more crash while using filter UI.

I don't try with my mozilla trunk source.

I reproduce tests case in comment #7 & #8 and no crashes.

Thanks a lot :)
*NIX build 0.4a (20030930) : After applying patch ID 132708, crash apparently no
longer occurs ("apparently" since I haven't yet tried more than a few times so
far). At this point fix appears solid.
How about telling us non-technocrats how to apply the patch.  The bug really is
lousy to live with!

Beker
George: Your best bet would probably be to wait until the patch is approved and
checked-in. Then it'll be available in the usual nightly builds.
Comment on attachment 132708 [details] [diff] [review]
Patch rev. 1

David, could you review this one? (or suggest someone else...)
See comment 29 and comment 30 for a description of the problem/patch
Attachment #132708 - Flags: review?(dbaron)
Comment on attachment 132708 [details] [diff] [review]
Patch rev. 1

>    nsCallbackEventRequest* node = mFirstCallbackEventRequest;
>+   for (; node ; node = mFirstCallbackEventRequest) {
Code style nit:
while (mFirstCallbackEventRequest) {
  nsCallbackEventRequest* node = mFirstCallbackEventRequest;
Attached patch Patch rev. 2 (obsolete) — Splinter Review
Updated as suggested in comment 39
Attachment #132708 - Attachment is obsolete: true
Attachment #132708 - Flags: review?(dbaron)
Attachment #133238 - Flags: review?(dbaron)
The patch referred to in comment 36 is not something that can be downloaded
to replaced the faulty bits of your application with good ones, but a
concise description of the modifications to the sources of Mozilla, and
is useless unless you have a working build tree.

To get back a working lizard, you should download a recent nightly 
(say around 17th September) which does not have the bug, but does
have the features that you want.

You do not want a current build until there is the all clear on this bug.
Comment on attachment 133238 [details] [diff] [review]
Patch rev. 2

The change that fixes this bug looks fine.  However, I'm a little puzzled by
this part of CancelReflowCallback:

	nsCallbackEventRequest* toFree = node;
	if (node == mFirstCallbackEventRequest) {
	  mFirstCallbackEventRequest = node->next;
	  node = mFirstCallbackEventRequest;
	  before = nsnull;
	} else {
	  node = node->next;
	  before->next = node;
	}

In the then-clause of the if, the assignment to |node| doesn't change anything,
and seems wrong.  Furthermore, the assignment to |before| doesn't change
anything either.  Shouldn't this code be modified:

-	   node = mFirstCallbackEventRequest;
-	   before = nsnull;
+	   node = node->next;
+	   NS_ASSERTION(before == nsnull, "impossible");

or am I missing something?
Attachment #133238 - Flags: review?(dbaron) → review+
> the assignment to |node| doesn't change anything

You missed the assignment to |mFirstCallbackEventRequest|.
I can change it to |node = mFirstCallbackEventRequest = node->next;|
if you think that is more clear. (the else-clause have a similar construct).

> the assignment to |before| doesn't change anything either

Agreed, it can be removed.
QA Contact: ian → nobody
Attached patch Patch rev. 3 (obsolete) — Splinter Review
Replace the |before = nsnull| with assertion that it already is nsnull.
Attachment #133238 - Attachment is obsolete: true
Attachment #133298 - Flags: review?(dbaron)
Hmm, maybe
	  node = node->next;
	  mFirstCallbackEventRequest = node;
is more clear? (would make it look more like the else-clause too)
Attached patch Patch rev. 4Splinter Review
OK, so it is.
Attachment #133298 - Attachment is obsolete: true
Attachment #133298 - Flags: review?(dbaron)
Attachment #133300 - Flags: review?(dbaron)
->mats
Assignee: other → mats.palmgren
Fix checked in to trunk, 2003-10-14 16:29 -0700.
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Currently using 2003101504 on WinXP... does this fix appear in this nightly?

If so, I don't think this is fixed yet... either that, or I'm experiencing a
different bug...

Steps:
1. Open Mail.
2. Select a message.
3. Select [Message] > Create Filter from Message...
4. CRASH!

um is this bug about crashing when closing the Edit dialog? I crash when i click
on the Edit button or when Creating a filter from a message. If it's this bug,
please reopen, this still crashes with a cvs trunk build
I am also seeing this with 2003101504 trunk. Talkback ID TB24475642Q. In my case
it occurred upon pressing "edit" in the Message Filters window.
Mozilla 2003101604-trunk(mozilla-win32-talkback.zip) still crashed when I
pressed "Edit" button in Tools/Message Filters window.
TalkBackID : TB24475506H

I use "Modern Theme".
No problem on Mozilla 1.5 release buid.
People may be seeing bug 222468 
REOPEN, I see this too in 2003-10-16-05 on Linux when I click "Edit..." on an
existing mail filter (TB24486614G).  A stack from the talkback would be nice.
Status: RESOLVED → REOPENED
Keywords: stackwanted
Resolution: FIXED → ---
Whiteboard: TB24486614G
Mats, that's bug 222468.  This bug really is fixed...
OK, thanks for checking.

-> FIXED
Status: REOPENED → RESOLVED
Closed: 21 years ago21 years ago
Keywords: stackwanted
Resolution: --- → FIXED
Whiteboard: TB24486614G
verified with
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6a) Gecko/20031018
Status: RESOLVED → VERIFIED
*** Bug 227160 has been marked as a duplicate of this bug. ***
Is it really fixed or should it be reopened ?
This is fixed, as far as I know.  The filter dialog still crashes, but that's a
different bug...
Crash Signature: [@ PresShell::PostReflowCallback]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: