Seen while reviewing crash stats. Started showing up in crash stats using the 2010120300 build. Low volume crash which is so far Windows only. http://tinyurl.com/34hwbmq to the crashes so far. Frame Module Signature [Expand] Source 0 xul.dll nsHttpPipeline::GetConnectionInfo netwerk/protocol/http/nsHttpPipeline.cpp:231 1 xul.dll nsHttpTransaction::ParseHead netwerk/protocol/http/nsHttpTransaction.cpp:827 2 mozcrt19.dll arena_dalloc obj-firefox/memory/jemalloc/crtsrc/jemalloc.c:4281 3 xul.dll nsHttpPipeline::Close netwerk/protocol/http/nsHttpPipeline.cpp:527 4 xul.dll nsHttpConnection::CloseTransaction netwerk/protocol/http/nsHttpConnection.cpp:648 5 xul.dll nsHttpConnection::OnInputStreamReady netwerk/protocol/http/nsHttpConnection.cpp:1000 6 xul.dll nsSocketInputStream::OnSocketReady netwerk/base/src/nsSocketTransport2.cpp:256 7 xul.dll nsSocketTransport::OnSocketReady netwerk/base/src/nsSocketTransport2.cpp:1526 8 xul.dll nsSocketTransportService::DoPollIteration netwerk/base/src/nsSocketTransportService2.cpp:687 9 xul.dll nsSocketTransportService::OnProcessNextEvent netwerk/base/src/nsSocketTransportService2.cpp:551 10 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:590 11 xul.dll NS_ProcessPendingEvents_P obj-firefox/xpcom/build/nsThreadUtils.cpp:200 12 xul.dll NS_ProcessNextEvent_P obj-firefox/xpcom/build/nsThreadUtils.cpp:250 13 xul.dll nsSocketTransportService::Run netwerk/base/src/nsSocketTransportService2.cpp:594 14 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:626 15 xul.dll nsThreadStartupEvent::Run xpcom/threads/nsThread.cpp:207 16 nspr4.dll _PR_NativeRunThread nsprpub/pr/src/threads/combined/pruthr.c:426 17 nspr4.dll pr_root nsprpub/pr/src/md/windows/w95thred.c:122 18 mozcrt19.dll _callthreadstartex obj-firefox/memory/jemalloc/crtsrc/threadex.c:348 19 mozcrt19.dll __dllonexit obj-firefox/memory/jemalloc/crtsrc/onexit.c:276 20 mozcrt19.dll _threadstartex obj-firefox/memory/jemalloc/crtsrc/threadex.c:326 21 kernel32.dll GetCodePageFileInfo

probably want to look at 363109 which landed yesterday and changed nsHttpTransaction::ParseHead()

Given that this is a very recent regression I think we should track this down before we ship beta8. Michal, Patrick seems to think this might be a regression from bug 363109, so handing this to you to start with.

Er, and actually reassigning, Michal, please see my previous comment.

nsHttpTransaction::ParseHead may be called from Close, where mConnection could be null. We should make sure that it's not before attempting to get its connection info.

Comment on attachment 495223 [details] [diff] [review] Patch (v1) Asking review from biesi too in case he can get to this sooner.

Comment on attachment 495223 [details] [diff] [review] Patch (v1) r=me if you s/value id/value is/ in the preexisting comment and add a comment explaining that if !ci then we can't possibly have a response body, so there's no reason to look for junk in it.

looks like this still is showing up in buildid 20101207033246 and 20101207030325 like http://crash-stats.mozilla.com/report/index/4847538a-db23-4fad-9d57-c0dd02101207 and http://crash-stats.mozilla.com/report/list?product=Firefox&version=Firefox%3A4.0b8pre&query_search=signature&query_type=exact&query=nsHttpPipeline%3A%3AGetConnectionInfo%28nsHttpConnectionInfo**%29&do_query=1&admin=&signature=nsHttpPipeline%3A%3AGetConnectionInfo%28nsHttpConnectionInfo**%29

> looks like this still is showing up in buildid 20101207033246 and > 2010120703032 ...with the same crash daily rate.

The crash has never been in nsHttpTransaction, but always in nsHttpPipeline. The Ehsan's fix is good to have but will never fix this problem. The true issue is here [1]: We release mConnection from nsHttpPipeline before we call Close on the nsHttpTransaction that (the connection), after bug 363109 has landed, is accessed under some circumstances in nsHttpTransaction::Close. [1] http://hg.mozilla.org/mozilla-central/annotate/0ff6d5984287/netwerk/protocol/http/nsHttpPipeline.cpp#l505 I have a real patch for this.

- release mConnection of nsHttpPipeline after we call Close on all transactions

For reference: release of the connection has been introduced at this place in bug 176919.

Unless we're going to get this in the tree in the next few hours, do we really have to block beta 8? We've gotta ship it immediately. We're going too slow.

Boris, can you review (and assuming the changes looks good, land this) ASAP?

Comment on attachment 496256 [details] [diff] [review] Additional patch - v1 Please add a comment at the new location saying that releasing mConnection needs to come after we close all the HTTP transactions we have in the pipeline, ok?

So... people who hit this bug presumably have HTTP pipelining enabled. Interesting that we saw this at all. ;)

A little late, but I was able to repro after what bz suggested in Comment 17 with one of the crash URLS. 1. Enable pipelining via about:config 2. Load http://www.accaglobal.com/ 3. Reproducible crash. https://crash-stats.mozilla.com/report/index/bp-e7bfb645-ff5d-4733-bb8d-7fcc82101208 is my report on Win 7.

Pushed http://hg.mozilla.org/mozilla-central/rev/4fccdd4bc023 and http://hg.mozilla.org/mozilla-central/rev/b0a815aedb7e for the comment fix. Honza, please write a test for this too?

Thanks for killing this, guys! Righteous. :)

Er, make that http://hg.mozilla.org/mozilla-central/rev/81aea2200c34 for the comment fix (silly precommit hook!).

(In reply to comment #19) > Pushed http://hg.mozilla.org/mozilla-central/rev/4fccdd4bc023 and > http://hg.mozilla.org/mozilla-central/rev/b0a815aedb7e for the comment fix. > > Honza, please write a test for this too? Thanks for pushing it, Boris. I'll try to have a test.

Verified fixed using Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101209 Firefox/4.0b8pre. I no longer crash following the STR In Comment 18.