Closed Bug 66748 Opened 24 years ago Closed 22 years ago

Plugin's within frameset documents crash because of stale window handles

Categories

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

x86
Windows 2000
defect

Tracking

(Not tracked)

VERIFIED FIXED
mozilla1.0

People

(Reporter: peterlubczynski-bugs, Assigned: kmcclusk)

References

()

Details

(Keywords: crash, topembed+, Whiteboard: [adt1] [Needs a=])

Attachments

(2 files, 1 obsolete file)

To reproduce visit test URL with Quicktime plugin installed. Click to play and once the movie is playing, close the browser window. Notice the crash. Haven't tried on Mac yet. From the stack trace, looks like we are releasing held memory. NTDLL! 77f9eea9() nsView::~nsView() line 165 nsView::`scalar deleting destructor'(unsigned int 1) + 15 bytes nsView::Destroy(nsView * const 0x160f9418) line 263 + 34 bytes nsFrame::Destroy(nsFrame * const 0x16129b8c, nsIPresContext * 0x161584b0) line 425 nsContainerFrame::Destroy(nsContainerFrame * const 0x16129b8c, nsIPresContext * 0x161584b0) line 99 nsObjectFrame::Destroy(nsObjectFrame * const 0x16129b8c, nsIPresContext * 0x161584b0) line 412 nsLineBox::DeleteLineList(nsIPresContext * 0x161584b0, nsLineBox * 0x16129bd4) line 252 nsBlockFrame::Destroy(nsBlockFrame * const 0x16129a6c, nsIPresContext * 0x161584b0) line 1230 + 16 bytes nsFrameList::DestroyFrames(nsIPresContext * 0x161584b0) line 36 nsContainerFrame::Destroy(nsContainerFrame * const 0x16129a0c, nsIPresContext * 0x161584b0) line 98 nsFrameList::DestroyFrames(nsIPresContext * 0x161584b0) line 36 nsContainerFrame::Destroy(nsContainerFrame * const 0x161299bc, nsIPresContext * 0x161584b0) line 98 nsFrameList::DestroyFrames(nsIPresContext * 0x161584b0) line 36 nsContainerFrame::Destroy(nsContainerFrame * const 0x16129978, nsIPresContext * 0x161584b0) line 98 nsFrameList::DestroyFrames(nsIPresContext * 0x161584b0) line 36 nsContainerFrame::Destroy(nsContainerFrame * const 0x16129910, nsIPresContext * 0x161584b0) line 98 nsTableFrame::Destroy(nsTableFrame * const 0x16129910, nsIPresContext * 0x161584b0) line 259 nsFrameList::DestroyFrames(nsIPresContext * 0x161584b0) line 36 nsContainerFrame::Destroy(nsContainerFrame * const 0x161298bc, nsIPresContext * 0x161584b0) line 98 nsTableOuterFrame::Destroy(nsTableOuterFrame * const 0x161298bc, nsIPresContext * 0x161584b0) line 64 nsLineBox::DeleteLineList(nsIPresContext * 0x161584b0, nsLineBox * 0x16135144) line 252 nsBlockFrame::Destroy(nsBlockFrame * const 0x161297e4, nsIPresContext * 0x161584b0) line 1230 + 16 bytes nsLineBox::DeleteLineList(nsIPresContext * 0x161584b0, nsLineBox * 0x16129830) line 252 nsBlockFrame::Destroy(nsBlockFrame * const 0x1612975c, nsIPresContext * 0x161584b0) line 1230 + 16 bytes nsFrameList::DestroyFrames(nsIPresContext * 0x161584b0) line 36 nsContainerFrame::Destroy(nsContainerFrame * const 0x1620e26c, nsIPresContext * 0x161584b0) line 98 nsFrameList::DestroyFrames(nsIPresContext * 0x161584b0) line 36 nsContainerFrame::Destroy(nsContainerFrame * const 0x1620e34c, nsIPresContext * 0x161584b0) line 98 nsBoxFrame::Destroy(nsBoxFrame * const 0x1620e34c, nsIPresContext * 0x161584b0) line 1013 + 13 bytes nsFrameList::DestroyFrames(nsIPresContext * 0x161584b0) line 36 nsContainerFrame::Destroy(nsContainerFrame * const 0x1620e2a4, nsIPresContext * 0x161584b0) line 98 nsBoxFrame::Destroy(nsBoxFrame * const 0x1620e2a4, nsIPresContext * 0x161584b0) line 1013 + 13 bytes nsGfxScrollFrame::Destroy(nsGfxScrollFrame * const 0x1620e2a4, nsIPresContext * 0x161584b0) line 448 nsFrameList::DestroyFrames(nsIPresContext * 0x161584b0) line 36 nsContainerFrame::Destroy(nsContainerFrame * const 0x1620e230, nsIPresContext * 0x161584b0) line 98 ViewportFrame::Destroy(ViewportFrame * const 0x1620e230, nsIPresContext * 0x161584b0) line 142 FrameManager::~FrameManager() line 409 FrameManager::`scalar deleting destructor'(unsigned int 1) + 15 bytes FrameManager::Release(FrameManager * const 0x1620ac00) line 388 + 157 bytes PresShell::~PresShell() line 1324 + 36 bytes PresShell::`scalar deleting destructor'() + 15 bytes PresShell::Release(PresShell * const 0x162f5450) line 1234 + 158 bytes nsCOMPtr<nsIPresShell>::~nsCOMPtr<nsIPresShell>() line 489 DocumentViewerImpl::~DocumentViewerImpl() line 449 + 97 bytes DocumentViewerImpl::`scalar deleting destructor'(unsigned int 1) + 15 bytes DocumentViewerImpl::Release(DocumentViewerImpl * const 0x16302280) line 357 + 154 bytes nsCOMPtr<nsIContentViewer>::assign_assuming_AddRef(nsIContentViewer * 0x00000000) line 471 nsCOMPtr<nsIContentViewer>::assign_with_AddRef(nsISupports * 0x00000000) line 951 nsCOMPtr<nsIContentViewer>::operator=(nsIContentViewer * 0x00000000) line 583 nsDocShell::Destroy(nsDocShell * const 0x160d365c) line 1696 nsWebShell::Destroy(nsWebShell * const 0x160d365c) line 1446 nsHTMLFrameInnerFrame::~nsHTMLFrameInnerFrame() line 490 nsHTMLFrameInnerFrame::`scalar deleting destructor'(unsigned int 1) + 15 bytes nsFrame::Destroy(nsFrame * const 0x160be054, nsIPresContext * 0x1507da90) line 425 + 34 bytes nsFrameList::DestroyFrames(nsIPresContext * 0x1507da90) line 36 nsContainerFrame::Destroy(nsContainerFrame * const 0x16094734, nsIPresContext * 0x1507da90) line 98 nsFrameList::DestroyFrames(nsIPresContext * 0x1507da90) line 36 nsContainerFrame::Destroy(nsContainerFrame * const 0x160946a4, nsIPresContext * 0x1507da90) line 98 nsBoxFrame::Destroy(nsBoxFrame * const 0x160946a4, nsIPresContext * 0x1507da90) line 1013 + 13 bytes nsFrameList::DestroyFrames(nsIPresContext * 0x1507da90) line 36 nsContainerFrame::Destroy(nsContainerFrame * const 0x16094614, nsIPresContext * 0x1507da90) line 98 nsBoxFrame::Destroy(nsBoxFrame * const 0x16094614, nsIPresContext * 0x1507da90) line 1013 + 13 bytes nsFrameList::DestroyFrames(nsIPresContext * 0x1507da90) line 36 nsContainerFrame::Destroy(nsContainerFrame * const 0x16094584, nsIPresContext * 0x1507da90) line 98 nsBoxFrame::Destroy(nsBoxFrame * const 0x16094584, nsIPresContext * 0x1507da90) line 1013 + 13 bytes nsFrameList::DestroyFrames(nsIPresContext * 0x1507da90) line 36 nsContainerFrame::Destroy(nsContainerFrame * const 0x16093e3c, nsIPresContext * 0x1507da90) line 98 nsBoxFrame::Destroy(nsBoxFrame * const 0x16093e3c, nsIPresContext * 0x1507da90) line 1013 + 13 bytes nsFrameList::DestroyFrames(nsIPresContext * 0x1507da90) line 36 nsContainerFrame::Destroy(nsContainerFrame * const 0x16093dac, nsIPresContext * 0x1507da90) line 98 nsBoxFrame::Destroy(nsBoxFrame * const 0x16093dac, nsIPresContext * 0x1507da90) line 1013 + 13 bytes nsFrameList::DestroyFrames(nsIPresContext * 0x1507da90) line 36 nsContainerFrame::Destroy(nsContainerFrame * const 0x16093d70, nsIPresContext * 0x1507da90) line 98 ViewportFrame::Destroy(ViewportFrame * const 0x16093d70, nsIPresContext * 0x1507da90) line 142 FrameManager::~FrameManager() line 409 FrameManager::`scalar deleting destructor'(unsigned int 1) + 15 bytes FrameManager::Release(FrameManager * const 0x1514caa0) line 388 + 157 bytes PresShell::~PresShell() line 1324 + 36 bytes PresShell::`scalar deleting destructor'() + 15 bytes PresShell::Release(PresShell * const 0x150db7a8) line 1234 + 158 bytes nsCOMPtr<nsIPresShell>::~nsCOMPtr<nsIPresShell>() line 489 DocumentViewerImpl::~DocumentViewerImpl() line 449 + 97 bytes DocumentViewerImpl::`scalar deleting destructor'(unsigned int 1) + 15 bytes DocumentViewerImpl::Release(DocumentViewerImpl * const 0x00c81078) line 357 + 154 bytes nsCOMPtr<nsIContentViewer>::assign_assuming_AddRef(nsIContentViewer * 0x00000000) line 471 nsCOMPtr<nsIContentViewer>::assign_with_AddRef(nsISupports * 0x00000000) line 951 nsCOMPtr<nsIContentViewer>::operator=(nsIContentViewer * 0x00000000) line 583 nsDocShell::Destroy(nsDocShell * const 0x150bd8e4) line 1696 nsWebShell::Destroy(nsWebShell * const 0x150bd8e4) line 1446 nsXULWindow::Destroy(nsXULWindow * const 0x151256d4) line 330 nsWebShellWindow::Destroy(nsWebShellWindow * const 0x151256d4) line 1751 + 9 bytes nsWebShellWindow::Close(nsWebShellWindow * const 0x15125734) line 347 nsWebShellWindow::HandleEvent(nsGUIEvent * 0x0012f4f4) line 412 nsWindow::DispatchEvent(nsWindow * const 0x1504c4e4, nsGUIEvent * 0x0012f4f4, nsEventStatus & nsEventStatus_eIgnore) line 687 + 10 bytes nsWindow::DispatchWindowEvent(nsGUIEvent * 0x0012f4f4) line 708 nsWindow::DispatchStandardEvent(unsigned int 101) line 728 + 15 bytes nsWindow::ProcessMessage(unsigned int 16, unsigned int 0, long 0, long * 0x0012f858) line 2791 nsWindow::WindowProc(HWND__ * 0x0008022c, unsigned int 16, unsigned int 0, long 0) line 922 + 27 bytes USER32! 77e148dc() USER32! 77e163fb() USER32! 77e1643d() NTDLL! 77f9f04b() USER32! 77e15b59() USER32! 77e148dc() USER32! 77e17ef9() USER32! 77e17f75() nsWindow::WindowProc(HWND__ * 0x0008022c, unsigned int 274, unsigned int 61536, long 918657) line 929 + 31 bytes USER32! 77e148dc() USER32! 77e163fb() USER32! 77e1643d() NTDLL! 77f9f04b() USER32! 77e15b59() USER32! 77e148dc() USER32! 77e17ef9() USER32! 77e17f75() nsWindow::WindowProc(HWND__ * 0x0008022c, unsigned int 161, unsigned int 20, long 918657) line 929 + 31 bytes USER32! 77e148dc() USER32! 77e14aa7() USER32! 77e266fd() nsAppShellService::Run(nsAppShellService * const 0x00bb56b0) line 408 main1(int 2, char * * 0x004a7b18, nsISupports * 0x00000000) line 978 + 32 bytes main(int 2, char * * 0x004a7b18) line 1272 + 37 bytes mainCRTStartup() line 338 + 17 bytes KERNEL32! 77e992a6()
The view that is getting destructed and is causing the crash is the same that was constructed for the widget. We shouldn't be releasing it again? Oddly enough, this only happens when you close a window. Navigating to another site while playing does not cause this crash. CC:ing Kevin
Status: NEW → ASSIGNED
Keywords: crash
Peter it crashes inside the plugin on CallNPP_DestroyProc. I spent some time on this crash and could not find anything reasonable to do except intriducing a safety layer to all NPP_* calls. If this is what I think, don't rely on the stack trace, but rather put a breakpoint on CallNPP_DestroyProc in ns4xPluginInstance.cpp. If this is the same thing you should crash instantly on the next step.
I think this may have morphed since last you looked at it. I set the breakpoint in Stop right before CallNPP_DestroyProc in ns4xPluginInstance.cpp and it called it just fine, no problems. It set other breakpoints into other NPP function in that file and none were being called. Right after it calls DestoryProc, we go back to the ObjectFrame's Destroy and it try's to call it's super's destroy which eventually bubbles up to the crash in view. I took a close look at the addresses of the views being created and destroyed and it looks like we are trying to destroy the view created by the Widget a second time. But it's interesting why it only happens on close and not navigate. Setting milestone to 0.9.
Target Milestone: --- → mozilla0.9
My guess is that the viewmanager is being destroyed before the framemanager. The viewmanager must be destroyed after the framemanager. If the viewmanager is destroyed it will automatically destroy all of the views that it manages. If frames are holding references to these views, they will hold stale pointers. If the framemanager is destroyed first, the frames will destroy their associated views and the viewmanager will destroy any views not destroyed by the frames.
So Kevin, if I understand you, that means that when you close a browser window, the viewmanager gets destroyed before the framemanager. However, if you navigate to another page in that window, the framemanager will be destroyed first and the viewmanager cleans up stale views. So, to fix this crash, I should do something like check if the viewmanager has already been destroyed before attempting to destory the view for the OBJECT during frame destruction? What do you think about that plan?
We should always destroy the FrameManager before the viewmanager. If the viewmanager is being destroyed before the FrameManager we should determine why and fix that if possible. It may be some other problem, but in the past the only time I've seen stale view pointers is if the viewmanager was destroyed before the frame manager.
Attached file simplified testcase (deleted) —
Note: this only crashes in Seamonkey. It does not crash in Viewer or Winembed.
Ok, so the crash happens when we are trying to release mWindow upon destruction of the Widget: // Destroy and release the widget if (nsnull != mWindow) { mWindow->SetClientData(nsnull); mWindow->Destroy(); NS_RELEASE(mWindow); } At this point, mViewManager is null. Could it be that the plugin host is still holding on to the Widget?
This only happens in debug builds. Looks like the window handler was already destroyed and the breakpoint in the VERIFY macro in mWindow->Destory() causes it to appear like a crash. Does not happen in release builds. Doesn't look like a stale window handler will cause much future damage as it only happens on window close. Lowering severity, updating summary, and futuring.
Severity: normal → minor
Keywords: crash
Priority: -- → P4
Summary: Crash while closing window while Quicktime movie is playing → Stale window handler when a view gets destoryed while playing on close
Target Milestone: mozilla0.9 → Future
*** Bug 89847 has been marked as a duplicate of this bug. ***
I believe this bug is causing problems for the java plugin. After talking with Sun, they believe that having the plugin window destryed before the plugin's destroy() method is called is the root cause of bug #54725 and bug #117283. See Zhengyu Gu's comment: http://bugzilla.mozilla.org/show_bug.cgi?id=117283#c25 -- rick
Severity: minor → critical
Keywords: topembed
Priority: P4 → P1
Target Milestone: Future → mozilla1.0
Keywords: topembedcrash, topembed+
Blocks: 54725
-->1.1 Need to verify the FrameManager is destroyed before the viewmanager.
Target Milestone: mozilla1.0 → mozilla1.1alpha
*** Bug 115835 has been marked as a duplicate of this bug. ***
Attached patch patch v1 (obsolete) (deleted) — Splinter Review
delete the frame before destroying the view
Attachment #82701 - Attachment is obsolete: true
I think the problem is the nsHTMLFrameInnerFrame is destroying the frames and views it contains within its destructor instead of inside the Destroy method. The patch I attached moves the logic that was inside the nsHTMLFrameInnerFrame's destructor to it's Destroy method.
Comment on attachment 82709 [details] [diff] [review] Fix the crash by modifying nsHTMLFrameInnerFrame Good catch. r=jkeiser.
Attachment #82709 - Flags: review+
*** Bug 143099 has been marked as a duplicate of this bug. ***
Testing info: attachment (id=82709) - dogfood: I don't see any problems with the patch after using it today as dogfood. - Frameset test: I tested the patch by running: file:///S:/mozilla/dist/WIN32_D.OBJ/bin/res/samples/test9.html. This is the Gecko frameset/iframe test which destroys frames within a frameset when you click on links. - Crash reported in this bug. I also ran it against the crash reported in this bug and it no longer crashes.
Taking this bug
Assignee: peterlubczynski → kmcclusk
Status: ASSIGNED → NEW
Comment on attachment 82709 [details] [diff] [review] Fix the crash by modifying nsHTMLFrameInnerFrame sr=attinasi
Attachment #82709 - Flags: superreview+
Target Milestone: mozilla1.1alpha → mozilla1.0
Keywords: nsbeta1+
Whiteboard: [adt1]
I tested both WIN32 and Linux using a current trunk build with attachment 82709 [details] [diff] [review] applied. Fix (attachment 82709 [details] [diff] [review]) checked into the trunk.
adt1.0.0
Status: NEW → RESOLVED
Closed: 22 years ago
Keywords: adt1.0.0, approval
Resolution: --- → FIXED
Shrir - Can you verify this one on the trunk, and make sure there are no regressions. thanks!
Whiteboard: [adt1] → [adt1] [Needs a= & adt+]
checked on win,looks ok on trunk..
adding adt1.0.0+ for checkin to Mozilla 1.0 Branch. Please get drivers approval before checking in and then add the fixed1.0.0 keyword.
Keywords: adt1.0.0adt1.0.0+
Blocks: 143047
Whiteboard: [adt1] [Needs a= & adt+] → [adt1] [Needs a=]
Blocks: 145287
Summary: Stale window handler when a view gets destoryed while playing on close → Plugin's within frameset documents crash because of stale window handles
Blocks: 142810
Attachment #82709 - Flags: approval+
Comment on attachment 82709 [details] [diff] [review] Fix the crash by modifying nsHTMLFrameInnerFrame a=scc (on behalf of drivers) for checkin to the mozilla1.0 branch
I'm updating my Moz1.0.0 branch now. I should have this fix checked into the 1.0.0 branch around 4:30-5:00pm
I checked in the fix to the Moz1.0.0 branch.
Keywords: fixed1.0.0
*** Bug 120815 has been marked as a duplicate of this bug. ***
does not crash anymore.
Status: RESOLVED → VERIFIED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: