diff --git a/netwerk/protocol/http/Http2Push.cpp b/netwerk/protocol/http/Http2Push.cpp index 88b3cb380..44cd12618 100644 --- a/netwerk/protocol/http/Http2Push.cpp +++ b/netwerk/protocol/http/Http2Push.cpp @@ -30,8 +30,8 @@ class CallChannelOnPush final : public nsRunnable { Http2PushedStream *pushStream) : mAssociatedChannel(associatedChannel) , mPushedURI(pushedURI) - , mPushedStream(pushStream) { + mPushedStreamWrapper = new Http2PushedStreamWrapper(pushStream); } NS_IMETHOD Run() @@ -40,21 +40,97 @@ class CallChannelOnPush final : public nsRunnable { RefPtr channel; CallQueryInterface(mAssociatedChannel, channel.StartAssignment()); MOZ_ASSERT(channel); - if (channel && NS_SUCCEEDED(channel->OnPush(mPushedURI, mPushedStream))) { + if (channel && + NS_SUCCEEDED(channel->OnPush(mPushedURI, mPushedStreamWrapper))) { return NS_OK; } LOG3(("Http2PushedStream Orphan %p failed OnPush\n", this)); - mPushedStream->OnPushFailed(); + mPushedStreamWrapper->OnPushFailed(); return NS_OK; } private: nsCOMPtr mAssociatedChannel; const nsCString mPushedURI; - Http2PushedStream *mPushedStream; + RefPtr mPushedStreamWrapper; }; +// Because WeakPtr isn't thread-safe we must ensure that the object is destroyed +// on the socket thread, so any Release() called on a different thread is +// dispatched to the socket thread. +bool Http2PushedStreamWrapper::DispatchRelease() { + if (PR_GetCurrentThread() == gSocketThread) { // OnSocketThread() + return false; + } + + gSocketTransportService->Dispatch( + NS_NewNonOwningRunnableMethod(this, + &Http2PushedStreamWrapper::Release), + NS_DISPATCH_NORMAL); + + return true; +} + +NS_IMPL_ADDREF(Http2PushedStreamWrapper) +NS_IMETHODIMP_(MozExternalRefCountType) +Http2PushedStreamWrapper::Release() { + nsrefcnt count = mRefCnt - 1; + if (DispatchRelease()) { + // Redispatched to the socket thread. + return count; + } + + MOZ_ASSERT(0 != mRefCnt, "dup release"); + count = --mRefCnt; + NS_LOG_RELEASE(this, count, "Http2PushedStreamWrapper"); + + if (0 == count) { + mRefCnt = 1; + delete (this); + return 0; + } + + return count; +} + +NS_INTERFACE_MAP_BEGIN(Http2PushedStreamWrapper) +NS_INTERFACE_MAP_END + +Http2PushedStreamWrapper::Http2PushedStreamWrapper( + Http2PushedStream* aPushStream) { + MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread, "not on socket thread"); + mStream = aPushStream; + mRequestString = aPushStream->GetRequestString(); +} + +Http2PushedStreamWrapper::~Http2PushedStreamWrapper() { + MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread, "not on socket thread"); +} + +Http2PushedStream* Http2PushedStreamWrapper::GetStream() { + MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread, "not on socket thread"); + if (mStream) { + Http2Stream* stream = mStream; + return static_cast(stream); + } + return nullptr; +} + +void Http2PushedStreamWrapper::OnPushFailed() { + if (PR_GetCurrentThread() == gSocketThread) { // OnSocketThread() + if (mStream) { + Http2Stream* stream = mStream; + static_cast(stream)->OnPushFailed(); + } + } else { + gSocketTransportService->Dispatch( + NS_NewRunnableMethod(this, + &Http2PushedStreamWrapper::OnPushFailed), + NS_DISPATCH_NORMAL); + } +} + ////////////////////////////////////////// // Http2PushedStream ////////////////////////////////////////// diff --git a/netwerk/protocol/http/Http2Push.h b/netwerk/protocol/http/Http2Push.h index 799543d4b..65bf545c2 100644 --- a/netwerk/protocol/http/Http2Push.h +++ b/netwerk/protocol/http/Http2Push.h @@ -121,6 +121,24 @@ private: uint32_t mBufferedHTTP1Consumed; }; +class Http2PushedStreamWrapper : public nsISupports { + public: + NS_DECL_THREADSAFE_ISUPPORTS + bool DispatchRelease(); + + explicit Http2PushedStreamWrapper(Http2PushedStream* aPushStream); + + nsCString& GetRequestString() { return mRequestString; } + Http2PushedStream* GetStream(); + void OnPushFailed(); + + private: + virtual ~Http2PushedStreamWrapper(); + + nsCString mRequestString; + WeakPtr mStream; +}; + } // namespace net } // namespace mozilla diff --git a/netwerk/protocol/http/Http2Stream.cpp b/netwerk/protocol/http/Http2Stream.cpp index 444dee1e9..27c4f2fad 100644 --- a/netwerk/protocol/http/Http2Stream.cpp +++ b/netwerk/protocol/http/Http2Stream.cpp @@ -460,12 +460,14 @@ Http2Stream::ParseHttpRequestHeaders(const char *buf, schedulingContext->GetSpdyPushCache(&cache); } + RefPtr pushedStreamWrapper; Http2PushedStream *pushedStream = nullptr; // If a push stream is attached to the transaction via onPush, match only with that // one. This occurs when a push was made with in conjunction with a nsIHttpPushListener nsHttpTransaction *trans = mTransaction->QueryHttpTransaction(); - if (trans && (pushedStream = trans->TakePushedStream())) { + if (trans && (pushedStreamWrapper = trans->TakePushedStream()) && + (pushedStream = pushedStreamWrapper->GetStream())) { if (pushedStream->mSession == mSession) { LOG3(("Pushed Stream match based on OnPush correlation %p", pushedStream)); } else { diff --git a/netwerk/protocol/http/nsHttpChannel.cpp b/netwerk/protocol/http/nsHttpChannel.cpp index c5797496c..cd747110b 100644 --- a/netwerk/protocol/http/nsHttpChannel.cpp +++ b/netwerk/protocol/http/nsHttpChannel.cpp @@ -7070,7 +7070,7 @@ nsHttpChannel::AwaitingCacheCallbacks() } void -nsHttpChannel::SetPushedStream(Http2PushedStream *stream) +nsHttpChannel::SetPushedStream(Http2PushedStreamWrapper *stream) { MOZ_ASSERT(stream); MOZ_ASSERT(!mPushedStream); @@ -7078,7 +7078,8 @@ nsHttpChannel::SetPushedStream(Http2PushedStream *stream) } nsresult -nsHttpChannel::OnPush(const nsACString &url, Http2PushedStream *pushedStream) +nsHttpChannel::OnPush(const nsACString &url, + Http2PushedStreamWrapper *pushedStream) { MOZ_ASSERT(NS_IsMainThread()); LOG(("nsHttpChannel::OnPush [this=%p]\n", this)); diff --git a/netwerk/protocol/http/nsHttpChannel.h b/netwerk/protocol/http/nsHttpChannel.h index 653ac2494..958e642e7 100644 --- a/netwerk/protocol/http/nsHttpChannel.h +++ b/netwerk/protocol/http/nsHttpChannel.h @@ -120,7 +120,8 @@ public: uint32_t aProxyResolveFlags, nsIURI *aProxyURI) override; - nsresult OnPush(const nsACString &uri, Http2PushedStream *pushedStream); + nsresult OnPush(const nsACString &uri, + Http2PushedStreamWrapper *pushedStream); static bool IsRedirectStatus(uint32_t status); @@ -411,7 +412,7 @@ private: nsresult OpenCacheInputStream(nsICacheEntry* cacheEntry, bool startBuffering, bool checkingAppCacheEntry); - void SetPushedStream(Http2PushedStream *stream); + void SetPushedStream(Http2PushedStreamWrapper *stream); void MaybeWarnAboutAppCache(); @@ -522,7 +523,7 @@ private: // Needed for accurate DNS timing RefPtr mDNSPrefetch; - Http2PushedStream *mPushedStream; + RefPtr mPushedStream; // True if the channel's principal was found on a phishing, malware, or // tracking (if tracking protection is enabled) blocklist bool mLocalBlocklist; diff --git a/netwerk/protocol/http/nsHttpConnectionMgr.cpp b/netwerk/protocol/http/nsHttpConnectionMgr.cpp index 2723ba1cb..22166543b 100644 --- a/netwerk/protocol/http/nsHttpConnectionMgr.cpp +++ b/netwerk/protocol/http/nsHttpConnectionMgr.cpp @@ -2060,11 +2060,15 @@ nsHttpConnectionMgr::ProcessNewTransaction(nsHttpTransaction *trans) trans->SetPendingTime(); - Http2PushedStream *pushedStream = trans->GetPushedStream(); - if (pushedStream) { - return pushedStream->Session()-> - AddStream(trans, trans->Priority(), false, nullptr) ? - NS_OK : NS_ERROR_UNEXPECTED; + RefPtr pushedStreamWrapper = + trans->GetPushedStream(); + if (pushedStreamWrapper) { + Http2PushedStream* pushedStream = pushedStreamWrapper->GetStream(); + if (pushedStream) { + return pushedStream->Session()-> + AddStream(trans, trans->Priority(), false, nullptr) ? + NS_OK : NS_ERROR_UNEXPECTED; + } } nsresult rv = NS_OK; diff --git a/netwerk/protocol/http/nsHttpTransaction.h b/netwerk/protocol/http/nsHttpTransaction.h index b7021dc13..4f5d244e1 100644 --- a/netwerk/protocol/http/nsHttpTransaction.h +++ b/netwerk/protocol/http/nsHttpTransaction.h @@ -134,14 +134,16 @@ public: nsHttpTransaction *QueryHttpTransaction() override { return this; } - Http2PushedStream *GetPushedStream() { return mPushedStream; } - Http2PushedStream *TakePushedStream() - { - Http2PushedStream *r = mPushedStream; - mPushedStream = nullptr; - return r; + already_AddRefed GetPushedStream() { + //return do_AddRef(mPushedStream); // XXX: add this support to RefPtr.h + RefPtr ref(mPushedStream); + return ref.forget(); } - void SetPushedStream(Http2PushedStream *push) { mPushedStream = push; } + already_AddRefed TakePushedStream() { + return mPushedStream.forget(); + } + + void SetPushedStream(Http2PushedStreamWrapper* push) { mPushedStream = push; } uint32_t InitialRwin() const { return mInitialRwin; }; // Locked methods to get and set timing info @@ -253,7 +255,7 @@ private: // so far been skipped. uint32_t mInvalidResponseBytesRead; - Http2PushedStream *mPushedStream; + RefPtr mPushedStream; uint32_t mInitialRwin; nsHttpChunkedDecoder *mChunkedDecoder;