180727 - nsViewManager::~nsViewManager doesn't nullcheck mEventQueueService

Status: RESOLVED FIXED

(Core :: Web Painting, defect)

Description

[timeless]

###!!! ASSERTION: You can't dereference a NULL nsCOMPtr with operator->().:
'mRawPtr != 0', file ../../dist/incl
ude/xpcom/nsCOMPtr.h, line 650
Break: at file ../../dist/include/xpcom/nsCOMPtr.h, line 650

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 1024 (LWP 26302)]
0x42b73ec2 in nsViewManager::~nsViewManager (this=0x880fa58, __in_chrg=3) at
nsViewManager.cpp:339
339                                                getter_AddRefs(eventQueue));
(gdb) bt
#0  0x42b73ec2 in nsViewManager::~nsViewManager (this=0x880fa58, __in_chrg=3) at
nsViewManager.cpp:339
#1  0x42b74708 in nsViewManager::Release (this=0x880fa58) at nsViewManager.cpp:430
#2  0x40520c93 in XPCWrappedNative::~XPCWrappedNative (this=0x894e520,
__in_chrg=3) at xpcwrappednative.cpp:547
#3  0x40521c9a in XPCWrappedNative::Release (this=0x894e520) at
xpcwrappednative.cpp:777
#4  0x40521eed in XPCWrappedNative::FlatJSObjectFinalized (this=0x894e520,
cx=0x80b80b8, obj=0x88834a0)
    at xpcwrappednative.cpp:896
#5  0x4052b644 in XPC_WN_NoHelper_Finalize (cx=0x80b80b8, obj=0x88834a0) at
xpcwrappednativejsops.cpp:631
#6  0x4008343c in js_FinalizeObject (cx=0x80b80b8, obj=0x88834a0) at jsobj.c:1840
#7  0x4005fa8c in js_GC (cx=0x80b80b8, gcflags=5) at jsgc.c:1311
#8  0x4005e55d in js_AllocGCThing (cx=0x80b80b8, flags=1) at jsgc.c:523
#9  0x400b8b66 in js_NewString (cx=0x80b80b8, chars=0x8a2d008, length=14,
gcflag=0) at jsstr.c:2418
#10 0x40030dd3 in JS_NewStringCopyZ (cx=0x80b80b8, s=0x80b4c61 "nsILDAPMessage")
at jsapi.c:3542
#11 0x404f8840 in nsXPCComponents_Interfaces::NewEnumerate (this=0x80d81a8,
wrapper=0x80d8278, cx=0x80b80b8,
    obj=0x808e5d0, enum_op=1, statep=0xbfffe51c, idp=0xbfffe53c,
_retval=0xbfffdbf8) at xpccomponents.cpp:195
#12 0x4052d1a8 in XPC_WN_JSOp_Enumerate (cx=0x80b80b8, obj=0x808e5d0,
enum_op=JSENUMERATE_NEXT,
    statep=0xbfffe51c, idp=0xbfffe53c) at xpcwrappednativejsops.cpp:1058
#13 0x40065bc1 in js_Interpret (cx=0x80b80b8, result=0xbffff6dc) at jsinterp.c:1775
#14 0x4006306a in js_Execute (cx=0x80b80b8, chain=0x808dcc0, script=0x80d5310,
down=0x0, special=0,
    result=0xbffff6dc) at jsinterp.c:1020
#15 0x40030677 in JS_ExecuteScript (cx=0x80b80b8, obj=0x808dcc0,
script=0x80d5310, rval=0xbffff6dc)
    at jsapi.c:3277
#16 0x804badb in Process (cx=0x80b80b8, obj=0x808dcc0, filename=0xbffff9d5
"driver.js", filehandle=0x0)
    at xpcshell.cpp:479
#17 0x804c1d5 in ProcessArgs (cx=0x80b80b8, obj=0x808dcc0, argv=0xbffff878,
argc=1) at xpcshell.cpp:655
#18 0x804cd8e in main (argc=1, argv=0xbffff878) at xpcshell.cpp:912
#19 0x403852eb in __libc_start_main (main=0x804c544 <main>, argc=2,
ubp_av=0xbffff874, init=0x804a7a4 <_init>,
    fini=0x804f030 <_fini>, rtld_fini=0x4000c130 <_dl_fini>, stack_end=0xbffff86c)
    at ../sysdeps/generic/libc-start.c:129

Comment 1

[timeless]

Attachment: nullcheck

Comment 2

[Hixie (not reading bugmail)]

um, wouldn't it not having an mEventQueueService be a serious problem?

i.e. isn't the patch just papering over the problem and moving the bug from a
simple crash to an insidious and hard to track down bug.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Yeah.

I think a better fix would be to just put the "if" around the call to
GetSpecialEventQueue. Then if mEventQueueService is null, we'll get an assertion
on "nsnull != eventQueue". Then also put an "if (nsnull != eventQueue)" around
eventQueue->RevokeEvents.

Comment 4

[David Bradley]

I know you've got a patch, but I wonder if ultimately this might have been
caused by bug 180727. I see the same gcflags=0x5 when js_GC is called.

Comment 5

[Robert Ginda]

no, this isn't papering over any problem.  Timeless probably should have
explained this, instead of just posting a stack trace and a patch and expecting
everyone to understand what the deal is.

This crash only happens when you instantiate a view manager and then destroy it
right away.  Like, if you were to create one, QI it for a few interfaces, and
then dispose of it, like the component viewer does.

I'm pretty sure this is the right patch.

Comment 6

[basic]

status?

Comment 7

[Martijn Wargers (dead)]

Attachment: testcase

You need to test the testcase locally to get the crash.

Comment 8

[Martijn Wargers (dead)]

Testcase doesn't crash anymore in 2006-02-22 trunk build. I guess fixed by bug 327655. Is this bug now completely fixed?

Comment 9

[Mats Palmgren (inactive)]

WFM, trunk debug build of SeaMonkey on Linux.

Building SeaMonkey from 2006-02-21-05-trunk source tarball -> crash:
  0x417dc32d in ~nsViewManager (this=0x891cf98) at nsViewManager.cpp:235
  235       mEventQueueService->GetSpecialEventQueue(...
with null mEventQueueService.

Applying patch from bug 327655 -> no crash.

-> FIXED (by bug 327655)