Bug 1514688 - In case we already have a h2 connection and it does not support websockets, we should not try again to use websockets over h2. r=michal

There are 2 parts of the patch:
1) we should not try to use websockets over h2 if we already know that they are not supported.
2) make sure that we clean up websockets waiting for the settings frame when we close a h2 connection. (the part 1) will fix the issue, this part is only to be 100% that we some how do not retrigger the issue)

Differential Revision: https://phabricator.services.mozilla.com/D15296

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Dragana Damjanovic 2019-01-07 23:09:35 +00:00
parent 66b1687bfb
commit 7bdd6628ba
2 changed files with 48 additions and 11 deletions

View File

@ -3835,6 +3835,37 @@ void Http2Session::Close(nsresult aReason) {
mStreamIDHash.Clear(); mStreamIDHash.Clear();
mStreamTransactionHash.Clear(); mStreamTransactionHash.Clear();
// If we have any websocket transactions waiting for settings frame,
// reinitiate them. This can happend if we close a h2 connection before the
// settings frame is received.
if (mWaitingWebsockets.Length()) {
MOZ_ASSERT(!mProcessedWaitingWebsockets);
MOZ_ASSERT(mWaitingWebsockets.Length() ==
mWaitingWebsocketCallbacks.Length());
mProcessedWaitingWebsockets = true;
for (size_t i = 0; i < mWaitingWebsockets.Length(); ++i) {
RefPtr<nsAHttpTransaction> httpTransaction = mWaitingWebsockets[i];
LOG3(("Http2Session::Close %p Re-queuing websocket.", this));
httpTransaction->SetConnection(nullptr);
nsHttpTransaction *trans = httpTransaction->QueryHttpTransaction();
if (trans) {
nsresult rv =
gHttpHandler->InitiateTransaction(trans, trans->Priority());
if (NS_FAILED(rv)) {
LOG3(
("Http2Session::Close %p failed to reinitiate websocket "
"transaction (%08x).\n",
this, static_cast<uint32_t>(rv)));
}
} else {
LOG3(("Http2Session::Close %p missing transaction?!", this));
}
}
mWaitingWebsockets.Clear();
mWaitingWebsocketCallbacks.Clear();
}
uint32_t goAwayReason; uint32_t goAwayReason;
if (mGoAwayReason != NO_HTTP_ERROR) { if (mGoAwayReason != NO_HTTP_ERROR) {
goAwayReason = mGoAwayReason; goAwayReason = mGoAwayReason;

View File

@ -1488,18 +1488,24 @@ nsresult nsHttpConnectionMgr::TryDispatchTransaction(
if (!(caps & NS_HTTP_DISALLOW_SPDY) && gHttpHandler->IsSpdyEnabled()) { if (!(caps & NS_HTTP_DISALLOW_SPDY) && gHttpHandler->IsSpdyEnabled()) {
RefPtr<nsHttpConnection> conn = GetSpdyActiveConn(ent); RefPtr<nsHttpConnection> conn = GetSpdyActiveConn(ent);
if (conn) { if (conn) {
bool websocketCheckOK = if (trans->IsWebsocketUpgrade() && !conn->CanAcceptWebsocket()) {
trans->IsWebsocketUpgrade() ? conn->CanAcceptWebsocket() : true; // This is a websocket transaction and we already have a h2 connection
if (websocketCheckOK && ((caps & NS_HTTP_ALLOW_KEEPALIVE) || // that do not support websockets, we should disable h2 for this
(caps & NS_HTTP_ALLOW_SPDY_WITHOUT_KEEPALIVE) || // transaction.
!conn->IsExperienced())) { trans->DisableSpdy();
LOG((" dispatch to spdy: [conn=%p]\n", conn.get())); caps &= NS_HTTP_DISALLOW_SPDY;
trans->RemoveDispatchedAsBlocking(); /* just in case */ } else {
nsresult rv = DispatchTransaction(ent, trans, conn); if ((caps & NS_HTTP_ALLOW_KEEPALIVE) ||
NS_ENSURE_SUCCESS(rv, rv); (caps & NS_HTTP_ALLOW_SPDY_WITHOUT_KEEPALIVE) ||
return NS_OK; !conn->IsExperienced()) {
LOG((" dispatch to spdy: [conn=%p]\n", conn.get()));
trans->RemoveDispatchedAsBlocking(); /* just in case */
nsresult rv = DispatchTransaction(ent, trans, conn);
NS_ENSURE_SUCCESS(rv, rv);
return NS_OK;
}
unusedSpdyPersistentConnection = conn;
} }
unusedSpdyPersistentConnection = conn;
} }
} }