From 91de3341df7e08094e17a34053b8e21c89ab02a7 Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Sun, 21 Jul 2019 21:30:26 +0200 Subject: Revert "Implement a threadsafe & revised version of http2PushedStream." Backed out because of gcc build failures. This reverts commit 66fae1d81013a2321e7d607a426f834a01b847ce. --- netwerk/protocol/http/Http2Push.cpp | 81 ++------------------------- netwerk/protocol/http/Http2Push.h | 18 ------ netwerk/protocol/http/Http2Session.cpp | 24 ++------ netwerk/protocol/http/Http2Stream.cpp | 4 +- netwerk/protocol/http/Http2Stream.h | 1 - netwerk/protocol/http/nsHttpChannel.cpp | 4 +- netwerk/protocol/http/nsHttpChannel.h | 9 ++- netwerk/protocol/http/nsHttpConnectionMgr.cpp | 19 +++---- netwerk/protocol/http/nsHttpTransaction.h | 16 +++--- 9 files changed, 32 insertions(+), 144 deletions(-) (limited to 'netwerk') diff --git a/netwerk/protocol/http/Http2Push.cpp b/netwerk/protocol/http/Http2Push.cpp index 34fc425d2..b6fc485e2 100644 --- a/netwerk/protocol/http/Http2Push.cpp +++ b/netwerk/protocol/http/Http2Push.cpp @@ -30,8 +30,8 @@ class CallChannelOnPush final : public Runnable { Http2PushedStream *pushStream) : mAssociatedChannel(associatedChannel) , mPushedURI(pushedURI) + , mPushedStream(pushStream) { - mPushedStreamWrapper = new Http2PushedStreamWrapper(pushStream); } NS_IMETHOD Run() override @@ -40,94 +40,21 @@ class CallChannelOnPush final : public Runnable { RefPtr channel; CallQueryInterface(mAssociatedChannel, channel.StartAssignment()); MOZ_ASSERT(channel); - if (channel && NS_SUCCEEDED(channel->OnPush(mPushedURI, mPushedStreamWrapper))) { + if (channel && NS_SUCCEEDED(channel->OnPush(mPushedURI, mPushedStream))) { return NS_OK; } LOG3(("Http2PushedStream Orphan %p failed OnPush\n", this)); - mPushedStreamWrapper->OnPushFailed(); + mPushedStream->OnPushFailed(); return NS_OK; } private: nsCOMPtr mAssociatedChannel; const nsCString mPushedURI; - RefPtr mPushedStreamWrapper; + Http2PushedStream *mPushedStream; }; -// 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) { - return false; - } - - gSocketTransportService->Dispatch( - 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) { - if (mStream) { - Http2Stream* stream = mStream; - static_cast(stream)->OnPushFailed(); - } - } else { - gSocketTransportService->Dispatch( - NewRunnableMethod(this, &Http2PushedStreamWrapper::OnPushFailed), - NS_DISPATCH_NORMAL); - } -} - ////////////////////////////////////////// // Http2PushedStream ////////////////////////////////////////// diff --git a/netwerk/protocol/http/Http2Push.h b/netwerk/protocol/http/Http2Push.h index d4b71c1ef..fd39eb2c7 100644 --- a/netwerk/protocol/http/Http2Push.h +++ b/netwerk/protocol/http/Http2Push.h @@ -123,24 +123,6 @@ 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/Http2Session.cpp b/netwerk/protocol/http/Http2Session.cpp index 86e8c74f6..4a178f091 100644 --- a/netwerk/protocol/http/Http2Session.cpp +++ b/netwerk/protocol/http/Http2Session.cpp @@ -380,24 +380,12 @@ Http2Session::AddStream(nsAHttpTransaction *aHttpTransaction, if (mClosed || mShouldGoAway) { nsHttpTransaction *trans = aHttpTransaction->QueryHttpTransaction(); - if (trans) { - RefPtr pushedStreamWrapper; - pushedStreamWrapper = trans->GetPushedStream(); - if (!pushedStreamWrapper || !pushedStreamWrapper->GetStream()) { - LOG3( - ("Http2Session::AddStream %p atrans=%p trans=%p session unusable - " - "resched.\n", this, aHttpTransaction, trans)); - aHttpTransaction->SetConnection(nullptr); - nsresult rv = - gHttpHandler->InitiateTransaction(trans, trans->Priority()); - if (NS_FAILED(rv)) { - LOG3( - ("Http2Session::AddStream %p atrans=%p trans=%p failed to " - "initiate transaction (%08x).\n", - this, aHttpTransaction, trans, static_cast(rv))); - } - return true; - } + if (trans && !trans->GetPushedStream()) { + LOG3(("Http2Session::AddStream %p atrans=%p trans=%p session unusable - resched.\n", + this, aHttpTransaction, trans)); + aHttpTransaction->SetConnection(nullptr); + gHttpHandler->InitiateTransaction(trans, trans->Priority()); + return true; } } diff --git a/netwerk/protocol/http/Http2Stream.cpp b/netwerk/protocol/http/Http2Stream.cpp index 22d8142c9..581ebe016 100644 --- a/netwerk/protocol/http/Http2Stream.cpp +++ b/netwerk/protocol/http/Http2Stream.cpp @@ -442,14 +442,12 @@ Http2Stream::ParseHttpRequestHeaders(const char *buf, requestContext->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 && (pushedStreamWrapper = trans->TakePushedStream()) && - (pushedStream = pushedStreamWrapper->GetStream())) { + if (trans && (pushedStream = trans->TakePushedStream())) { if (pushedStream->mSession == mSession) { LOG3(("Pushed Stream match based on OnPush correlation %p", pushedStream)); } else { diff --git a/netwerk/protocol/http/Http2Stream.h b/netwerk/protocol/http/Http2Stream.h index 30ade870f..8783eefed 100644 --- a/netwerk/protocol/http/Http2Stream.h +++ b/netwerk/protocol/http/Http2Stream.h @@ -28,7 +28,6 @@ class Http2Decompressor; class Http2Stream : public nsAHttpSegmentReader , public nsAHttpSegmentWriter - , public SupportsWeakPtr { public: NS_DECL_NSAHTTPSEGMENTREADER diff --git a/netwerk/protocol/http/nsHttpChannel.cpp b/netwerk/protocol/http/nsHttpChannel.cpp index 05383916f..481df5ff0 100644 --- a/netwerk/protocol/http/nsHttpChannel.cpp +++ b/netwerk/protocol/http/nsHttpChannel.cpp @@ -7828,7 +7828,7 @@ nsHttpChannel::AwaitingCacheCallbacks() } void -nsHttpChannel::SetPushedStream(Http2PushedStreamWrapper *stream) +nsHttpChannel::SetPushedStream(Http2PushedStream *stream) { MOZ_ASSERT(stream); MOZ_ASSERT(!mPushedStream); @@ -7836,7 +7836,7 @@ nsHttpChannel::SetPushedStream(Http2PushedStreamWrapper *stream) } nsresult -nsHttpChannel::OnPush(const nsACString &url, Http2PushedStreamWrapper *pushedStream) +nsHttpChannel::OnPush(const nsACString &url, Http2PushedStream *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 defd710c3..0038e1f71 100644 --- a/netwerk/protocol/http/nsHttpChannel.h +++ b/netwerk/protocol/http/nsHttpChannel.h @@ -126,7 +126,7 @@ public: const nsID& aChannelId, nsContentPolicyType aContentPolicyType) override; - nsresult OnPush(const nsACString &uri, Http2PushedStreamWrapper *pushedStream); + nsresult OnPush(const nsACString &uri, Http2PushedStream *pushedStream); static bool IsRedirectStatus(uint32_t status); @@ -448,7 +448,7 @@ private: nsresult OpenCacheInputStream(nsICacheEntry* cacheEntry, bool startBuffering, bool checkingAppCacheEntry); - void SetPushedStream(Http2PushedStreamWrapper *stream); + void SetPushedStream(Http2PushedStream *stream); void SetDoNotTrack(); @@ -578,10 +578,9 @@ private: nsTArray mRedirectFuncStack; // Needed for accurate DNS timing - RefPtr mDNSPrefetch; + RefPtr mDNSPrefetch; - RefPtr mPushedStream; - + Http2PushedStream *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 d402b4104..71c19a9ec 100644 --- a/netwerk/protocol/http/nsHttpConnectionMgr.cpp +++ b/netwerk/protocol/http/nsHttpConnectionMgr.cpp @@ -1827,18 +1827,13 @@ nsHttpConnectionMgr::ProcessNewTransaction(nsHttpTransaction *trans) trans->SetPendingTime(); - RefPtr pushedStreamWrapper = - trans->GetPushedStream(); - if (pushedStreamWrapper) { - Http2PushedStream* pushedStream = pushedStreamWrapper->GetStream(); - if (pushedStream) { - LOG((" ProcessNewTransaction %p tied to h2 session push %p\n", trans, - pushedStream->Session())); - return pushedStream->Session()->AddStream(trans, trans->Priority(), false, - nullptr) - ? NS_OK - : NS_ERROR_UNEXPECTED; - } + Http2PushedStream *pushedStream = trans->GetPushedStream(); + if (pushedStream) { + LOG((" ProcessNewTransaction %p tied to h2 session push %p\n", + trans, pushedStream->Session())); + 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 1197bd98e..262796d71 100644 --- a/netwerk/protocol/http/nsHttpTransaction.h +++ b/netwerk/protocol/http/nsHttpTransaction.h @@ -131,14 +131,14 @@ public: nsHttpTransaction *QueryHttpTransaction() override { return this; } - already_AddRefed GetPushedStream() { - return do_AddRef(mPushedStream); + Http2PushedStream *GetPushedStream() { return mPushedStream; } + Http2PushedStream *TakePushedStream() + { + Http2PushedStream *r = mPushedStream; + mPushedStream = nullptr; + return r; } - already_AddRefed TakePushedStream() { - return mPushedStream.forget(); - } - - void SetPushedStream(Http2PushedStreamWrapper* push) { mPushedStream = push; } + void SetPushedStream(Http2PushedStream *push) { mPushedStream = push; } uint32_t InitialRwin() const { return mInitialRwin; }; bool ChannelPipeFull() { return mWaitingOnPipeOut; } @@ -264,7 +264,7 @@ private: // so far been skipped. uint32_t mInvalidResponseBytesRead; - RefPtr mPushedStream; + Http2PushedStream *mPushedStream; uint32_t mInitialRwin; nsHttpChunkedDecoder *mChunkedDecoder; -- cgit v1.2.3