334516 - HTTP assert while starting up: "If we have a txn pump, request must be it"

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect, P1)

Description

[Boris Zbarsky [:bzbarsky]]

BUILD:  Current trunk build

STEPS TO REPRODUCE: Not sure.  I just got this while starting up; might have to shut down previous time in mid-http-load.

DETAILS:  I get:

###!!! ASSERTION: If we have a txn pump, request must be it: '!mTransactionPump || request == mTransactionPump', file ../../../../../mozilla/netwerk/protocol/http/src/nsHttpChannel.cpp, line 3976

With the stack:

#2  0xb781db70 in nsHttpChannel::OnStartRequest (this=0x8bddd58, request=0x8c093d0, 
    ctxt=0x0) at ../../../../../mozilla/netwerk/protocol/http/src/nsHttpChannel.cpp:3975
#3  0xb774eff7 in nsInputStreamPump::OnStateStart (this=0x8c093d0)
    at ../../../../mozilla/netwerk/base/src/nsInputStreamPump.cpp:429
#4  0xb774ee49 in nsInputStreamPump::OnInputStreamReady (this=0x8c093d0, 
    stream=0x8c0b364) at ../../../../mozilla/netwerk/base/src/nsInputStreamPump.cpp:385
#5  0xb7e06e31 in nsInputStreamReadyEvent::EventHandler (plevent=0x8685b3c)
    at ../../../mozilla/xpcom/io/nsStreamUtils.cpp:120

In frame 2 we have:

(gdb) p request
$1 = (nsIRequest *) 0x8c093d0
(gdb) p mCachePump
$2 = {mRawPtr = 0x8c093d0}
(gdb) p mTransactionPump
$3 = {mRawPtr = 0x8bdf1a8}
(gdb) p *(class nsStandardURL*)mURI
...
        mData = 0x8bddce0 "http://www.mozilla.org/projects/minefield/minefield-icon.png"

(gdb) p *this
$8 = {<nsHashPropertyBag> = {<nsIWritablePropertyBag> = {<nsIPropertyBag> = {<nsISupports> = {
          _vptr.nsISupports = 0xb7869fe8}, <No data fields>}, <No data fields>}, <nsIWritablePropertyBag2> = {<nsIPropertyBag2> = {<nsIPropertyBag> = {<nsISupports> = {
            _vptr.nsISupports = 0xb786a190}, <No data fields>}, <No data fields>}, <No data fields>}, mRefCnt = {mValue = 4}, _mOwningThread = {mThread = 0x804a548}, 
    mPropertyHash = {<nsBaseHashtable<nsStringHashKey, nsCOMPtr<nsIVariant>, nsIVariant*>> = {<nsTHashtable<nsBaseHashtableET<nsStringHashKey, nsCOMPtr<nsIVariant> > >> = {
          mTable = {ops = 0xb7e99480, data = 0x0, hashShift = 28, 
            maxAlphaFrac = 192 'À', minAlphaFrac = 64 '@', entrySize = 24, 
            entryCount = 0, removedCount = 0, generation = 0, 
            entryStore = 0x8bddee8 ""}}, <No data fields>}, <No data fields>}}, <nsIHttpChannel> = {<nsIChannel> = {<nsIRequest> = {<nsISupports> = {
          _vptr.nsISupports = 0xb786a1fc}, <No data fields>}, <No data fields>}, <No data fields>}, <nsIHttpChannelInternal> = {<nsISupports> = {
      _vptr.nsISupports = 0xb786a2c4}, <No data fields>}, <nsIStreamListener> = {<nsIRequestObserver> = {<nsISupports> = {
        _vptr.nsISupports = 0xb786a2f0}, <No data fields>}, <No data fields>}, <nsICachingChannel> = {<nsISupports> = {
      _vptr.nsISupports = 0xb786a310}, <No data fields>}, <nsIUploadChannel> = {<nsISupports> = {
      _vptr.nsISupports = 0xb786a344}, <No data fields>}, <nsICacheListener> = {<nsISupports> = {
      _vptr.nsISupports = 0xb786a360}, <No data fields>}, <nsIEncodedChannel> = {<nsISupports> = {
      _vptr.nsISupports = 0xb786a378}, <No data fields>}, <nsITransportEventSink> = {<nsISupports> = {
      _vptr.nsISupports = 0xb786a398}, <No data fields>}, <nsIResumableChannel> = {<nsISupports> = {
      _vptr.nsISupports = 0xb786a3b0}, <No data fields>}, <nsISupportsPriority> = {<nsISupports> = {
      _vptr.nsISupports = 0xb786a3cc}, <No data fields>}, <nsIProtocolProxyCallback> = {<nsISupports> = {_vptr.nsISupports = 0xb786a3ec}, <No data fields>}, mOriginalURI = {
    mRawPtr = 0x8bddbb8}, mURI = {mRawPtr = 0x8bddbb8}, mDocumentURI = {
    mRawPtr = 0x8b07050}, mListener = {mRawPtr = 0x8bde1c0}, 
  mListenerContext = {<nsCOMPtr_base> = {mRawPtr = 0x0}, <No data fields>}, 
  mLoadGroup = {mRawPtr = 0x8bde500}, mOwner = {<nsCOMPtr_base> = {
      mRawPtr = 0x0}, <No data fields>}, mCallbacks = {mRawPtr = 0x891f6d0}, 
  mProgressSink = {mRawPtr = 0x891f218}, mUploadStream = {mRawPtr = 0x0}, mReferrer = {
    mRawPtr = 0x8bde1f8}, mSecurityInfo = {<nsCOMPtr_base> = {
      mRawPtr = 0x0}, <No data fields>}, mEventQ = {mRawPtr = 0x80e2d68}, 
  mProxyRequest = {mRawPtr = 0x0}, mRequestHead = {mHeaders = {
      mHeaders = {<nsTArray_base> = {static sEmptyHdr = {mLength = 0, mCapacity = 0}, 
          mHdr = 0x8bde2c8}, <No data fields>}}, mMethod = {_val = 0xb78578aa "GET"}, 
    mVersion = 11 '\v', mRequestURI = {<nsCSubstring> = {<nsACString_internal> = {
          mVTable = 0xb7e9c6c8, 
          mData = 0x8bde7b0 "/projects/minefield/minefield-icon.png", mLength = 38, 
          mFlags = 5}, <No data fields>}, <No data fields>}}, mResponseHead = 0x8bde8b8, 
  mTransactionPump = {mRawPtr = 0x8bdf1a8}, mTransaction = 0x8bde7f8, 
  mConnectionInfo = 0x8bde070, mSpec = {<nsCSubstring> = {<nsACString_internal> = {
        mVTable = 0xb7e9c6c8, 
        mData = 0x8bddce0 "http://www.mozilla.org/projects/minefield/minefield-icon.png", mLength = 60, mFlags = 5}, <No data fields>}, <No data fields>}, mLoadFlags = 0, 
  mStatus = 0, mLogicalOffset = {mValue = 0}, mCaps = 1 '\001', mPriority = 9, 
  mContentTypeHint = {<nsCSubstring> = {<nsACString_internal> = {mVTable = 0xb7e9c6c8, 
        mData = 0xb7e91220 "", mLength = 0, 
        mFlags = 1}, <No data fields>}, <No data fields>}, 
  mContentCharsetHint = {<nsCSubstring> = {<nsACString_internal> = {
        mVTable = 0xb7e9c6c8, mData = 0xb7e91220 "", mLength = 0, 
        mFlags = 1}, <No data fields>}, <No data fields>}, 
  mUserSetCookieHeader = {<nsCSubstring> = {<nsACString_internal> = {
        mVTable = 0xb7e9c6c8, mData = 0xb7e91220 "", mLength = 0, 
        mFlags = 1}, <No data fields>}, <No data fields>}, mCacheEntry = {
    mRawPtr = 0x8bde768}, mCachePump = {mRawPtr = 0x8c093d0}, mCachedResponseHead = 0x0, 
  mCacheAccess = 3, mPostID = 0, mRequestTime = 1145385884, 
  mProxyAuthContinuationState = 0x0, 
  mProxyAuthType = {<nsCSubstring> = {<nsACString_internal> = {mVTable = 0xb7e9c6c8, 
        mData = 0xb7e91220 "", mLength = 0, 
        mFlags = 1}, <No data fields>}, <No data fields>}, mAuthContinuationState = 0x0, 
  mAuthType = {<nsCSubstring> = {<nsACString_internal> = {mVTable = 0xb7e9c6c8, 
        mData = 0xb7e91220 "", mLength = 0, 
        mFlags = 1}, <No data fields>}, <No data fields>}, mIdent = {mUser = 0x0, 
    mPass = 0x0, mDomain = 0x0}, mProxyIdent = {mUser = 0x0, mPass = 0x0, 
    mDomain = 0x0}, mEntityID = {<nsCSubstring> = {<nsACString_internal> = {
        mVTable = 0xb7e9c6c8, mData = 0xb7e91220 "", mLength = 0, 
        mFlags = 1}, <No data fields>}, <No data fields>}, 
  mStartPos = 18446744073709551615, mRedirectionLimit = 20 '\024', mIsPending = 1, 
  mApplyConversion = 1, mAllowPipelining = 1, mCachedContentIsValid = 1, 
  mCachedContentIsPartial = 1, mResponseHeadersModified = 0, mCanceled = 0, 
  mTransactionReplaced = 0, mUploadStreamHasHeaders = 0, mAuthRetryPending = 0, 
  mSuppressDefensiveAuth = 0, mResuming = 0, mInitedCacheEntry = 0}

Comment 1

[Christian :Biesinger (don't email me, ping me on IRC)]

> mCachedContentIsPartial = 1

hm, clearly I didn't consider that case when writing this code. I suspect it leads to content sniffers getting the wrong data.

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

What I believe happens is this:
- we have cached partial content
- we find the cache entry and set up the transaction w/ range headers, and set up the pump
- server sends 206, httpchannel gets OnStartRequest for the txn
- we suspend the transaction and set up the cache pump
- we get another onstartrequest for the cache pump and assert

This does lead to a bug - content sniffers get the wrong data.

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch

something like this, probably. didn't test this yet.

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 218879 [details] [diff] [review]
patch

this fixes it.

Comment 5

[Darin Fisher]

Comment on attachment 218879 [details] [diff] [review]
patch

>+        // NOTE: We can have both a txn pump and a cache pump when the cache
>+        // content is partial. In that case, we need to read from the cache,
>+        // because that's the one that has the initial contents.
>+        if (mCachePump)
>             mCachePump->PeekStream(CallTypeSniffers,
>                                    NS_STATIC_CAST(nsIChannel*, this));
>+        else
>+            mTransactionPump->PeekStream(CallTypeSniffers,
>+                                         NS_STATIC_CAST(nsIChannel*, this));
>     }

How about making only one call to PeekStream, like this:

  nsIInputStreamPump *pump = mCachePump ? mCachePump : mTransactionPump;
  pump->PeekStream(...);


r+sr=darin

(Please land this on the 1.8 branch as well.)

Comment 6

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: final patch

(with nsInputStreamPump rather than the interface, because PeekStream is not on the iface)

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

(applies identically to the branch, with some offsets)

Comment 8

[Christian :Biesinger (don't email me, ping me on IRC)]

HEAD:
Checking in netwerk/protocol/http/src/nsHttpChannel.cpp;
/cvsroot/mozilla/netwerk/protocol/http/src/nsHttpChannel.cpp,v  <--  nsHttpChannel.cpp
new revision: 1.284; previous revision: 1.283
done

MOZILLA_1_8_BRANCH:
Checking in netwerk/protocol/http/src/nsHttpChannel.cpp;
/cvsroot/mozilla/netwerk/protocol/http/src/nsHttpChannel.cpp,v  <--  nsHttpChannel.cpp
new revision: 1.256.2.15; previous revision: 1.256.2.14
done