Closed Bug 57044 Opened 24 years ago Closed 24 years ago

[FIX]crash deref-ing a null pointer in nsWindow::SetFocus

Categories

(Core :: XUL, defect, P2)

x86
Windows NT
defect

Tracking

()

VERIFIED FIXED

People

(Reporter: buster, Assigned: rods)

References

Details

(Keywords: crash, Whiteboard: [rtm++]Fix in hand, reviewed, superreviewed)

I've seen this crash several times in the past several days, using a variety of
debug builds on 2 different WinNT machines.  Sorry, I don't have a particular
URL or reproducable senario.

nsToolkit::IsGuiThread() line 52 + 18 bytes      <-- bogus call through null ptr
nsWindow::SetFocus() line 1607 + 8 bytes
nsEventStateManager::SendFocusBlur() line 2816
nsEventStateManager::SetContentState() line 2496
nsGenericHTMLElement::HandleDOMEventForAnchors() line 1106
nsHTMLAnchorElement::HandleDOMEvent() line 405
nsGenericDOMDataNode::HandleDOMEvent() line 689 + 39 bytes
nsTextNode::HandleDOMEvent() line 253
PresShell::HandleEventInternal() line 4894 + 41 bytes
PresShell::HandleEvent() line 4823 + 25 bytes
nsView::HandleEvent() line 379
nsView::HandleEvent() line 352
nsView::HandleEvent() line 352
nsViewManager2::DispatchEvent() line 1439
HandleEvent() line 68
nsWindow::DispatchEvent() line 682 + 10 bytes
nsWindow::DispatchWindowEvent() line 703
nsWindow::DispatchMouseEvent() line 3895 + 21 bytes
ChildWindow::DispatchMouseEvent() line 4105
nsWindow::ProcessMessage() line 2960 + 24 bytes
nsWindow::WindowProc() line 951 + 27 bytes
USER32! 77e71820()
GetUIEventProperty() line 78 + 3 bytes


The code could probably use a null pointer check:

NS_METHOD nsWindow::SetFocus(void)
{
    nsToolkit* toolkit = (nsToolkit *)mToolkit;     <--- mToolkit is not checked
    if (!toolkit->IsGuiThread()) {                  <--- crash
        MethodInfo info(this, nsWindow::SET_FOCUS);
        toolkit->CallMethod(&info);
        return NS_ERROR_FAILURE;
    }
according to lxr, kevin actually added the offending line of code.  maybe the
bug should be assigned to you, kevin?
I can take it.

I agree it would be good to put in a check, but I wonder why it is null, it 
really shouldn't be.

I think this would be a good one to fix for RTM, to prevent further crashes, if 
so, assign it to me.
->rods.  thanks Rod!
Assignee: trudelle → rods
Here's the patch:

Index: src/windows/nsWindow.cpp
===================================================================
RCS file: /cvsroot/mozilla/widget/src/windows/nsWindow.cpp,v
retrieving revision 3.294.2.1
diff -u -r3.294.2.1 nsWindow.cpp
--- nsWindow.cpp        2000/10/11 21:47:48     3.294.2.1
+++ nsWindow.cpp        2000/10/17 20:34:18
@@ -1604,7 +1604,8 @@
     // be executed on the "gui thread"...
     //
     nsToolkit* toolkit = (nsToolkit *)mToolkit;
-    if (!toolkit->IsGuiThread()) {
+    NS_ASSERTION(toolkit != nsnull, "This should never be null!");
+    if (toolkit != nsnull && !toolkit->IsGuiThread()) {
         MethodInfo info(this, nsWindow::SET_FOCUS);
         toolkit->CallMethod(&info);
         return NS_ERROR_FAILURE;
Status: NEW → ASSIGNED
Keywords: rtm
Whiteboard: [rtm need info]
r = saari
Summary: crash deref-ing a null pointer in nsWindow::SetFocus → [IX]crash deref-ing a null pointer in nsWindow::SetFocus
Whiteboard: [rtm need info] → [rtm need info]Fix in hand
Summary: [IX]crash deref-ing a null pointer in nsWindow::SetFocus → [FIX]crash deref-ing a null pointer in nsWindow::SetFocus
Priority: P3 → P2
Adding crash kwd. for Rod.
Keywords: crash
a=buster (I think I sent out an email already, but forgot to mark the bug)
Marked rtm+.  Simple check for null pointer that fixes crasher.
Whiteboard: [rtm need info]Fix in hand → [rtm+]Fix in hand, reviewed, superreviewed
checked into trunk
rtm++
Whiteboard: [rtm+]Fix in hand, reviewed, superreviewed → [rtm++]Fix in hand, reviewed, superreviewed
checked into branch
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
*** Bug 56711 has been marked as a duplicate of this bug. ***
verified fixed, 2000101909 win2k (after the checkin)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.