217853 - @mozilla.org/network/async-stream-listener;1 crashes if it isn't initialized in debug-only builds [@ nsAsyncStreamObserver::OnStopRequest]

Status: RESOLVED INCOMPLETE

(Core :: Networking, defect)

Description

[timeless]

Steps:
run xpcshell and execute:
const
io=Components.classes["@mozilla.org/network/io-service;1"].getService(Components.interfaces.nsIIOService);
url="http://qa.mandrakesoft.com/show_bug.cgi?id=5012"
io.newChannelFromURI(io.newURI(url,0,{})).asyncOpen(Components.classes["@mozilla.org/network/async-stream-listener;1"].createInstance(),{});
io.newChannelFromURI(io.newURI(url,0,{})).asyncOpen(Components.classes["@mozilla.org/network/async-stream-listener;1"].createInstance(),{});
quit()

In some cases I needed to have extra code before the quit to give the async
thread time to warm up so it starts processing before xpcom shuts down. one
thing i did was touch -d "1/1/1990" chrome/chrome.rdf

This code crashed timeless@raistlin:w2k/mozilla/xpcshell as well as
drepper:linux/firebird/xpcshell. the version i gave to drepper only had one call
to asyncOpen.

First there's a warning about a null mSTS, and I had the stack for that but I
went to file a bug for the nsDebugImpl and mozilla ate my bug report for this
bug and now i'm having a hard time getting it back.

Then there's an assertion about dereferencing a null pointer (followed by the
crash when it actually gets dereferenced):
nsDebug::Assertion(const char * 0x0336f358 `string', const char * 0x0336f39c
`string', const char * 0x0336f998 `string', int 668) line 109
nsCOMPtr<nsIEventQueue>::operator->() line 668 + 34 bytes
nsAsyncStreamObserver::OnStartRequest(nsAsyncStreamObserver * const 0x00926ef0,
nsIRequest * 0x0091b570, nsISupports * 0x00926620) line 241 + 15 bytes
nsAsyncStreamListener::OnStartRequest(nsAsyncStreamListener * const 0x00926ef0,
nsIRequest * 0x0091b570, nsISupports * 0x00926620) line 89
nsHttpChannel::CallOnStartRequest() line 639 + 60 bytes
nsHttpChannel::OnStartRequest(nsHttpChannel * const 0x0091b578, nsIRequest *
0x00924a70, nsISupports * 0x00000000) line 3255
nsInputStreamPump::OnStateStart() line 362 + 42 bytes
nsInputStreamPump::OnInputStreamReady(nsInputStreamPump * const 0x00924a74,
nsIAsyncInputStream * 0x00924e8c) line 318 + 11 bytes
nsInputStreamReadyEvent::EventHandler(PLEvent * 0x009238d4) line 117
PL_HandleEvent(PLEvent * 0x009238d4) line 670 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x008f2ca0) line 605 + 9 bytes
nsEventQueueImpl::ProcessPendingEvents(nsEventQueueImpl * const 0x008f2d70) line
385 + 12 bytes
NS_ShutdownXPCOM(nsIServiceManager * 0x00000000) line 725
main(int 0, char * * 0x008b4294, char * * 0x008b0e00) line 1097 + 8 bytes
mainCRTStartup() line 338 + 17 bytes

With patches for asyn-stream-listener....
Running to completion (well almost), one gets this:
WARNING: unable to post DNS lookup complete message, file
i:/build/mozilla/netwerk/base/src/nsSocketTransport2.cpp, line
 1654
* * * * XPCOM shutdown. Access will be denied * * * *
+++ JavaScript debuging hooks removed.
Getting service on shutdown. Denied.
  ContractID: @mozilla.org/js/xpc/RuntimeService;1
         IID: {dcfbb49c-1dd1-11b2-9f39-935aef484817}
###!!! ASSERTION: nsDebugImpl not thread-safe: '_mOwningThread.GetThread() ==
PR_GetCurrentThread()', file i:/build/mozi
lla/xpcom/base/nsDebugImpl.cpp, line 145
Break: at file i:/build/mozilla/xpcom/base/nsDebugImpl.cpp, line 145

Which is sufficient to indicate that the networking object is done and won't
crash. it's also bug 217852.

Comment 1

[timeless]

Attachment: NS_ENSURE_TRUE(mEventQueue, ...)

PR_NewLogModule can fail and PR_LOG will crash if it fails.
this file has about 5 gotos and the one goto i'm removing was just really hard
to follow because of the indentation, i kept reading it as:

  if (NS_FAILED(rv))
  return rv;

failed:
  delete rv;
  return rv;

Comment 2

[Darin Fisher]

oh come now... PR_NewLogModule failing?  it is not worth the code to null check
that.  that will only happen if there is an out-of-memory situation, and
moreover this code is only enabled for debug builds.  we don't check the return
value of PR_NewLogModule elsewhere, so why bother doing so here.  please don't
bother.  however, if you were to insist, then the conditional should be hidden
behind a preprocessor macro or something.

#ifdef PR_LOGGING
#define LOG(args)
  PR_BEGIN_MACRO \
    if (gLogModule) \
      PR_LOG(gLogModule, PR_LOG_DEBUG, args); \
  PR_END_MACRO
#endif

but this just seems overkill to me.

Comment 3

[Darin Fisher]

Comment on attachment 130673 [details] [diff] [review]
NS_ENSURE_TRUE(mEventQueue, ...)

sr- per previous comment.

Comment 4

[Robert Kaiser]

This bug has no information on current versions and no clear lead as to what the crashes that it tracks are. This should either be updated with current info and reopened or new bugs be filed on concrete actions/crashes on current code.