Closed Bug 51174 Opened 24 years ago Closed 23 years ago

crash after having seen a page whith plug-in "mod plug player"

Categories

(Core :: XUL, defect, P1)

x86
Windows 95
defect

Tracking

()

RESOLVED WORKSFORME
Future

People

(Reporter: qtaz, Assigned: serhunt)

References

()

Details

(Keywords: crash)

when i go to a page wich is using the plug-in to listen mod file (xm,s3m,it etc)
and then, i load an other page, mozilla crashes
Adding crash keyword...
Keywords: crash
i missed how to reproduce this bug
o got only my page for example (sorry)
so 
1. go to http://qtaz.ctw.net
2. cclick on mod (left menu)
3. if you dont have th eplug-in, install it
4. choose a song and let mozilla play it
5. now cliwk on an other link at the left menu, and mozilla crashes
reporter - what build id do you have and what platform/os are you on?
i'm on windows95 osr2
128ram, pentium 200
my build is the nightly build of the 1/9 (2000091110)
over to plugins.  reporter can you please test this with a talkback build and
report the Incident ID of the report it generates.  THanks
Assignee: asa → av
Component: Browser-General → Plug-ins
QA Contact: doronr → shrir
Dereferencing null pointer in nsWindow, no plugin stuff is involved, reassigning 
to Browser-general for finding a right owner. Confirming.

Here is the stack trace:

nsWindow::WindowProc(HWND__ * 0x011a043a, unsigned int 130, unsigned int 0, long 
0) line 869 + 3 bytes
USER32! 77e71ab7()
USER32! 77e71a77()
NTDLL! 77f7624f()
nsView::~nsView() line 166
nsView::`scalar deleting destructor'(unsigned int 1) + 15 bytes
nsView::Destroy(nsView * const 0x0334a3a0) line 264 + 34 bytes
nsFrame::Destroy(nsFrame * const 0x02d800f0, nsIPresContext * 0x036c9730) line 
424
nsFrameList::DestroyFrames(nsIPresContext * 0x036c9730) line 36
nsContainerFrame::Destroy(nsContainerFrame * const 0x02d80080, nsIPresContext * 
0x036c9730) line 98
nsFrameList::DestroyFrames(nsIPresContext * 0x036c9730) line 36
nsContainerFrame::Destroy(nsContainerFrame * const 0x02d7ff64, nsIPresContext * 
0x036c9730) line 98
nsFrameList::DestroyFrames(nsIPresContext * 0x036c9730) line 36
nsContainerFrame::Destroy(nsContainerFrame * const 0x02d7fe2c, nsIPresContext * 
0x036c9730) line 98
nsLineBox::DeleteLineList(nsIPresContext * 0x036c9730, nsLineBox * 0x02d7fed8) 
line 252
nsBlockFrame::Destroy(nsBlockFrame * const 0x02d7fda4, nsIPresContext * 
0x036c9730) line 1213 + 16 bytes
nsFrameList::DestroyFrames(nsIPresContext * 0x036c9730) line 36
nsContainerFrame::Destroy(nsContainerFrame * const 0x02d7fd6c, nsIPresContext * 
0x036c9730) line 98
nsFrameList::DestroyFrames(nsIPresContext * 0x036c9730) line 36
nsContainerFrame::Destroy(nsContainerFrame * const 0x02d7fd30, nsIPresContext * 
0x036c9730) line 98
ViewportFrame::Destroy(ViewportFrame * const 0x02d7fd30, nsIPresContext * 
0x036c9730) line 144
FrameManager::~FrameManager() line 383
FrameManager::`scalar deleting destructor'(unsigned int 1) + 15 bytes
FrameManager::Release(FrameManager * const 0x03249320) line 362 + 157 bytes
PresShell::~PresShell() line 1125 + 27 bytes
PresShell::`scalar deleting destructor'() + 15 bytes
PresShell::Release(PresShell * const 0x03249bf0) line 1041 + 158 bytes
nsCOMPtr<nsIPresShell>::~nsCOMPtr<nsIPresShell>() line 490
DocumentViewerImpl::~DocumentViewerImpl() line 439 + 97 bytes
DocumentViewerImpl::`scalar deleting destructor'(unsigned int 1) + 15 bytes
DocumentViewerImpl::Release(DocumentViewerImpl * const 0x036ce680) line 348 + 
154 bytes
nsCOMPtr<nsIContentViewer>::assign_assuming_AddRef(nsIContentViewer * 
0x00000000) line 472
nsCOMPtr<nsIContentViewer>::assign_with_AddRef(nsISupports * 0x00000000) line 
849
nsCOMPtr<nsIContentViewer>::operator=(nsIContentViewer * 0x00000000) line 584
nsDocShell::SetupNewViewer(nsDocShell * const 0x031d6af0, nsIContentViewer * 
0x03955da0) line 2679
nsWebShell::SetupNewViewer(nsWebShell * const 0x031d6af0, nsIContentViewer * 
0x03955da0) line 386 + 13 bytes
nsDocShell::Embed(nsDocShell * const 0x031d6b10, nsIContentViewer * 0x03955da0, 
const char * 0x021ccf2c, nsISupports * 0x00000000) line 2382 + 23 bytes
nsWebShell::Embed(nsWebShell * const 0x031d6b10, nsIContentViewer * 0x03955da0, 
const char * 0x021ccf2c, nsISupports * 0x00000000) line 419
nsDocShell::CreateContentViewer(nsDocShell * const 0x031d6af0, const char * 
0x0012f8c0, nsIChannel * 0x0391f4c0, nsIStreamListener * * 0x0012f914) line 2550 
+ 32 bytes
nsDSURIContentListener::DoContent(nsDSURIContentListener * const 0x031d62a0, 
const char * 0x0012f8c0, int 0, const char * 0x100a1088 gCommonEmptyBuffer, 
nsIChannel * 0x0391f4c0, nsIStreamListener * * 0x0012f914, int * 0x0012f8a4) 
line 100 + 33 bytes
nsDocumentOpenInfo::DispatchContent(nsIChannel * 0x0391f4c0, nsISupports * 
0x00000000) line 359 + 109 bytes
nsDocumentOpenInfo::OnStartRequest(nsDocumentOpenInfo * const 0x039227c0, 
nsIChannel * 0x0391f4c0, nsISupports * 0x00000000) line 233 + 16 bytes
nsHTTPFinalListener::OnStartRequest(nsHTTPFinalListener * const 0x038f7880, 
nsIChannel * 0x0391f4c0, nsISupports * 0x00000000) line 1155
InterceptStreamListener::OnStartRequest(InterceptStreamListener * const 
0x03920b80, nsIChannel * 0x0391f4c0, nsISupports * 0x00000000) line 1168
nsHTTPServerListener::FinishedResponseHeaders() line 1080 + 48 bytes
nsHTTPServerListener::OnDataAvailable(nsHTTPServerListener * const 0x03907400, 
nsIChannel * 0x038b5994, nsISupports * 0x0391f4c0, nsIInputStream * 0x03907dbc, 
unsigned int 0, unsigned int 1202) line 424 + 8 bytes
nsOnDataAvailableEvent::HandleEvent(nsOnDataAvailableEvent * const 0x038f3810) 
line 400 + 47 bytes
nsStreamListenerEvent::HandlePLEvent(PLEvent * 0x038f37c0) line 97 + 12 bytes
PL_HandleEvent(PLEvent * 0x038f37c0) line 587 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x00f75770) line 528 + 9 bytes
_md_EventReceiverProc(HWND__ * 0x00c30492, unsigned int 49393, unsigned int 0, 
long 16209776) line 1043 + 9 bytes
USER32! 77e71250()
Assignee: av → asa
Status: UNCONFIRMED → NEW
Component: Plug-ins → Browser-General
Ever confirmed: true
QA Contact: shrir → doronr
over to widgets
Assignee: asa → trudelle
Component: Browser-General → XP Toolkit/Widgets
QA Contact: doronr → jrgm
cc dan and kevin
Severity: normal → critical
OS: other → Windows 95
What do you mean 'no plugin stuff involved'?  What are the steps to reproduce
this without any plugins involved?  They don't have to be on the current stack
to have caused the problem.
cc av
Right, that's what I meant. I could not immediately see _how_ the plugin causes 
the crash, but could see the exact place where it crashes. The quick fix most 
likely would be checking pointer for null in the code which is not plugin's. 
This was the only reason I reassigned it. If you don't agree with this please 
give it back to me.
It would be good to put an assertion in nsWindow to cover this, but let's try to
fix the cause of the null, rather than patch all the places it is used. ->av
Assignee: trudelle → av
I can reproduce this and I think I have a fix. 
Could you attach a patch, please?
Andrew's stack trace makes this a dupe of bug 50208; currently #19 in the list of 
top Talkback crashers. One which only Frank seems to be able to reproduce in-
house. Frank, if you have a fix, kill them both, eh?
Here is it. Not a perfect patch. We don't know why someWindow is null, but at
least now we won't crash.
I think this is really a widget group's problem. The code in nsWindow.cpp need
to be more defensive.


===================================================================
RCS file: /m/pub/mozilla/widget/src/windows/nsWindow.cpp,v
retrieving revision 3.294.2.1
diff -u -2 -r3.294.2.1 nsWindow.cpp
--- nsWindow.cpp        2000/10/11 21:47:48     3.294.2.1
+++ nsWindow.cpp        2000/10/18 00:37:28
@@ -935,5 +935,5 @@
     // is not really an interface.
     nsCOMPtr<nsISupports> kungFuDeathGrip;
-    if (!someWindow->mIsDestroying) // not if we're in the destructor!
+    if ((someWindow != nsnull ) && (!someWindow->mIsDestroying)) // not if we'r
e in the destructor!
       kungFuDeathGrip = do_QueryInterface((nsBaseWidget*)someWindow);

@@ -952,5 +952,4 @@
             return retValue;
         }
-    }

 #if defined(STRICT)
@@ -961,4 +960,8 @@
                             msg, wParam, lParam);
 #endif
+       } else {
+               NS_ASSERTION(someWindow, "someWindow is null, cannot call any Ca
llWindowProc");
+               return NULL;
+       }
 }
basically, I add if statement to check someWindow is null or not before I use it.
I also put the last ::CallWindowProc inside the existing if(someWindow !=
nsnull) block and add a else block to assert and return FALSE.
I think we should check in for rtm. trudelle, do you agree this is a good fix?
If so, please mark it as [rtm need info] and ask your engineer to check in.
Assignee: av → trudelle
Keywords: rtm
The fix is equivalent to the patch I proposed for bug 50208 except I returned
the result of the default window proc instead of returning NULL.

if (nsnull == someWindow) return ::DefWindowProc(hWnd, msg, wParam, lParam);

I would change the return NULL in your patch to return ::DefWindowProc(hWnd,
msg, wParam, lParam); just to be safe.

CC'ing rods@netscape.com


Rod, I believe you own this portion of nsWindow.cpp. If so please reassign the
bug to yourself.
  It shouldn't be possible for someWindow to be null -- if it is, something's 
amiss. I say we just go with the patches in 50208. But: Frank, can you reproduce 
the bug reliably? I'm more interested in knowing whether the patch in 50208 which 
attempts to go for the root of the problem, rather than working around the 
mysteriously null pointer, is successful.
rods owns this, reassigning to him. A dead-simple bullet-proofing fix probably
has the best chance of being taken by PDT.
Assignee: trudelle → rods
danm-
1. I reproduce this crash before I try to fix it. The someWindow is null while
that procedure receive a message of 0x00082 ( WM_NCDESTROY ) called by plugin
code. The stack trace is exactly what av report on 9/5
You can easily reproduce it by following the reproduce procedure stated by  T'aZ
2000-09-03 01:55

>It shouldn't be possible for someWindow to be null
It is null on the stack trace. I agree with you there may be other problem you
want to fix.
That is why I leave a NS_ASSERTION there so later you can still hit that.
It won't hurt to add if checking for someWindow is null.
Time is running out. Why don't we check in this change and you can do longer
term research for non crashing (maybe leaking) problem later ?
I haven't found the cause quite yet, but frank's patch does keep the app from 
crashing. We should go with that for now, and then continue to research why it 
is crashing.
Status: NEW → ASSIGNED
r=kmcclusk@netscape.com. Simple check for null pointer which has no downside.
Should also fix bug 50208 which is a topcrash
Whiteboard: [rtm need info]
crashers are P1, not P3.

a=buster.

suggest you submit a new bug saying that we hit the (newly added) assertion, and
as dan says the variable should never be null.  then add a comment around the
assertion citing the new bug number.  the new bug gets "Future", this one gets
"fixed".
Priority: P3 → P1
This bug is really a dup of 50208, so I am setting this bug to be futured and 
since 50208 is already rtm+ I will add an updated patch to it and check that in 
for rtm.

So this is now fixed as far as crashing goes, but we need to figure out why the 
assert gets hit.
Keywords: rtm
Whiteboard: [rtm need info]
Target Milestone: --- → Future
remove crash from keyword since 50208 is already fixed.
Keywords: crash
Keywords: crash
this is probably av's bug
Assignee: rods → av
Status: ASSIGNED → NEW
Is this fixed?
The fix checked in for this crash, on bug 50208 was

  //XXX This fixes 50208 and we are leaving 51174 open to further investigate
  // why we are hitting this assert                                           
  if (nsnull == someWindow) {                                                 
    NS_ASSERTION(someWindow, 
         "someWindow is null, cannot call any CallWindowProc");
    return ::DefWindowProc(hWnd, msg, wParam, lParam);    
  }

However, the URL for the initially report on this bug report, no longer 
appears to have the same content, so I can't force the assertion to occur.
I'm not seeing the problem on Win2K. Marking worksforme.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.