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

VERIFIED FIXED

Status

()

Core
XUL
P2
critical
VERIFIED FIXED
17 years ago
17 years ago

People

(Reporter: buster, Assigned: rods (gone))

Tracking

({crash})

Trunk
x86
Windows NT
crash
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

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

(Reporter)

Description

17 years ago
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;
    }
(Reporter)

Comment 1

17 years ago
according to lxr, kevin actually added the offending line of code.  maybe the
bug should be assigned to you, kevin?
(Assignee)

Comment 2

17 years ago
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.

Comment 3

17 years ago
->rods.  thanks Rod!
Assignee: trudelle → rods
(Assignee)

Comment 4

17 years ago
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]

Comment 5

17 years ago
r = saari
(Assignee)

Updated

17 years ago
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
(Assignee)

Updated

17 years ago
Summary: [IX]crash deref-ing a null pointer in nsWindow::SetFocus → [FIX]crash deref-ing a null pointer in nsWindow::SetFocus
(Assignee)

Updated

17 years ago
Priority: P3 → P2

Comment 6

17 years ago
Adding crash kwd. for Rod.
Keywords: crash
(Reporter)

Comment 7

17 years ago
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
(Assignee)

Comment 9

17 years ago
checked into trunk

Comment 10

17 years ago
rtm++
Whiteboard: [rtm+]Fix in hand, reviewed, superreviewed → [rtm++]Fix in hand, reviewed, superreviewed
(Assignee)

Comment 11

17 years ago
checked into branch
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
(Assignee)

Comment 12

17 years ago
*** Bug 56711 has been marked as a duplicate of this bug. ***

Comment 13

17 years ago
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.