diff --git a/netwerk/protocol/http/SpdySession2.cpp b/netwerk/protocol/http/SpdySession2.cpp index 403c26d60107..03cfaafeeadf 100644 --- a/netwerk/protocol/http/SpdySession2.cpp +++ b/netwerk/protocol/http/SpdySession2.cpp @@ -92,9 +92,12 @@ SpdySession2::ShutdownEnumerator(nsAHttpTransaction *key, // On a clean server hangup the server sets the GoAwayID to be the ID of // the last transaction it processed. If the ID of stream in the - // local session is greater than that it can safely be restarted because the - // server guarantees it was not partially processed. - if (self->mCleanShutdown && (stream->StreamID() > self->mGoAwayID)) + // local stream is greater than that it can safely be restarted because the + // server guarantees it was not partially processed. Streams that have not + // registered an ID haven't actually been sent yet so they can always be + // restarted. + if (self->mCleanShutdown && + (stream->StreamID() > self->mGoAwayID || !stream->HasRegisteredID())) self->CloseStream(stream, NS_ERROR_NET_RESET); // can be restarted else self->CloseStream(stream, NS_ERROR_ABORT); @@ -102,6 +105,22 @@ SpdySession2::ShutdownEnumerator(nsAHttpTransaction *key, return PL_DHASH_NEXT; } +PLDHashOperator +SpdySession2::GoAwayEnumerator(nsAHttpTransaction *key, + nsAutoPtr &stream, + void *closure) +{ + SpdySession2 *self = static_cast(closure); + + // these streams were not processed by the server and can be restarted. + // Do that after the enumerator completes to avoid the risk of + // a restart event re-entrantly modifying this hash. + if (stream->StreamID() > self->mGoAwayID || !stream->HasRegisteredID()) + self->mGoAwayStreamsToRestart.Push(stream); + + return PL_DHASH_NEXT; +} + SpdySession2::~SpdySession2() { LOG3(("SpdySession2::~SpdySession2 %p mDownstreamState=%X", @@ -1325,9 +1344,38 @@ SpdySession2::HandleGoAway(SpdySession2 *self) self->mGoAwayID = PR_ntohl(reinterpret_cast(self->mInputFrameBuffer.get())[2]); self->mCleanShutdown = true; - - LOG3(("SpdySession2::HandleGoAway %p GOAWAY Last-Good-ID 0x%X.", - self, self->mGoAwayID)); + + // Find streams greater than the last-good ID and mark them for deletion + // in the mGoAwayStreamsToRestart queue with the GoAwayEnumerator. They can + // be restarted. + self->mStreamTransactionHash.Enumerate(GoAwayEnumerator, self); + + // Process the streams marked for deletion and restart. + uint32_t size = self->mGoAwayStreamsToRestart.GetSize(); + for (uint32_t count = 0; count < size; ++count) { + SpdyStream2 *stream = + static_cast(self->mGoAwayStreamsToRestart.PopFront()); + + self->CloseStream(stream, NS_ERROR_NET_RESET); + if (stream->HasRegisteredID()) + self->mStreamIDHash.Remove(stream->StreamID()); + self->mStreamTransactionHash.Remove(stream->Transaction()); + } + + // Queued streams can also be deleted from this session and restarted + // in another one. (they were never sent on the network so they implicitly + // are not covered by the last-good id. + size = self->mQueuedStreams.GetSize(); + for (uint32_t count = 0; count < size; ++count) { + SpdyStream2 *stream = + static_cast(self->mQueuedStreams.PopFront()); + self->CloseStream(stream, NS_ERROR_NET_RESET); + self->mStreamTransactionHash.Remove(stream->Transaction()); + } + + LOG3(("SpdySession2::HandleGoAway %p GOAWAY Last-Good-ID 0x%X." + "live streams=%d\n", self, self->mGoAwayID, + self->mStreamTransactionHash.Count())); self->ResumeRecv(); self->ResetDownstreamState(); return NS_OK; diff --git a/netwerk/protocol/http/SpdySession2.h b/netwerk/protocol/http/SpdySession2.h index 0b8ab3aa8e5a..9cd073a35d6a 100644 --- a/netwerk/protocol/http/SpdySession2.h +++ b/netwerk/protocol/http/SpdySession2.h @@ -202,6 +202,10 @@ private: nsAutoPtr &, void *); + static PLDHashOperator GoAwayEnumerator(nsAHttpTransaction *, + nsAutoPtr &, + void *); + // This is intended to be nsHttpConnectionMgr:nsHttpConnectionHandle taken // from the first transaction on this session. That object contains the // pointer to the real network-level nsHttpConnection object. @@ -335,6 +339,9 @@ private: PRIntervalTime mLastDataReadEpoch; // used for IdleTime() PRIntervalTime mPingSentEpoch; uint32_t mNextPingID; + + // used as a temporary buffer while enumerating the stream hash during GoAway + nsDeque mGoAwayStreamsToRestart; }; }} // namespace mozilla::net diff --git a/netwerk/protocol/http/SpdySession3.cpp b/netwerk/protocol/http/SpdySession3.cpp index 1287ba12acde..5c6e82f9d36b 100644 --- a/netwerk/protocol/http/SpdySession3.cpp +++ b/netwerk/protocol/http/SpdySession3.cpp @@ -93,9 +93,12 @@ SpdySession3::ShutdownEnumerator(nsAHttpTransaction *key, // On a clean server hangup the server sets the GoAwayID to be the ID of // the last transaction it processed. If the ID of stream in the - // local session is greater than that it can safely be restarted because the - // server guarantees it was not partially processed. - if (self->mCleanShutdown && (stream->StreamID() > self->mGoAwayID)) + // local stream is greater than that it can safely be restarted because the + // server guarantees it was not partially processed. Streams that have not + // registered an ID haven't actually been sent yet so they can always be + // restarted. + if (self->mCleanShutdown && + (stream->StreamID() > self->mGoAwayID || !stream->HasRegisteredID())) self->CloseStream(stream, NS_ERROR_NET_RESET); // can be restarted else self->CloseStream(stream, NS_ERROR_ABORT); @@ -103,6 +106,22 @@ SpdySession3::ShutdownEnumerator(nsAHttpTransaction *key, return PL_DHASH_NEXT; } +PLDHashOperator +SpdySession3::GoAwayEnumerator(nsAHttpTransaction *key, + nsAutoPtr &stream, + void *closure) +{ + SpdySession3 *self = static_cast(closure); + + // these streams were not processed by the server and can be restarted. + // Do that after the enumerator completes to avoid the risk of + // a restart event re-entrantly modifying this hash. + if (stream->StreamID() > self->mGoAwayID || !stream->HasRegisteredID()) + self->mGoAwayStreamsToRestart.Push(stream); + + return PL_DHASH_NEXT; +} + SpdySession3::~SpdySession3() { LOG3(("SpdySession3::~SpdySession3 %p mDownstreamState=%X", @@ -301,7 +320,6 @@ SpdySession3::AddStream(nsAHttpTransaction *aHttpTransaction, &mUpstreamZlib, aPriority); - LOG3(("SpdySession3::AddStream session=%p stream=%p NextID=0x%X (tentative)", this, stream, mNextStreamID)); @@ -1203,10 +1221,39 @@ SpdySession3::HandleGoAway(SpdySession3 *self) self->mGoAwayID = PR_ntohl(reinterpret_cast(self->mInputFrameBuffer.get())[2]); self->mCleanShutdown = true; - - LOG3(("SpdySession3::HandleGoAway %p GOAWAY Last-Good-ID 0x%X status 0x%X\n", - self, self->mGoAwayID, - PR_ntohl(reinterpret_cast(self->mInputFrameBuffer.get())[3]))); + + // Find streams greater than the last-good ID and mark them for deletion + // in the mGoAwayStreamsToRestart queue with the GoAwayEnumerator. They can + // be restarted. + self->mStreamTransactionHash.Enumerate(GoAwayEnumerator, self); + + // Process the streams marked for deletion and restart. + uint32_t size = self->mGoAwayStreamsToRestart.GetSize(); + for (uint32_t count = 0; count < size; ++count) { + SpdyStream3 *stream = + static_cast(self->mGoAwayStreamsToRestart.PopFront()); + + self->CloseStream(stream, NS_ERROR_NET_RESET); + if (stream->HasRegisteredID()) + self->mStreamIDHash.Remove(stream->StreamID()); + self->mStreamTransactionHash.Remove(stream->Transaction()); + } + + // Queued streams can also be deleted from this session and restarted + // in another one. (they were never sent on the network so they implicitly + // are not covered by the last-good id. + size = self->mQueuedStreams.GetSize(); + for (uint32_t count = 0; count < size; ++count) { + SpdyStream3 *stream = + static_cast(self->mQueuedStreams.PopFront()); + self->CloseStream(stream, NS_ERROR_NET_RESET); + self->mStreamTransactionHash.Remove(stream->Transaction()); + } + + LOG3(("SpdySession3::HandleGoAway %p GOAWAY Last-Good-ID 0x%X status 0x%X " + "live streams=%d\n", self, self->mGoAwayID, + PR_ntohl(reinterpret_cast(self->mInputFrameBuffer.get())[3]), + self->mStreamTransactionHash.Count())); self->ResumeRecv(); self->ResetDownstreamState(); diff --git a/netwerk/protocol/http/SpdySession3.h b/netwerk/protocol/http/SpdySession3.h index 64b3a36690c0..b3d196473af9 100644 --- a/netwerk/protocol/http/SpdySession3.h +++ b/netwerk/protocol/http/SpdySession3.h @@ -213,6 +213,10 @@ private: nsAutoPtr &, void *); + static PLDHashOperator GoAwayEnumerator(nsAHttpTransaction *, + nsAutoPtr &, + void *); + static PLDHashOperator UpdateServerRwinEnumerator(nsAHttpTransaction *, nsAutoPtr &, void *); @@ -346,6 +350,9 @@ private: PRIntervalTime mLastDataReadEpoch; // used for IdleTime() PRIntervalTime mPingSentEpoch; uint32_t mNextPingID; + + // used as a temporary buffer while enumerating the stream hash during GoAway + nsDeque mGoAwayStreamsToRestart; }; }} // namespace mozilla::net diff --git a/netwerk/protocol/http/SpdyStream2.h b/netwerk/protocol/http/SpdyStream2.h index a82a9f2d3ce8..61aa191329fa 100644 --- a/netwerk/protocol/http/SpdyStream2.h +++ b/netwerk/protocol/http/SpdyStream2.h @@ -41,6 +41,8 @@ public: mFullyOpen = 1; } + bool HasRegisteredID() { return mStreamID != 0; } + nsAHttpTransaction *Transaction() { return mTransaction; @@ -121,7 +123,7 @@ private: // The quanta upstream data frames are chopped into uint32_t mChunkSize; - // Flag is set when all http request headers have been read + // Flag is set when all http request headers have been read and ID is stable uint32_t mSynFrameComplete : 1; // Flag is set when the HTTP processor has more data to send diff --git a/netwerk/protocol/http/SpdyStream3.h b/netwerk/protocol/http/SpdyStream3.h index 5bbcc7f070d6..1397fd838b16 100644 --- a/netwerk/protocol/http/SpdyStream3.h +++ b/netwerk/protocol/http/SpdyStream3.h @@ -40,6 +40,8 @@ public: mFullyOpen = 1; } + bool HasRegisteredID() { return mStreamID != 0; } + nsAHttpTransaction *Transaction() { return mTransaction; @@ -143,7 +145,7 @@ private: // The quanta upstream data frames are chopped into uint32_t mChunkSize; - // Flag is set when all http request headers have been read + // Flag is set when all http request headers have been read and ID is stable uint32_t mSynFrameComplete : 1; // Flag is set when the HTTP processor has more data to send