131626 - Crash occurs after closing js window containing flash animation

Status: VERIFIED FIXED

(Core Graveyard :: Plug-ins, defect, P1)

Description

[Chris Petersen]

Build: 2002-03-16-08
Platform: OS X 
Expected Results: Window should close
What I got : The window does close but application crashes

Steps to reproduce:

1) Go to http://www.randomhouse.com/seussville/

2) Click on character to open JS window

3) As the "loading" content appears ( and audio plays), click the close to js
window.

4) If this doesn't result in a crash, try step two and three again. It should
crash after closing this window.

**********

Date/Time:  2002-03-17 16:56:43 -0800
OS Version: 10.1.3 (Build 5Q45)


Command:    Mozilla
PID:        659

Exception:  EXC_BAD_ACCESS (0x0001)
Codes:      KERN_PROTECTION_FAILURE (0x0002) at 0x00000000

Thread 0 Crashed:
 #0   0x040dee20 in 0x40dee20
 #1   0x040dee18 in 0x40dee18
 #2   0x0400ecf8 in 0x400ecf8
 #3   0x0400e8c0 in 0x400e8c0
 #4   0x026b1210 in ns4xPluginStreamListener::CallURLNotify(short)
 #5   0x026b1110 in ns4xPluginStreamListener::CleanUpStream(short)
 #6   0x026b25ac in ns4xPluginInstance::Stop(void)
 #7   0x0288d430 in nsObjectFrame::Destroy(nsIPresContext *)
 #8   0x0292ab64 in nsFrameList::DestroyFrames(nsIPresContext *)
 #9   0x02816378 in nsContainerFrame::Destroy(nsIPresContext *)
 #10  0x028e7bd4 in nsLineBox::DeleteLineList(nsIPresContext *, nsLineList &)
 #11  0x028b7fac in nsBlockFrame::Destroy(nsIPresContext *)
 #12  0x028e7bd4 in nsLineBox::DeleteLineList(nsIPresContext *, nsLineList &)
 #13  0x028b7fac in nsBlockFrame::Destroy(nsIPresContext *)
 #14  0x028eb05c in nsAreaFrame::Destroy(nsIPresContext *)
 #15  0x0292ab64 in nsFrameList::DestroyFrames(nsIPresContext *)
 #16  0x02816378 in nsContainerFrame::Destroy(nsIPresContext *)
 #17  0x0292ab64 in nsFrameList::DestroyFrames(nsIPresContext *)
 #18  0x02816378 in nsContainerFrame::Destroy(nsIPresContext *)
 #19  0x029309cc in nsBoxFrame::Destroy(nsIPresContext *)
 #20  0x0292ab64 in nsFrameList::DestroyFrames(nsIPresContext *)
 #21  0x02816378 in nsContainerFrame::Destroy(nsIPresContext *)
 #22  0x029309cc in nsBoxFrame::Destroy(nsIPresContext *)
 #23  0x02961900 in nsGfxScrollFrame::Destroy(nsIPresContext *)
 #24  0x0292ab64 in nsFrameList::DestroyFrames(nsIPresContext *)
 #25  0x02816378 in nsContainerFrame::Destroy(nsIPresContext *)
 #26  0x028fef00 in ViewportFrame::Destroy(nsIPresContext *)
 #27  0x02957c8c in FrameManager::Destroy(void)
 #28  0x0282d394 in PresShell::Destroy(void)
 #29  0x020a25d4 in DocumentViewerImpl::Destroy(void)
 #30  0x02556af0 in nsDocShell::Destroy(void)
 #31  0x02578708 in nsWebShell::Destroy(void)
 #32  0x029bed10 in nsHTMLFrameInnerFrame::_dt(void)
 #33  0x0281901c in nsFrame::Destroy(nsIPresContext *)
 #34  0x0292ab64 in nsFrameList::DestroyFrames(nsIPresContext *)
 #35  0x02816378 in nsContainerFrame::Destroy(nsIPresContext *)
 #36  0x0292ab64 in nsFrameList::DestroyFrames(nsIPresContext *)
 #37  0x02816378 in nsContainerFrame::Destroy(nsIPresContext *)
 #38  0x029309cc in nsBoxFrame::Destroy(nsIPresContext *)
 #39  0x0292ab64 in nsFrameList::DestroyFrames(nsIPresContext *)
 #40  0x02816378 in nsContainerFrame::Destroy(nsIPresContext *)
 #41  0x029309cc in nsBoxFrame::Destroy(nsIPresContext *)
 #42  0x0292ab64 in nsFrameList::DestroyFrames(nsIPresContext *)
 #43  0x02816378 in nsContainerFrame::Destroy(nsIPresContext *)
 #44  0x029309cc in nsBoxFrame::Destroy(nsIPresContext *)
 #45  0x0292ab64 in nsFrameList::DestroyFrames(nsIPresContext *)
 #46  0x02816378 in nsContainerFrame::Destroy(nsIPresContext *)
 #47  0x029309cc in nsBoxFrame::Destroy(nsIPresContext *)
 #48  0x0292ab64 in nsFrameList::DestroyFrames(nsIPresContext *)
 #49  0x02816378 in nsContainerFrame::Destroy(nsIPresContext *)
 #50  0x029309cc in nsBoxFrame::Destroy(nsIPresContext *)
 #51  0x0292ab64 in nsFrameList::DestroyFrames(nsIPresContext *)
 #52  0x02816378 in nsContainerFrame::Destroy(nsIPresContext *)
 #53  0x029309cc in nsBoxFrame::Destroy(nsIPresContext *)
 #54  0x0292ab64 in nsFrameList::DestroyFrames(nsIPresContext *)
 #55  0x02816378 in nsContainerFrame::Destroy(nsIPresContext *)
 #56  0x029309cc in nsBoxFrame::Destroy(nsIPresContext *)
 #57  0x0292ab64 in nsFrameList::DestroyFrames(nsIPresContext *)
 #58  0x02816378 in nsContainerFrame::Destroy(nsIPresContext *)
 #59  0x029309cc in nsBoxFrame::Destroy(nsIPresContext *)
 #60  0x0292ab64 in nsFrameList::DestroyFrames(nsIPresContext *)
 #61  0x02816378 in nsContainerFrame::Destroy(nsIPresContext *)
 #62  0x028fef00 in ViewportFrame::Destroy(nsIPresContext *)
 #63  0x02957c8c in FrameManager::Destroy(void)
 #64  0x0282d394 in PresShell::Destroy(void)
 #65  0x020a25d4 in DocumentViewerImpl::Destroy(void)
 #66  0x02556af0 in nsDocShell::Destroy(void)
 #67  0x02578708 in nsWebShell::Destroy(void)
 #68  0x023daf7c in nsXULWindow::Destroy(void)
 #69  0x023c73b0 in nsWebShellWindow::Destroy(void)
 #70  0x023c33c8 in nsWebShellWindow::Close(void)
 #71  0x023c3680 in 0x23c3680
 #72  0x02401930 in nsWindow::DispatchEvent(nsGUIEvent *, nsEventStatus &)
 #73  0x02401a0c in nsWindow::DispatchWindowEvent(nsGUIEvent &)
 #74  0x02411910 in nsMacEventDispatchHandler::DispatchGuiEvent(nsWindow *,
unsigned int)
 #75  0x02413a2c in nsMacEventHandler::HandleMouseDownEvent(EventRecord &)
 #76  0x0241218c in nsMacEventHandler::HandleOSEvent(EventRecord &)
 #77  0x02411084 in nsMacWindow::DispatchEvent(void *, int *)
 #78  0x02418ce0 in DispatchOSEventToRaptor__16nsMacMessagePumpFR11EventRecordP15O
 #79  0x02418554 in nsMacMessagePump::DoMouseDown(EventRecord &)
 #80  0x02417a1c in nsMacMessagePump::DispatchEvent(int, EventRecord *)
 #81  0x024176e0 in nsMacMessagePump::DoMessagePump(void)
 #82  0x0241705c in nsAppShell::Run(void)
 #83  0x023ca2fc in nsAppShellService::Run(void)
 #84  0x004cc224 in main1(int, char **, nsISupports *)
 #85  0x004cccfc in main

Thread 1:
 #0   0x7000497c in syscall
 #1   0x70557600 in BSD_waitevent
 #2   0x70554b80 in CarbonSelectThreadFunc
 #3   0x7002054c in _pthread_body

Thread 2:
 #0   0x7003f4c8 in semaphore_wait_signal_trap
 #1   0x7003f2c8 in _pthread_cond_wait
 #2   0x705593ec in CarbonOperationThreadFunc
 #3   0x7002054c in _pthread_body

Thread 3:
 #0   0x70044cf8 in semaphore_timedwait_signal_trap
 #1   0x70044cd8 in semaphore_timedwait_signal
 #2   0x70283ea4 in TSWaitOnConditionTimedRelative
 #3   0x7027d748 in TSWaitOnSemaphoreCommon
 #4   0x702c2078 in TimerThread
 #5   0x7002054c in _pthread_body

Thread 4:
 #0   0x7003f4c8 in semaphore_wait_signal_trap
 #1   0x7003f2c8 in _pthread_cond_wait
 #2   0x70250ab0 in TSWaitOnCondition
 #3   0x7027d730 in TSWaitOnSemaphoreCommon
 #4   0x70243d14 in AsyncFileThread
 #5   0x7002054c in _pthread_body

Thread 5:
 #0   0x7003f4c8 in semaphore_wait_signal_trap
 #1   0x7003f2c8 in _pthread_cond_wait
 #2   0x7055b884 in CarbonInetOperThreadFunc
 #3   0x7002054c in _pthread_body

Thread 6:
 #0   0x70000978 in mach_msg_overwrite_trap
 #1   0x70005a04 in mach_msg
 #2   0x7017bf98 in __CFRunLoopRun
 #3   0x701b7100 in CFRunLoopRunSpecific
 #4   0x7017b8e0 in CFRunLoopRunInMode
 #5   0x7061be08 in
XIOAudioDeviceManager::NotificationThread(XIOAudioDeviceManager *)
 #6   0x706141c0 in CAPThread::Entry(CAPThread *)
 #7   0x7002054c in _pthread_body

Thread 7:
 #0   0x70000978 in mach_msg_overwrite_trap
 #1   0x70005a04 in mach_msg
 #2   0x70026a2c in _pthread_become_available
 #3   0x70026724 in pthread_exit
 #4   0x70020550 in _pthread_body

PPC Thread State:
  srr0: 0x040dee20 srr1: 0x0200f030                vrsave: 0x00000000
   xer: 0x2000000e   lr: 0x040dee18  ctr: 0x0400e8b0   mq: 0x00000000
    r0: 0x040dee18   r1: 0xbfffd880   r2: 0x0470b000   r3: 0x0000006a
    r4: 0x00000000   r5: 0x00000002   r6: 0x00000003   r7: 0x03f1e90c
    r8: 0x00000001   r9: 0x02a11a18  r10: 0x162f6b61  r11: 0x00000000
   r12: 0x04705684  r13: 0x00000000  r14: 0x00000000  r15: 0x00000000
   r16: 0x00000000  r17: 0x00000000  r18: 0x00000000  r19: 0x00000000
   r20: 0x00000000  r21: 0x02459770  r22: 0x00000000  r23: 0x0422773c
   r24: 0x00000001  r25: 0x02a199d8  r26: 0x02a8ab80  r27: 0x02a8abf8
   r28: 0x00000000  r29: 0x0000006a  r30: 0x04706e91  r31: 0x00000000

**********

Comment 1

[Chris Petersen]

I haven't tried with OS 9 so I can't say it just a OS X issue. However, I can't
reproduce this crash on IE 5.1.3 (OS X) or NS 6.2.1(0S X). I have the latest
Flash plug-in installed: Shockwave Flash 6.0 r21.

Comment 2

[Chris Petersen]

Attachment: Placed site's function into test case

Clicking on text references the function popper() which will create a js window
containing the flash animation.

Comment 3

[rubydoo123]

using flash 5.0 and this does not crash, upgraded flash to the newest version
from Macromedia and it does crash. This does not crash on win32. I tested on Mac
OSX and not on 9. My stack is a bit differnt than the one Chris posted. I will
attach mine for you

Comment 4

[rubydoo123]

Attachment: stacktrace

Attached the stacktrace since it is different than the one Chris dropped in

Comment 5

[Winnie Lam]

adding ADT2 to status whiteboard as per discussion with beppe.  this only
happens to mac users and the newest version of the plugin.

Comment 6

[Peter Lubczynski]

Okay, I see why this is happening. We are passing the wrong URL in URLNotify:
http://developer.netscape.com/docs/manuals/communicator/plugin/pgfn2.htm#1007593

According to the old spec, we should pass the same URL given to us by
GetURLNotify and PostURLNotify. Instead, we are passing the URL given us by
NPP_NewStream. Flash 6 is crashing in this case because the URL we pass is null.
The URL is null because NPP_NewStream is not called before the user closed the
window (and hence terminates the plugin and all its streams).

Even though this crash is only seen on Mac with Flash 6, it might show up with
other plugins on other platforms as we are not doing what 4.x did.

Comment 7

[Peter Lubczynski]

Attachment: possible patch?

Here's a possible patch to send the correct url to URLNotify from GetURLNotify
and PostURLNotify.

Comment 8

[Peter Lubczynski]

Attachment: same patch - minus some tabs

Comment 9

[av (gone)]

Comment on attachment 76842 [details] [diff] [review]
same patch - minus some tabs

Looks like it will make it up to the spec. Couple of questions though:
- why relative url?
- why is stream url null?

Comment 10

[Peter Lubczynski]

Thanks Andrei. To answer your questions:

1) It maybe relative, but that doesn't matter because it's allocated by the
plugin. We need to pass the plugin the same pointer in URLNotify according to
the spec (probably so that it can be free'ed).
2) The stream url is null because NewStream was never called and that's where it
gets set. NewStream is never called because the plugin terminates before the
results of the request can come back. Because NewStream may not always be
called, it makes even more sense to use the URL from GetURLNotify/PostURLNotify.

Patrick, can I get a super review?

Comment 11

[Patrick C. Beard]

Comment on attachment 76842 [details] [diff] [review]
same patch - minus some tabs

Who owns the memory for the const char* aURL parameter to the
ns4xPluginStreamListener constructor?

Comment 12

[Peter Lubczynski]

Comment on attachment 76842 [details] [diff] [review]
same patch - minus some tabs

The plugin owns the memory. It is passed in as a const char * in
GetURLNotify/PostURLNotify:
http://developer.netscape.com/docs/manuals/communicator/plugin/pgfn2.htm#100766
8

Comment 13

[Patrick C. Beard]

Comment on attachment 76842 [details] [diff] [review]
same patch - minus some tabs

sr=beard

Comment 14

[Peter Lubczynski]

Attachment: patch v.3

It is safer if we own the URL.

Comment 15

[Peter Lubczynski]

Attachment: patch v.4

oops, wrong patch :-)

Comment 16

[serge (gone)]

Maybe I'm wrong but it looks to me  like we're going to keep 2 identical url 
strings 
per each ns4xPluginStreamListener object
1) mNPStream.url 
2) mNotifyURL

Comment 17

[Peter Lubczynski]

No, I don't think they are the same. mNPStream.url gets set with the URL from
the channel when the request comes back. This bug happens because URLNotify
needs to be called before the request comes back and therefore mNPStream.url is
null and we crash. Because one comes from the channel and the other comes from
the plugin, they may not be the same.

Comment 18

[Patrick C. Beard]

Comment on attachment 77188 [details] [diff] [review]
patch v.4

sr=beard
Glad I asked about that ownership issue.

Comment 19

[serge (gone)]

I see, ns4xPluginStreamListener.mNPStream.url  is a pointer to 
nsPluginStreamInfo.mURL
and last one we free/strdup on any ODA call, by  
mPluginStreamInfo->SetURL(urlString.get());
What is the reason to do so? I do not think there is a chance channel url can 
change between 2 consecutive ODA calls. 
Personally I don't like this cross objects pointer idea,
e.g. it's not a thread safe, if plugin caches mNPStream.url ptr, and on its own 
thread will try to access it after we free it, something bad could happen.
Shall we open new bug on it or we can fix it here?
Though NPP_NewStream() spec only says "The NPStream* pointer is valid until the 
stream is destroyed."

>they may not be the same
yep, e.g. in case of redirection.

Comment 20

[Peter Lubczynski]

With the crash in this bug we don't get any ODA's calls. |SetURL| is not called
in this case, but we should open another bug on the multiple calls.

Comment 21

[av (gone)]

Comment on attachment 77188 [details] [diff] [review]
patch v.4

r=av

Comment 22

[Asa Dotzler [:asa]]

Comment on attachment 77188 [details] [diff] [review]
patch v.4

a=asa (on behalf of drivers) for checkin to the 1.0 trunk

Comment 23

[Jaime Rodriguez, Jr.]

adt1.0.0+ (on ADT's behalf) approval for checkin to 1.0.

Comment 24

[Peter Lubczynski]

Patch in trunk (before branching), marking FIXED.

Comment 25

[shrirang khanzode]

no crash on 2002040411 on os x . verif.