Commit 21860bb8 authored by rtenneti's avatar rtenneti Committed by Commit bot

Fix a QUIC bug where the headers stream becomes flow control blocked and

never writes data again.

Merge internal change: 77265365

R=rch@chromium.org
BUG=423022

Review URL: https://codereview.chromium.org/644243002

Cr-Commit-Position: refs/heads/master@{#299361}
parent 22dd9298
...@@ -528,14 +528,12 @@ void QuicSession::OnNewStreamFlowControlWindow(uint32 new_window) { ...@@ -528,14 +528,12 @@ void QuicSession::OnNewStreamFlowControlWindow(uint32 new_window) {
// Inform all existing streams about the new window. // Inform all existing streams about the new window.
if (connection_->version() >= QUIC_VERSION_21) { if (connection_->version() >= QUIC_VERSION_21) {
GetCryptoStream()->flow_controller()->UpdateSendWindowOffset(new_window); GetCryptoStream()->UpdateSendWindowOffset(new_window);
headers_stream_->flow_controller()->UpdateSendWindowOffset(new_window); headers_stream_->UpdateSendWindowOffset(new_window);
} }
for (DataStreamMap::iterator it = stream_map_.begin(); for (DataStreamMap::iterator it = stream_map_.begin();
it != stream_map_.end(); ++it) { it != stream_map_.end(); ++it) {
if (it->second->flow_controller()->UpdateSendWindowOffset(new_window)) { it->second->UpdateSendWindowOffset(new_window);
it->second->OnCanWrite();
}
} }
} }
......
...@@ -9,6 +9,8 @@ ...@@ -9,6 +9,8 @@
#include "base/basictypes.h" #include "base/basictypes.h"
#include "base/containers/hash_tables.h" #include "base/containers/hash_tables.h"
#include "base/rand_util.h"
#include "base/strings/string_number_conversions.h"
#include "net/quic/crypto/crypto_protocol.h" #include "net/quic/crypto/crypto_protocol.h"
#include "net/quic/quic_crypto_stream.h" #include "net/quic/quic_crypto_stream.h"
#include "net/quic/quic_flags.h" #include "net/quic/quic_flags.h"
...@@ -656,6 +658,81 @@ TEST_P(QuicSessionTest, HandshakeUnblocksFlowControlBlockedStream) { ...@@ -656,6 +658,81 @@ TEST_P(QuicSessionTest, HandshakeUnblocksFlowControlBlockedStream) {
EXPECT_FALSE(stream2->flow_controller()->IsBlocked()); EXPECT_FALSE(stream2->flow_controller()->IsBlocked());
} }
TEST_P(QuicSessionTest, HandshakeUnblocksFlowControlBlockedCryptoStream) {
if (version() <= QUIC_VERSION_19) {
return;
}
// Test that if the crypto stream is flow control blocked, then if the SHLO
// contains a larger send window offset, the stream becomes unblocked.
session_.set_writev_consumes_all_data(true);
TestCryptoStream* crypto_stream = session_.GetCryptoStream();
EXPECT_FALSE(crypto_stream->flow_controller()->IsBlocked());
QuicHeadersStream* headers_stream =
QuicSessionPeer::GetHeadersStream(&session_);
EXPECT_FALSE(headers_stream->flow_controller()->IsBlocked());
// Write until the crypto stream is flow control blocked.
int i = 0;
while (!crypto_stream->flow_controller()->IsBlocked() && i < 1000) {
QuicConfig config;
CryptoHandshakeMessage crypto_message;
config.ToHandshakeMessage(&crypto_message);
crypto_stream->SendHandshakeMessage(crypto_message);
++i;
}
EXPECT_TRUE(crypto_stream->flow_controller()->IsBlocked());
EXPECT_FALSE(headers_stream->flow_controller()->IsBlocked());
EXPECT_FALSE(session_.HasDataToWrite());
EXPECT_TRUE(crypto_stream->HasBufferedData());
// The handshake message will call OnCanWrite, so the stream can
// resume writing.
EXPECT_CALL(*crypto_stream, OnCanWrite());
// Now complete the crypto handshake, resulting in an increased flow control
// send window.
CryptoHandshakeMessage msg;
session_.GetCryptoStream()->OnHandshakeMessage(msg);
// Stream is now unblocked and will no longer have buffered data.
EXPECT_FALSE(crypto_stream->flow_controller()->IsBlocked());
}
TEST_P(QuicSessionTest, HandshakeUnblocksFlowControlBlockedHeadersStream) {
if (version() <= QUIC_VERSION_19) {
return;
}
// Test that if the header stream is flow control blocked, then if the SHLO
// contains a larger send window offset, the stream becomes unblocked.
session_.set_writev_consumes_all_data(true);
TestCryptoStream* crypto_stream = session_.GetCryptoStream();
EXPECT_FALSE(crypto_stream->flow_controller()->IsBlocked());
QuicHeadersStream* headers_stream =
QuicSessionPeer::GetHeadersStream(&session_);
EXPECT_FALSE(headers_stream->flow_controller()->IsBlocked());
QuicStreamId stream_id = 5;
// Write until the header stream is flow control blocked.
while (!headers_stream->flow_controller()->IsBlocked() && stream_id < 2000) {
SpdyHeaderBlock headers;
headers["header"] = base::Uint64ToString(base::RandUint64()) +
base::Uint64ToString(base::RandUint64()) +
base::Uint64ToString(base::RandUint64());
headers_stream->WriteHeaders(stream_id, headers, true, nullptr);
stream_id += 2;
}
EXPECT_TRUE(headers_stream->flow_controller()->IsBlocked());
EXPECT_FALSE(crypto_stream->flow_controller()->IsBlocked());
EXPECT_FALSE(session_.HasDataToWrite());
EXPECT_TRUE(headers_stream->HasBufferedData());
// Now complete the crypto handshake, resulting in an increased flow control
// send window.
CryptoHandshakeMessage msg;
session_.GetCryptoStream()->OnHandshakeMessage(msg);
// Stream is now unblocked and will no longer have buffered data.
EXPECT_FALSE(headers_stream->flow_controller()->IsBlocked());
EXPECT_FALSE(headers_stream->HasBufferedData());
}
TEST_P(QuicSessionTest, InvalidFlowControlWindowInHandshake) { TEST_P(QuicSessionTest, InvalidFlowControlWindowInHandshake) {
// TODO(rjshade): Remove this test when removing QUIC_VERSION_19. // TODO(rjshade): Remove this test when removing QUIC_VERSION_19.
// Test that receipt of an invalid (< default) flow control window from // Test that receipt of an invalid (< default) flow control window from
......
...@@ -532,6 +532,12 @@ void ReliableQuicStream::AddBytesConsumed(uint64 bytes) { ...@@ -532,6 +532,12 @@ void ReliableQuicStream::AddBytesConsumed(uint64 bytes) {
} }
} }
void ReliableQuicStream::UpdateSendWindowOffset(uint64 new_window) {
if (flow_controller_.UpdateSendWindowOffset(new_window)) {
OnCanWrite();
}
}
bool ReliableQuicStream::IsFlowControlBlocked() { bool ReliableQuicStream::IsFlowControlBlocked() {
if (flow_controller_.IsBlocked()) { if (flow_controller_.IsBlocked()) {
return true; return true;
......
...@@ -111,6 +111,10 @@ class NET_EXPORT_PRIVATE ReliableQuicStream { ...@@ -111,6 +111,10 @@ class NET_EXPORT_PRIVATE ReliableQuicStream {
// WINDOW_UPDATE frame. // WINDOW_UPDATE frame.
void AddBytesConsumed(uint64 bytes); void AddBytesConsumed(uint64 bytes);
// Updates the flow controller's send window offset and calls OnCanWrite if
// it was blocked before.
void UpdateSendWindowOffset(uint64 new_offset);
// Returns true if the stream is flow control blocked, by the stream flow // Returns true if the stream is flow control blocked, by the stream flow
// control window or the connection flow control window. // control window or the connection flow control window.
bool IsFlowControlBlocked(); bool IsFlowControlBlocked();
...@@ -123,6 +127,9 @@ class NET_EXPORT_PRIVATE ReliableQuicStream { ...@@ -123,6 +127,9 @@ class NET_EXPORT_PRIVATE ReliableQuicStream {
return fin_received_ || rst_received_; return fin_received_ || rst_received_;
} }
// Returns true if the stream has queued data waiting to write.
bool HasBufferedData() const;
protected: protected:
// Sends as much of 'data' to the connection as the connection will consume, // Sends as much of 'data' to the connection as the connection will consume,
// and then buffers any remaining data in queued_data_. // and then buffers any remaining data in queued_data_.
...@@ -151,8 +158,6 @@ class NET_EXPORT_PRIVATE ReliableQuicStream { ...@@ -151,8 +158,6 @@ class NET_EXPORT_PRIVATE ReliableQuicStream {
// Close the write side of the socket. Further writes will fail. // Close the write side of the socket. Further writes will fail.
void CloseWriteSide(); void CloseWriteSide();
bool HasBufferedData() const;
bool fin_buffered() const { return fin_buffered_; } bool fin_buffered() const { return fin_buffered_; }
const QuicSession* session() const { return session_; } const QuicSession* session() const { return session_; }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment