214176 - Crash Opening Mail/News Component

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[jay garcia]

Win98SE Build 2003072804 1.5b Trunk

Opening mail/news causes crash in KERNEL32.DLL

100% reproducible

Comment 1

[jay garcia]

talkback ID=TB22263310Z

Comment 2

[Mitch]

I get a hang here on Solaris with today's CVS build when opening the Mail/News
component. 

Rebuilding with Debug on and will provide a stack asap.

Comment 3

[jay garcia]

Win98SE PC Stack Trace

http://www.ufaq.org/mailnewscrash.txt

Comment 4

[Mitch]

Attachment: Stack from a crash on Solaris

Here's a nicer UNIX stack. Looks like some sort of memory freeing that's
perhaps not allocated (or double free ?)

Note, jsscope.c was checked in by brendan onthe 26th so some
possibility to be the cause of this.

Comment 5

[Mitch]

shouldn't jsscope.c:339 be

    nbytes = newsize * sizeof(JSScopeProperty *);

Comment 6

[Mitch]

I recompiled with my recommended fix and it seems to be working now.

Brendan can you confirm/deny the fix pls.

Comment 7

[Brendan Eich [:brendan]]

The stack indicates heap corruption, not the bug Mitch found (thanks!) where the
new table is over-large.

I'm fixing the newsize computation bug Mitch found, but I wonder if there isn't
still an impurity somewhere (not clear where) that various malloc callers will
still trip over.

/be

Comment 8

[Jim Booth]

Lost all my email settings mailboxes, and browser bookmarks.  
Talkback IDs TB22269547E, TB22269532H.

Marking DATALOSS and elevating to Blocker

Comment 9

[Jim Booth]

I take that back.  Inadvertently changed profile when restarting, panicked and
thought I'd lost all my settings.  No dataloss, but DOES crash 100% of the time.
 I'll leave the blocker.

Comment 10

[Brendan Eich [:brendan]]

Attachment: fix for the jsscope.c overlarge allocation bug

But again, this fix seems to me only to hide the impurity, if it makes the
crash go away.	Without this fix, scope tables that grow or shrink are
oversized by a factor of seven.  It seems as though that changes the layout of
memory enough to cause (or to make more likely) the crash.

/be

Comment 11

[Brendan Eich [:brendan]]

Comment on attachment 128731 [details] [diff] [review]
fix for the jsscope.c overlarge allocation bug

This is a non-minimal change.  I wondered why I wasn't using the
SCOPE_TABLE_SIZE macro, and noticed it wasn't used anywhere.  I renamed it and
used it to scale consistently throughout.

/be

Comment 12

[Brendan Eich [:brendan]]

The followup change to jsscript.c for bug 208030, now backed out in its
essential part, may be the cause of this impurity being exposed.  Or it may have
been the cause of the impurity, period.

If someone can retest with an up to date js/src/jsscript.c and verify that the
bug here can't be reproduced, with or without my patch attachment 128731 [details] [diff] [review], then I
would like to close this bug as a dup of 208030.

But I'd like to get the patch in -- Mitch found a real footprint bloat bug, due
to a stupid almost-typo (the sizeof(JSScopeProperty) rather than
sizeof(JSScopeProperty *) dates from an old version of the jsscope.c code that
didn't have a property tree in which to share JSScopeProperty structs, pointed
at by many scope hash table entries).

/be

Comment 13

[Mitch]

Brendan, looks fine to me. I can't retest till tomorrow (since i'm at home)
on Solaris, but seems to do the right thing by using the macro.

Comment 14

[Brendan Eich [:brendan]]

Checked in.  I think the crash was a dup of 208030, but I'm just going to close
this, since it did turn up a different bug and capture a reviewed patch.

/be

Comment 15

[Brendan Eich [:brendan]]

Proposing for 1.4.1.

/be

Comment 16

[Brendan Eich [:brendan]]

Comment on attachment 128731 [details] [diff] [review]
fix for the jsscope.c overlarge allocation bug

This patch applies with some hunk offset slop to 1.4.1.  I have an exact patch
created via cvs up -j that I'll check in, if it is approved.

/be

Comment 17

[Asa Dotzler [:asa]]

Comment on attachment 128731 [details] [diff] [review]
fix for the jsscope.c overlarge allocation bug

a=asa (on behalf of drivers) for checkin to the  1.4 branch.

Comment 18

[Brendan Eich [:brendan]]

Fixed on the 1.4 branch.

/be