50208 - Crash [@ nsWindow::WindowProc ]

Status: VERIFIED FIXED

(SeaMonkey :: General, defect, P1)

Description

[Jay Patel [:jay]]

No specific steps to reproduce are given in the talkback data, but this is a top 
crasher on Win98 and Win95 at startup.  The stack trace is below along with some 
entries.  I am unable to reproduce this crash.  CVS blame shows pinkerton at 
nsWindows.cpp, line 815...so adding him to cc list.

Incident ID 16205833 
nsWindow::WindowProc 
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 815] 
KERNEL32.DLL + 0x363b (0xbff7363b) 
KERNEL32.DLL + 0x242e7 (0xbff942e7) 

nsWindow::WindowProc e25a5634
        
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/widget/src/windows/nsWindow.
cpp line 815
        Build: 2000080712 CrashDate: 2000-08-24 UptimeMinutes: 396  Total: 1407 
        OS: Windows 98  4.10 build 67766446
        URL: 
        Comment: Failed during start up.
        Stacktrace: 
http://cyclone/reports/stackcommentemail.cfm?dynamicBBID=16234225

nsWindow::WindowProc e25a5634
        
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/widget/src/windows/nsWindow.
cpp line 815
        Build: 2000080712 CrashDate: 2000-08-23 UptimeMinutes: 0  Total: 174 
        OS: Windows 98  4.10 build 67766446
        URL: 
        Comment: Doing nothing just clicked on icon to get Netscape 6 started.
        Stacktrace: 
http://cyclone/reports/stackcommentemail.cfm?dynamicBBID=16216971

Comment 1

[Jay Patel [:jay]]

adding topcrash keyword.  [@ nsWindow::WindowProc ]

Comment 2

[Blake Ross]

Adding crash keyword

Comment 3

[Mike Pinkerton (not reading bugmail)]

*shrug*

Comment 4

[Doron Rosenberg (IBM)]

ok, perhaps people did not uninstall pr1 (how could they? no option) and this
caused corruption?  Often, such crashes are due to profile corruptions as well
(a lot changed from pr1 to pr2)

Comment 5

[Jay Patel [:jay]]

Just for the record, this is back for latest branch builds.  Here are the latest 
entries:

nsWindow::WindowProc
Stack Trace: 

         nsWindow::WindowProc   
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp  line 936] 
         KERNEL32.DLL + 0x363b (0xbff7363b)  
         KERNEL32.DLL + 0x24407 (0xbff94407)  
    nsWindow::WindowProc e25a5634
        
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/widget/src/windows/nsWindow.
cpp line 936
        Build: 2000100409 CrashDate: 2000-10-04 UptimeMinutes: 40  Total: 40 
        OS: Windows 98  4.10 build 67766446
        URL: 
        Comment: 
         Detailed : http://climate/reports/incidenttemplate.cfm?bbid=18521009
         StackTrace: 
http://climate/reports/stackcommentemail.cfm?dynamicBBID=18521009

    nsWindow::WindowProc e25a5634
        
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/widget/src/windows/nsWindow.
cpp line 936
        Build: 2000100409 CrashDate: 2000-10-04 UptimeMinutes: 3  Total: 43 
        OS: Windows 98  4.10 build 67766446
        URL: 
        Comment: See bugzilla 39573
         Detailed : http://climate/reports/incidenttemplate.cfm?bbid=18521273
         StackTrace: 
http://climate/reports/stackcommentemail.cfm?dynamicBBID=18521273

    nsWindow::WindowProc 4577858c
        
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/widget/src/windows/nsWindow.
cpp line 936
        Build: 2000100309 CrashDate: 2000-10-04 UptimeMinutes: 609  Total: 609 
        OS: Windows 98  4.90 build 73010104
        URL: 
        Comment: 
         Detailed : http://climate/reports/incidenttemplate.cfm?bbid=18502400
         StackTrace: 
http://climate/reports/stackcommentemail.cfm?dynamicBBID=18502400

    nsWindow::WindowProc e25a5634
        
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/widget/src/windows/nsWindow.
cpp line 936
        Build: 2000100609 CrashDate: 2000-10-07 UptimeMinutes: 580  Total: 580 
        OS: Windows 98  4.10 build 67766446
        URL: 
        Comment: 
         Detailed : http://climate/reports/incidenttemplate.cfm?bbid=18689928
         StackTrace: 
http://climate/reports/stackcommentemail.cfm?dynamicBBID=18689928


NOTE: One of the entries mention bug 39573.

Comment 6

[Jon Granrose]

nominate rtm, mark rtm need info.

since chofmann's on sabbatical we need someone else to look at this more closely.

Comment 7

[Doron Rosenberg (IBM)]

all crashes are in a netscape internal url.  What service is this? does it exist
outside of netscape?

Comment 8

[Jon Granrose]

adding danm & kmcclusk since they've been hacking around in there as well.  A
query on climate shows this is still happening both in PR3 and on the branch.

doron - all these URLs point to servers inside netscape, but are not the URL the
crashes are on, just URLS for netscape talkback reports and bonsai (cvs blame).

Comment 9

[Kevin McCluskey (gone)]

Right after the call to:
nsWindow *someWindow = GetNSWindowPtr(hWnd); at line 931 nsWindow.cpp 

I think we should try adding a check for a null someWindow and return the result 
of the default window handler.

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

The nsWindow::OnDestroy method resets the pointer to the nsIWidget instance to 
null, so it's possible that after the OnDestroy is called we are getting a 
message for the window which results in a null someWindow.

Comment 10

[Dan M]

  Of course in theory we shouldn't even be in our windowproc unless the window 
is currently subclassed, which it shouldn't be outside of OnDestroy and 
StandardWindowCreate. And of course no one here can reproduce the bug. My 
personal take on this, also unlikely to be true, is that maybe-just-maybe a 
message is sneaking in for the window early on during its creation, between the 
line where we take its windowproc and hook up its (nsWindow *) property so 
GetNSWindowPtr will work.
  I think those two lines are done out of order in SubclassWindow, so I'd 
recommend this patch

--- ./widget/src/windows/nsWindow.cpp.3.295	Mon Oct 16 19:59:03 2000
+++ ./widget/src/windows/nsWindow.cpp	Mon Oct 16 19:58:37 2000
@@ -3555,12 +3555,12 @@
     NS_PRECONDITION(::IsWindow(mWnd), "Invalid window handle");
     
     if (bState) {
+        // connect the this pointer to the nsWindow handle
+        SetNSWindowPtr(mWnd, this);
         // change the nsWindow proc
         mPrevWndProc = (WNDPROC)::SetWindowLong(mWnd, GWL_WNDPROC, 
                                                  (LONG)nsWindow::WindowProc);
         NS_ASSERTION(mPrevWndProc, "Null standard window procedure");
-        // connect the this pointer to the nsWindow handle
-        SetNSWindowPtr(mWnd, this);
     } 
     else {
         ::SetWindowLong(mWnd, GWL_WNDPROC, (LONG)mPrevWndProc);

on the offhand chance that some message is sneaking in right there.
  I can't say that I have a great deal of confidence that this is the right fix, 
either. Point in support: on the day this bug was filed, 24 Aug, the build in 
use was probably using nsWindow.cpp version 3.289. Line 815 of that file points 
to some pretty unlikely line in the function DealWithPopups.

Comment 11

[Jon Granrose]

ok, thanks for taking a look at this one.  doesn't sound like we have a good
handle on the problem or the fix, so I'm rtm- for now, and we can play with this
post-RTM.

kmcclusk/danm: can one of you guys take this bug over from chofmann since I
doubt he's the right person to own it?

Comment 12

[Dan M]

  Oh heck. We're all working in the dark because no one can duplicate it, but 
Kevin and I have both posted simple, harmless patches that may address the issue, 
judging from what we can tell in the talkback reports.
  This is a top crasher. It deserves a shot, as best as we can. In a sane 
development environment we'd simply check them in and hope they helped. Marking 
rtm+ in the vague hopes that we can get these changes past micromanagement, but I 
promise to leave it minused if someone comes along and reminuses.

Comment 13

[rods (gone)]

taking bug

Comment 14

[rods (gone)]

*** Bug 51174 has been marked as a duplicate of this bug. ***

Comment 15

[rods (gone)]

Attachment: patch

Comment 16

[rods (gone)]

With this patch we are now hitting the assert and preventing the crash. We will 
research for the cause of the assert under bug 51174. Bugs 50208 and 51174 have 
the same stack trace.

Comment 17

[Kevin McCluskey (gone)]

Patch looks good. Simple, low risk check for null pointer.
r=[e-mail]

Comment 18

[buster]

a=buster

Comment 19

[Mike Pinkerton (not reading bugmail)]

when I've seen this stack before, it's because the nsWindow has been destroyed,
but doesn't really know it yet (i haven't been able to determine why). As a
result, its data members are a crapshoot and are probably mostly random. I
suspect we're getting lucky if the hWnd is NULL and we shouldn't rely on it.

Comment 20

[Kevin McCluskey (gone)]

The patch doesn't rely on the nsWindow pointer being valid. Instead, It relies
on the ptr being stored with the native window being null. The nsWindow instance
should have set this pointer (stored on native window) to null when it was
destroyed. 

Comment 21

[rods (gone)]

checked into trunk

Comment 22

[Phil Peterson]

rtm++

Comment 23

[rods (gone)]

checked into branch

Comment 24

[Doron Rosenberg (IBM)]

is there a way for me to verify this fixed? jpatel - any recent talkback reports
from this? lchiang wants all rtm++ bugs verified and I have no idea how to do
this one.

Comment 25

[Shiva Thirumazhusai]

This crash is not visible in the Talkback Topcrash Reports.

Comment 26

[Doron Rosenberg (IBM)]

cc: lchiang. Lisa: is it enough that this crash is not showing up anymore for
this to get verified?

Comment 27

[lchiang]

Yes, doron, that is good enough for this case.

We never had reproducible steps and had a lot of talkback reports on this.  This
trace no longer shows up in talkback topcrash. So, will mark verified.