115698 - Crash often while processing Mac events when sounds are playing

Status: VERIFIED FIXED

(Core :: XUL, defect, P2)

Description

[Jean-Francois Ducarroz]

using commercial MacOS build id 2001121304, I crash often doing various think
like closing a browser window, typing in a msg compose window. Stack trace are
not always the same but they are all related to getting Mac events:

Date/Time:  2001-12-11 14:38:06 -0800
OS Version: 10.1.1 (Build 5M28)
Host:       h-198-93-95-246.mcom.com

Command:    Netscape 6
PID:        303

Exception:  EXC_BAD_ACCESS (0x0001)
Codes:      KERN_INVALID_ADDRESS (0x0001) at 0xdaeb0515

Thread 0 Crashed:
 #0   0x734e2d64 in InternalColor2Index
 #1   0x734ff81c in Color2IndexWithPort
 #2   0x73503920 in SetPortRGBForeColor
 #3   0x7350e1ec in RGBForeColor
 #4   0x73f96564 in TaskMovie_priv
 #5   0x73f9b02c in frequentlyTaskMovies
 #6   0x70196984 in __CFRunLoopDoTimer
 #7   0x7017c17c in __CFRunLoopRun
 #8   0x701b6ba0 in CFRunLoopRunSpecific
 #9   0x7017b804 in CFRunLoopRunInMode
 #10  0x7312d614 in RunEventLoopInModeUntilEventArrives
 #11  0x731c32d0 in RunEventLoopUntilEventArrives
 #12  0x731a1040 in GetNextEventMatchingMask
 #13  0x731adff0 in WNEInternal
 #14  0x731c5c60 in WaitNextEvent
 #15  0x0050aaf0 in 0x50aaf0
 #16  0x005045e8 in 0x5045e8
 #17  0x00506100 in 0x506100


**********

Date/Time:  2001-12-14 18:38:53 -0800
OS Version: 10.1.1 (Build 5M28)
Host:       h-198-93-95-246.mcom.com

Command:    Netscape 6
PID:        758

Exception:  EXC_BAD_ACCESS (0x0001)
Codes:      KERN_INVALID_ADDRESS (0x0001) at 0x2b206e75

Thread 0 Crashed:
 #0   0x734fa6a4 in OffsetRgn
 #1   0x734fc638 in SetOrigin
 #2   0x73f95f50 in TaskMovie_priv
 #3   0x73f9b02c in frequentlyTaskMovies
 #4   0x70196984 in __CFRunLoopDoTimer
 #5   0x7017c17c in __CFRunLoopRun
 #6   0x701b6ba0 in CFRunLoopRunSpecific
 #7   0x7017b804 in CFRunLoopRunInMode
 #8   0x7312d614 in RunEventLoopInModeUntilEventArrives
 #9   0x731404f8 in ReceiveNextEventCommon
 #10  0x732f2398 in _AcquireNextEvent
 #11  0x7310fe78 in IsUserStillTracking(MenuSelectData *, unsigned char *)
 #12  0x732d2b9c in TrackMenuCommon(MenuSelectData &, unsigned char *)
 #13  0x7316526c in MenuSelectCore(Point, double, unsigned long,
OpaqueMenuHandle **, unsigned short *)
 #14  0x73187b74 in MenuSelect
 #15  0x01d46560 in 0x1d46560
 #16  0x01d4623c in nsMacMessagePump::DispatchEvent(int, EventRecord *)
 #17  0x01d45f10 in nsMacMessagePump::DoMessagePump(void)
 #18  0x01d4584c in nsAppShell::Run(void)
 #19  0x01d05cec in nsAppShellService::Run(void)
 #20  0x004c9464 in main1(int, char **, nsISupports *)
 #21  0x004c9f6c in main

**********

Date/Time:  2001-12-15 08:29:48 -0800
OS Version: 10.1.1 (Build 5M28)
Host:       h-198-93-95-246.mcom.com

Command:    Netscape 6
PID:        878

Exception:  EXC_BAD_ACCESS (0x0001)
Codes:      KERN_INVALID_ADDRESS (0x0001) at 0xff5659d0

Thread 0 Crashed:
 #0   0x734fc644 in SetOrigin
 #1   0x73f95f50 in TaskMovie_priv
 #2   0x73f9b02c in frequentlyTaskMovies
 #3   0x70196984 in __CFRunLoopDoTimer
 #4   0x7017c17c in __CFRunLoopRun
 #5   0x701b6ba0 in CFRunLoopRunSpecific
 #6   0x7017b804 in CFRunLoopRunInMode
 #7   0x7312d614 in RunEventLoopInModeUntilEventArrives
 #8   0x731c32d0 in RunEventLoopUntilEventArrives
 #9   0x731a0f34 in GetNextEventMatchingMask
 #10  0x731adff0 in WNEInternal
 #11  0x731c5c60 in WaitNextEvent
 #12  0x01d46144 in nsMacMessagePump::GetEvent(EventRecord &)
 #13  0x01d45efc in nsMacMessagePump::DoMessagePump(void)
 #14  0x01d4584c in nsAppShell::Run(void)
 #15  0x01d05cec in nsAppShellService::Run(void)
 #16  0x004c9464 in main1(int, char **, nsISupports *)
 #17  0x004c9f6c in main

**********

Date/Time:  2001-12-15 13:26:12 -0800
OS Version: 10.1.1 (Build 5M28)
Host:       h-198-93-95-246.mcom.com

Command:    Netscape 6
PID:        991

Exception:  EXC_BAD_ACCESS (0x0001)
Codes:      KERN_INVALID_ADDRESS (0x0001) at 0x060d0134

Thread 0 Crashed:
 #0   0x01d340b8 in nsMacMemoryCushion::RecoverMemoryReserve(long)
 #1   0x0582ef50 in 0x582ef50
 #2   0x01d34018 in nsMacMemoryCushion::RepeatAction(EventRecord const &)
 #3   0x010d4b14 in Repeater::DoRepeaters(EventRecord const &)
 #4   0x01d46338 in nsMacMessagePump::DispatchEvent(int, EventRecord *)
 #5   0x01d45f10 in nsMacMessagePump::DoMessagePump(void)
 #6   0x01d4584c in nsAppShell::Run(void)
 #7   0x01d05cec in nsAppShellService::Run(void)
 #8   0x004c9464 in main1(int, char **, nsISupports *)
 #9   0x004c9f6c in main

**********

Date/Time:  2001-12-17 15:17:31 -0800
OS Version: 10.1.1 (Build 5M28)
Host:       h-198-93-95-246.mcom.com

Command:    Netscape 6
PID:        306

Exception:  EXC_BAD_ACCESS (0x0001)
Codes:      KERN_INVALID_ADDRESS (0x0001) at 0x458b032e

Thread 0 Crashed:
 #0   0x734e2d64 in InternalColor2Index
 #1   0x734ff81c in Color2IndexWithPort
 #2   0x73503920 in SetPortRGBForeColor
 #3   0x7350e1ec in RGBForeColor
 #4   0x73f96564 in TaskMovie_priv
 #5   0x73f9b02c in frequentlyTaskMovies
 #6   0x70196984 in __CFRunLoopDoTimer
 #7   0x7017c17c in __CFRunLoopRun
 #8   0x701b6ba0 in CFRunLoopRunSpecific
 #9   0x7017b804 in CFRunLoopRunInMode
 #10  0x7312d614 in RunEventLoopInModeUntilEventArrives
 #11  0x731c32d0 in RunEventLoopUntilEventArrives
 #12  0x731a0f34 in GetNextEventMatchingMask
 #13  0x731233bc in EventAvail
 #14  0x01d00088 in nsMacMessagePump::GetEvent(EventRecord &)
 #15  0x01cffefc in nsMacMessagePump::DoMessagePump(void)
 #16  0x01cff84c in nsAppShell::Run(void)
 #17  0x01cbfcec in nsAppShellService::Run(void)
 #18  0x004c9464 in main1(int, char **, nsISupports *)
 #19  0x004c9f6c in main

**********

Comment 1

[Jean-Francois Ducarroz]

Nominating nsbeta1. I don't think hyatt is the right owner for this one! cc'ing
pinkerton and saari

Comment 2

[Mike Pinkerton (not reading bugmail)]

looks like corrupted memory or a corrupted grafport somewhere. we have another
bug with similar stack traces.

Comment 3

[Peter Trudelle]

nsbeta1+ per Nav triage team. ->jag

Comment 4

[Simon Fraser [no longer active]]

Taking this. The TaskMovie crashes need a fix. The 
nsMacMemoryCushion::RecoverMemoryReserve() crash has already been fixed.

Comment 5

[scalkins]

This is dogfood for IM.

Comment 6

[scalkins]

Per bugscape bug http://bugscape.netscape.com/show_bug.cgi?id=10884, this is
also on Mac OS9.1

Comment 7

[Tracy Walker [:tracy]]

this isn't just crash the app on mac os9, it's forcing a system restart :-(
the crash I'm seeing is from bugscape bug 11236 which was closed in favor of 
this bug.

Comment 8

[Simon Fraser [no longer active]]

Attachment: Patch to nsSound.cpp

This patch fixes the problem, and a number of side issues.

The real problem here is that QuickTime movies are attached to a GrafPort. When
we import sounds, we use the current grafport (which will be the current
window, usually). When we close that window, QuickTime gets rather unhappy, and
crashes.
So the fix is to make a small permanent GWorld which the movies live in, which
is owned by a stack-based class. This fixes the crash.

There were a couple of other problems. Firstly, when we started a timer to
process a playing movie, the timer never stopped. This happened because both
the nsSoundRequest base class, and the nsMovieSoundRequest class had mSound and
mTimer members, so we failed to shut down the timer.

Second, the loop in TaskActiveMovies() had some dubious logic, and could walk
off the end of the nsVoidArray. Fixed.

Comment 9

[Simon Fraser [no longer active]]

Attachment: Patch without tabs

Comment 10

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 75522 [details] [diff] [review]
Patch without tabs

r=pink

Comment 11

[Scott Collins]

Comment on attachment 75522 [details] [diff] [review]
Patch without tabs

well, the only problem I see with this is that it's written as though it's
re-entrant, but it's not.  Anyone can come along and instantiate another
|nsMoviePortOwner| (well ... at least, anyone in this file) which will
immediately clear the one and only |nsMoviePortOwner::sMoviePort|.  I don't
think you can totally resolve this issue.  But you should (a) put a comment
that explicitly notes the class is not re-entrant.  (b), initialize
|nsMoviePortOwner::sMoviePort| in its solo declaration below the class, rather
than in the constructor.  Rather than having a public static instance, have an
a local function at file scope that contains the static |nsMoviePortOwner|,
this function is a |friend|, and |nsMoviePortOwner|'
s constructor becomes private.	That will ensure no one ever accidentally leaks
our |GWorldPtr| by constructing a second instance.  In fact, your file scope
function can not only declare the static instance, it can directly return
|.GetMoviePort| on the static instance.  Perhaps this function would be named,
e.g., |GetSingletonMoviePort|, or |GetTheMoviePort|.  Since it's a friend, you
could actually simplify your class (and callers).

Comment 12

[Patrick C. Beard]

Comment on attachment 75522 [details] [diff] [review]
Patch without tabs

I would inline all of EnsureMoviePort() in GetMoviePort(), or at least inline
the  !sMoviePort test. Otherwise, sr=beard

Comment 13

[Simon Fraser [no longer active]]

I'll get a new patch addressing scc's comments.

Comment 14

[Simon Fraser [no longer active]]

Attachment: Revised nsSound patch addressing scc's comments

Comment 15

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 75641 [details] [diff] [review]
Revised nsSound patch addressing scc's comments

r=pink

Comment 16

[Scott Collins]

Comment on attachment 75641 [details] [diff] [review]
Revised nsSound patch addressing scc's comments

Excellent.  sr=scc.  For extra credit you could even make
|nsMoviePortOwner::sMoviePort| not be |static| any more, since the function
does it for you by making the containing instance static.  Thus, your extra
declaration is redundant, but not harmfully so.  Check in as it is, or if you
make a new patch with that change, feel free to transfer my sr to it.

Comment 17

[Scott Collins]

Comment on attachment 75641 [details] [diff] [review]
Revised nsSound patch addressing scc's comments

a=scc

Comment 18

[Simon Fraser [no longer active]]

Fix checked in.

Comment 19

[Tracy Walker [:tracy]]

marking verified. I haven't seen this crashed from this in a couple months