92604 - trunk crash [@ nsFileChannel::AsyncOpen]

Status: VERIFIED FIXED

(Core :: Networking: File, defect)

Description

[Chase Tingley]

I stumbled on this while trying to messing around in xpcshell to make sense of
the RDF services.  

Steps to reproduce [xpcshell]:

js> var foo = Components.classes['@mozilla.org/rdf/rdf-service;1'].getService()
js> foo = foo.QueryInterface(Components.interfaces.nsIRDFService)
js> var x = foo.GetDataSource("file:///scratch/test.rdf")

Where "/scratch/test.rdf" exists.

Results:

###!!! ASSERTION: event queue is null: 'mEventQueue', file nsParser.cpp, line 366
###!!! Break: at file nsParser.cpp, line 366
###!!! ASSERTION: You can't dereference a NULL nsCOMPtr with operator->().:
'mRawPtr != 0', file ../../../dist/include/nsCOMPtr.h, line 649
###!!! Break: at file ../../../dist/include/nsCOMPtr.h, line 649

Followed by a segfault.

I got a stack trace showing the crash in nsFileChannel::AsyncOpen():

Program received signal SIGSEGV, Segmentation fault.
0x40691691 in nsFileChannel::AsyncOpen (this=0x8111c68, listener=0x80fb068,
    ctxt=0x0) at nsFileChannel.cpp:294
294             nsresult rv2 = mLoadGroup->RemoveRequest(this, ctxt, rv);  //
XXX fix error message
(gdb) bt
#0  0x40691691 in nsFileChannel::AsyncOpen (this=0x8111c68,
    listener=0x80fb068, ctxt=0x0) at nsFileChannel.cpp:294
#1  0x405961f5 in NS_OpenURI (aConsumer=0x80fb068, context=0x0, uri=0x8143230,
    ioService=0x0, loadGroup=0x0, notificationCallbacks=0x0, loadAttributes=0)
    at ../../../dist/include/nsNetUtil.h:198
#2  0x4057dba0 in RDFXMLDataSourceImpl::Refresh (this=0x80fb058, aBlocking=0)
    at nsRDFXMLDataSource.cpp:884
#3  0x4057bb63 in RDFServiceImpl::GetDataSource (this=0x80f8548,
    aURI=0x80fb900 "file:///scratch/w3.rss", aDataSource=0xbfffd958)
    at nsRDFService.cpp:1237
#4  0x401acab3 in XPTC_InvokeByIndex (that=0x80f8548, methodIndex=14,
    paramCount=2, params=0xbfffd948) at xptcinvoke_unixish_x86.cpp:138
#5  0x404fca11 in XPCWrappedNative::CallMethod (ccx=@0xbfffda04,
    mode=CALL_METHOD) at xpcwrappednative.cpp:1882
#6  0x40505838 in XPC_WN_CallMethod (cx=0x80d9e10, obj=0x80dc2f0, argc=1,
    argv=0x80f4c80, vp=0xbfffdb34) at xpcwrappednativejsops.cpp:1252
#7  0x4005975d in js_Invoke (cx=0x80d9e10, argc=1, flags=0) at jsinterp.c:807
#8  0x40068319 in js_Interpret (cx=0x80d9e10, result=0xbffff6fc)
    at jsinterp.c:2697
#9  0x40059e7a in js_Execute (cx=0x80d9e10, chain=0x80db378, script=0x80bd300,
    down=0x0, special=0, result=0xbffff6fc) at jsinterp.c:986
#10 0x4002c807 in JS_ExecuteScript (cx=0x80d9e10, obj=0x80db378,
    script=0x80bd300, rval=0xbffff6fc) at jsapi.c:3169
#11 0x0804b491 in Process (cx=0x80d9e10, obj=0x80db378, filename=0x0,
    filehandle=0x40380b40) at xpcshell.cpp:523
#12 0x0804ba05 in ProcessArgs (cx=0x80d9e10, obj=0x80db378, argv=0xbffff888,
    argc=0) at xpcshell.cpp:661
#13 0x0804c34c in main (argc=0, argv=0xbffff888) at xpcshell.cpp:913
#14 0x402b8cbe in __libc_start_main () from /lib/libc.so.6
(gdb) p mLoadGroup
$1 = {mRawPtr = 0x0}

A prior call to mLoadGroup->AddRequest is protected by if(mLoadGroup) { ... },
but the call to mLoadGroup->RemoveRequest lacks the same check.

Comment 1

[Chase Tingley]

Attachment: fix

Comment 2

[Doug Turner (:dougt)]

nice find.  r/sr=me.  tingley, do you have access to check this in?

Comment 3

[Chase Tingley]

Nope, if you could check it in, that would be great.

Comment 4

[Doug Turner (:dougt)]

sure, I will.

A slight change to the diff (since we don't care what removerequest returns):

Index: nsFileChannel.cpp
===================================================================
RCS file: /cvsroot/mozilla/netwerk/protocol/file/src/nsFileChannel.cpp,v
retrieving revision 1.111
diff -u -r1.111 nsFileChannel.cpp
--- nsFileChannel.cpp	2001/07/25 07:53:24	1.111
+++ nsFileChannel.cpp	2001/07/27 18:29:40
@@ -291,8 +291,11 @@
                                    getter_AddRefs(mCurrentRequest));
 
     if (NS_FAILED(rv)) {
-        nsresult rv2 = mLoadGroup->RemoveRequest(this, ctxt, rv);  // XXX fix
error message
-        NS_ASSERTION(NS_SUCCEEDED(rv2), "RemoveRequest failed");
+        if (mLoadGroup) {
+            (void) mLoadGroup->RemoveRequest(this, ctxt, rv);  
+            NS_ASSERTION(NS_SUCCEEDED(rv2), "RemoveRequest failed");
+        }
+
         // release the transport so that we don't think we're in progress
         mFileTransport = nsnull;
         mCurrentRequest = nsnull;

Comment 5

[Chase Tingley]

NS_ASSERTION(NS_SUCCEEDED(rv2)) will break the build in the changed version, as
rv2 is no longer declared.

I'd either put rv2 back in or yank the assertion.

Comment 6

[Doug Turner (:dougt)]

d'oh. it will end up something like:


if (NS_FAILED(rv)) {
  if (mLoadGroup) {
    (void) mLoadGroup->RemoveRequest(this, ctxt, rv);  
  }

Comment 7

[Christopher Blizzard (:blizzard)]

r=blizzard

a=blizzzard on behalf of drivers for 0.9.3

Comment 8

[Doug Turner (:dougt)]

cvs commit: Examining .
Checking in nsFileChannel.cpp;
/cvsroot/mozilla/netwerk/protocol/file/src/nsFileChannel.cpp,v  <-- 
nsFileChannel.cpp
new revision: 1.112; previous revision: 1.111
done

Comment 9

[benc]

VERIFIED:
Since the report patched, and the check is documented for an untargeted bug, I'm 
verifying.