Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(916)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: net/quic/chromium/quic_stream_factory_test.cc

Issue 2319343004: Makes migration on write error asynchronous to avoid reentrancy issues (Closed)
Patch Set: Cleaning up. Created 4 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« net/quic/chromium/quic_stream_factory.cc ('K') | « net/quic/chromium/quic_stream_factory.cc ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: net/quic/chromium/quic_stream_factory_test.cc
diff --git a/net/quic/chromium/quic_stream_factory_test.cc b/net/quic/chromium/quic_stream_factory_test.cc
index 2ca8dbe9f779857174a7f32dec655cf4b8cc3dba..6428dc465ef7b3937c76ff2247b18b80e5fa3020 100644
--- a/net/quic/chromium/quic_stream_factory_test.cc
+++ b/net/quic/chromium/quic_stream_factory_test.cc
@@ -507,6 +507,14 @@ class QuicStreamFactoryTestBase {
EXPECT_TRUE(socket_data2.AllWriteDataConsumed());
}
+ // Helper methods for tests of connection migration on write error.
+ void MigrateSessionOnWriteErrorNonMigratableStream(IoMode mode);
+ void MigrateSessionOnWriteErrorMigrationDisabled(IoMode mode);
+ void MigrateSessionOnWriteError(IoMode mode);
+ void MigrateSessionOnWriteErrorNoNewNetwork(IoMode mode);
+ void MigrateSessionOnMultipleWriteErrors(IoMode mode1, IoMode mode2);
+ void MigrateSessionOnWriteErrorWithNotificationQueued(bool disconnected);
+
MockHostResolver host_resolver_;
scoped_refptr<SSLConfigService> ssl_config_service_;
MockClientSocketFactory socket_factory_;
@@ -2412,7 +2420,15 @@ TEST_P(QuicStreamFactoryTest, MigrateSessionEarlyConnectionMigrationDisabled) {
EXPECT_TRUE(socket_data.AllWriteDataConsumed());
}
-TEST_P(QuicStreamFactoryTest, MigrateSessionOnWriteError) {
+TEST_P(QuicStreamFactoryTest, MigrateSessionOnWriteErrorSynchronous) {
+ MigrateSessionOnWriteError(SYNCHRONOUS);
+}
+
+TEST_P(QuicStreamFactoryTest, MigrateSessionOnWriteErrorAsync) {
+ MigrateSessionOnWriteError(ASYNC);
+}
+
+void QuicStreamFactoryTestBase::MigrateSessionOnWriteError(IoMode mode) {
InitializeConnectionMigrationTest(
{kDefaultNetworkForTests, kNewNetworkForTests});
ProofVerifyDetailsChromium verify_details = DefaultProofVerifyDetails();
@@ -2421,7 +2437,7 @@ TEST_P(QuicStreamFactoryTest, MigrateSessionOnWriteError) {
MockQuicData socket_data;
socket_data.AddRead(SYNCHRONOUS, ERR_IO_PENDING);
- socket_data.AddWrite(SYNCHRONOUS, ERR_ADDRESS_UNREACHABLE);
+ socket_data.AddWrite(mode, ERR_ADDRESS_UNREACHABLE);
socket_data.AddSocketDataToFactory(&socket_factory_);
// Create request and QuicHttpStream.
@@ -2466,8 +2482,8 @@ TEST_P(QuicStreamFactoryTest, MigrateSessionOnWriteError) {
EXPECT_EQ(OK, stream->SendRequest(request_headers, &response,
callback_.callback()));
- // Run the message loop so that data queued in the new socket is read by the
- // packet reader.
+ // Run the message loop so that the migration attempt is executed and
+ // data queued in the new socket is read by the packet reader.
base::RunLoop().RunUntilIdle();
// The session should now be marked as going away. Ensure that
@@ -2489,14 +2505,24 @@ TEST_P(QuicStreamFactoryTest, MigrateSessionOnWriteError) {
EXPECT_TRUE(socket_data1.AllWriteDataConsumed());
}
-TEST_P(QuicStreamFactoryTest, MigrateSessionOnWriteErrorNoNewNetwork) {
+TEST_P(QuicStreamFactoryTest,
+ MigrateSessionOnWriteErrorNoNewNetworkSynchronous) {
+ MigrateSessionOnWriteErrorNoNewNetwork(SYNCHRONOUS);
+}
+
+TEST_P(QuicStreamFactoryTest, MigrateSessionOnWriteErrorNoNewNetworkAsync) {
+ MigrateSessionOnWriteErrorNoNewNetwork(ASYNC);
+}
+
+void QuicStreamFactoryTestBase::MigrateSessionOnWriteErrorNoNewNetwork(
+ IoMode mode) {
InitializeConnectionMigrationTest({kDefaultNetworkForTests});
ProofVerifyDetailsChromium verify_details = DefaultProofVerifyDetails();
crypto_client_stream_factory_.AddProofVerifyDetails(&verify_details);
MockQuicData socket_data;
socket_data.AddRead(SYNCHRONOUS, ERR_IO_PENDING);
- socket_data.AddWrite(SYNCHRONOUS, ERR_ADDRESS_UNREACHABLE);
+ socket_data.AddWrite(mode, ERR_ADDRESS_UNREACHABLE);
socket_data.AddSocketDataToFactory(&socket_factory_);
// Create request and QuicHttpStream.
@@ -2525,18 +2551,31 @@ TEST_P(QuicStreamFactoryTest, MigrateSessionOnWriteErrorNoNewNetwork) {
// a connection migration attempt.
HttpResponseInfo response;
HttpRequestHeaders request_headers;
- EXPECT_EQ(
- ERR_QUIC_PROTOCOL_ERROR,
- stream->SendRequest(request_headers, &response, callback_.callback()));
-
- // Migration fails, and session is marked as going away.
+ EXPECT_EQ(OK, stream->SendRequest(request_headers, &response,
+ callback_.callback()));
+ // Run message loop to execute migration attempt.
+ base::RunLoop().RunUntilIdle();
+ // Migration fails, and session is closed and deleted.
+ EXPECT_FALSE(QuicStreamFactoryPeer::IsLiveSession(factory_.get(), session));
EXPECT_FALSE(HasActiveSession(host_port_pair_));
EXPECT_TRUE(socket_data.AllReadDataConsumed());
EXPECT_TRUE(socket_data.AllWriteDataConsumed());
}
-TEST_P(QuicStreamFactoryTest, MigrateSessionOnWriteErrorNonMigratableStream) {
+TEST_P(QuicStreamFactoryTest,
+ MigrateSessionOnWriteErrorNonMigratableStreamSynchronous) {
+ MigrateSessionOnWriteErrorNonMigratableStream(SYNCHRONOUS);
+}
+
+TEST_P(QuicStreamFactoryTest,
+ MigrateSessionOnWriteErrorNonMigratableStreamAsync) {
+ MigrateSessionOnWriteErrorNonMigratableStream(ASYNC);
+}
+
+void QuicStreamFactoryTestBase::MigrateSessionOnWriteErrorNonMigratableStream(
+ IoMode mode) {
+ DVLOG(1) << "Mode: " << ((mode == SYNCHRONOUS) ? "SYNCHRONOUS" : "ASYNC");
InitializeConnectionMigrationTest(
{kDefaultNetworkForTests, kNewNetworkForTests});
ProofVerifyDetailsChromium verify_details = DefaultProofVerifyDetails();
@@ -2544,7 +2583,7 @@ TEST_P(QuicStreamFactoryTest, MigrateSessionOnWriteErrorNonMigratableStream) {
MockQuicData socket_data;
socket_data.AddRead(SYNCHRONOUS, ERR_IO_PENDING);
- socket_data.AddWrite(SYNCHRONOUS, ERR_ADDRESS_UNREACHABLE);
+ socket_data.AddWrite(mode, ERR_ADDRESS_UNREACHABLE);
socket_data.AddSocketDataToFactory(&socket_factory_);
// Create request and QuicHttpStream.
@@ -2574,18 +2613,32 @@ TEST_P(QuicStreamFactoryTest, MigrateSessionOnWriteErrorNonMigratableStream) {
// a connection migration attempt.
HttpResponseInfo response;
HttpRequestHeaders request_headers;
- EXPECT_EQ(
- ERR_QUIC_PROTOCOL_ERROR,
- stream->SendRequest(request_headers, &response, callback_.callback()));
+ EXPECT_EQ(OK, stream->SendRequest(request_headers, &response,
+ callback_.callback()));
+
+ // Run message loop to execute migration attempt.
+ base::RunLoop().RunUntilIdle();
- // Migration fails, and session is marked as going away.
+ // Migration fails, and session is closed and deleted.
+ EXPECT_FALSE(QuicStreamFactoryPeer::IsLiveSession(factory_.get(), session));
EXPECT_FALSE(HasActiveSession(host_port_pair_));
EXPECT_TRUE(socket_data.AllReadDataConsumed());
EXPECT_TRUE(socket_data.AllWriteDataConsumed());
}
-TEST_P(QuicStreamFactoryTest, MigrateSessionOnWriteErrorMigrationDisabled) {
+TEST_P(QuicStreamFactoryTest,
+ MigrateSessionOnWriteErrorMigrationDisabledSynchronous) {
+ MigrateSessionOnWriteErrorMigrationDisabled(SYNCHRONOUS);
+}
+
+TEST_P(QuicStreamFactoryTest,
+ MigrateSessionOnWriteErrorMigrationDisabledAsync) {
+ MigrateSessionOnWriteErrorMigrationDisabled(ASYNC);
+}
+
+void QuicStreamFactoryTestBase::MigrateSessionOnWriteErrorMigrationDisabled(
+ IoMode mode) {
InitializeConnectionMigrationTest(
{kDefaultNetworkForTests, kNewNetworkForTests});
ProofVerifyDetailsChromium verify_details = DefaultProofVerifyDetails();
@@ -2593,7 +2646,7 @@ TEST_P(QuicStreamFactoryTest, MigrateSessionOnWriteErrorMigrationDisabled) {
MockQuicData socket_data;
socket_data.AddRead(SYNCHRONOUS, ERR_IO_PENDING);
- socket_data.AddWrite(SYNCHRONOUS, ERR_ADDRESS_UNREACHABLE);
+ socket_data.AddWrite(mode, ERR_ADDRESS_UNREACHABLE);
socket_data.AddSocketDataToFactory(&socket_factory_);
// Create request and QuicHttpStream.
@@ -2626,15 +2679,199 @@ TEST_P(QuicStreamFactoryTest, MigrateSessionOnWriteErrorMigrationDisabled) {
// a connection migration attempt.
HttpResponseInfo response;
HttpRequestHeaders request_headers;
- EXPECT_EQ(
- ERR_QUIC_PROTOCOL_ERROR,
- stream->SendRequest(request_headers, &response, callback_.callback()));
+ EXPECT_EQ(OK, stream->SendRequest(request_headers, &response,
+ callback_.callback()));
+ // Run message loop to execute migration attempt.
+ base::RunLoop().RunUntilIdle();
+ // Migration fails, and session is closed and deleted.
+ EXPECT_FALSE(QuicStreamFactoryPeer::IsLiveSession(factory_.get(), session));
+ EXPECT_FALSE(HasActiveSession(host_port_pair_));
+ EXPECT_TRUE(socket_data.AllReadDataConsumed());
+ EXPECT_TRUE(socket_data.AllWriteDataConsumed());
+}
+
+TEST_P(QuicStreamFactoryTest, MigrateSessionOnMultipleWriteErrorsSyncSync) {
+ MigrateSessionOnMultipleWriteErrors(SYNCHRONOUS, SYNCHRONOUS);
+}
+
+TEST_P(QuicStreamFactoryTest, MigrateSessionOnMultipleWriteErrorsSyncAsync) {
+ MigrateSessionOnMultipleWriteErrors(SYNCHRONOUS, ASYNC);
+}
+
+TEST_P(QuicStreamFactoryTest, MigrateSessionOnMultipleWriteErrorsAsyncSync) {
+ MigrateSessionOnMultipleWriteErrors(ASYNC, SYNCHRONOUS);
+}
+
+TEST_P(QuicStreamFactoryTest, MigrateSessionOnMultipleWriteErrorsAsyncAsync) {
+ MigrateSessionOnMultipleWriteErrors(ASYNC, ASYNC);
+}
+
+void QuicStreamFactoryTestBase::MigrateSessionOnMultipleWriteErrors(
+ IoMode mode1,
+ IoMode mode2) {
+ const int kMaxReadersPerQuicSession = 5;
+ InitializeConnectionMigrationTest(
+ {kDefaultNetworkForTests, kNewNetworkForTests});
+ ProofVerifyDetailsChromium verify_details = DefaultProofVerifyDetails();
+ crypto_client_stream_factory_.AddProofVerifyDetails(&verify_details);
+ crypto_client_stream_factory_.AddProofVerifyDetails(&verify_details);
+
+ // Set up kMaxReadersPerQuicSession socket data providers, since migration
+ // will
+ // cause kMaxReadersPerQuicSession write failures as the session hops
+ // repeatedly
+ // between the two networks.
+ MockQuicData socket_data[kMaxReadersPerQuicSession + 1];
+ for (int i = 0; i <= kMaxReadersPerQuicSession; ++i) {
+ // The last socket is created but never used.
+ if (i < kMaxReadersPerQuicSession) {
+ socket_data[i].AddRead(SYNCHRONOUS, ERR_IO_PENDING);
+ socket_data[i].AddWrite((i % 2 == 0) ? mode1 : mode2, ERR_FAILED);
+ }
+ socket_data[i].AddSocketDataToFactory(&socket_factory_);
+ }
+
+ // Create request and QuicHttpStream.
+ QuicStreamRequest request(factory_.get());
+ EXPECT_EQ(ERR_IO_PENDING,
+ request.Request(host_port_pair_, privacy_mode_,
+ /*cert_verify_flags=*/0, url_, "GET", net_log_,
+ callback_.callback()));
+ EXPECT_EQ(OK, callback_.WaitForResult());
+ std::unique_ptr<QuicHttpStream> stream = request.CreateStream();
+ EXPECT_TRUE(stream.get());
+
+ // Cause QUIC stream to be created.
+ HttpRequestInfo request_info;
+ request_info.method = "GET";
+ request_info.url = GURL("https://www.example.org/");
+ EXPECT_EQ(OK, stream->InitializeStream(&request_info, DEFAULT_PRIORITY,
+ net_log_, CompletionCallback()));
+
+ // Ensure that session is alive and active.
+ QuicChromiumClientSession* session = GetActiveSession(host_port_pair_);
+ EXPECT_TRUE(QuicStreamFactoryPeer::IsLiveSession(factory_.get(), session));
+ EXPECT_TRUE(HasActiveSession(host_port_pair_));
+
+ // Send GET request on stream. This should cause a write error, which triggers
+ // a connection migration attempt.
+ HttpResponseInfo response;
+ HttpRequestHeaders request_headers;
+ EXPECT_EQ(OK, stream->SendRequest(request_headers, &response,
+ callback_.callback()));
+
+ // Run the message loop so that data queued in the new socket is read by the
+ // packet reader.
+ base::RunLoop().RunUntilIdle();
+
+ // The connection should be closed because of a write error after migration.
+ EXPECT_FALSE(QuicStreamFactoryPeer::IsLiveSession(factory_.get(), session));
+ EXPECT_FALSE(HasActiveSession(host_port_pair_));
+
+ // EXPECT_EQ(ERR_QUIC_PROTOCOL_ERROR, callback_.WaitForResult());
+ // EXPECT_EQ(ERR_QUIC_PROTOCOL_ERROR,
+ // stream->ReadResponseHeaders(callback_.callback()));
+
+ stream.reset();
+ for (int i = 0; i <= kMaxReadersPerQuicSession; ++i) {
+ DLOG(INFO) << "Socket number: " << i;
+ EXPECT_TRUE(socket_data[i].AllReadDataConsumed());
+ EXPECT_TRUE(socket_data[i].AllWriteDataConsumed());
+ }
+}
+
+TEST_P(QuicStreamFactoryTest,
+ MigrateSessionOnWriteErrorWithNetworkDisconnected) {
+ MigrateSessionOnWriteErrorWithNotificationQueued(/*disconnected=*/true);
+}
+
+TEST_P(QuicStreamFactoryTest,
+ MigrateSessionOnWriteErrorWithNetworkMadeDefault) {
+ MigrateSessionOnWriteErrorWithNotificationQueued(/*disconnected=*/false);
+}
+
+void QuicStreamFactoryTestBase::
+ MigrateSessionOnWriteErrorWithNotificationQueued(bool disconnected) {
+ InitializeConnectionMigrationTest(
+ {kDefaultNetworkForTests, kNewNetworkForTests});
+ ProofVerifyDetailsChromium verify_details = DefaultProofVerifyDetails();
+ crypto_client_stream_factory_.AddProofVerifyDetails(&verify_details);
+ crypto_client_stream_factory_.AddProofVerifyDetails(&verify_details);
+
+ MockQuicData socket_data;
+ socket_data.AddRead(SYNCHRONOUS, ERR_IO_PENDING);
+ socket_data.AddWrite(SYNCHRONOUS, ERR_ADDRESS_UNREACHABLE);
+ socket_data.AddSocketDataToFactory(&socket_factory_);
+
+ // Create request and QuicHttpStream.
+ QuicStreamRequest request(factory_.get());
+ EXPECT_EQ(ERR_IO_PENDING,
+ request.Request(host_port_pair_, privacy_mode_,
+ /*cert_verify_flags=*/0, url_, "GET", net_log_,
+ callback_.callback()));
+ EXPECT_EQ(OK, callback_.WaitForResult());
+ std::unique_ptr<QuicHttpStream> stream = request.CreateStream();
+ EXPECT_TRUE(stream.get());
+
+ // Cause QUIC stream to be created.
+ HttpRequestInfo request_info;
+ request_info.method = "GET";
+ request_info.url = GURL("https://www.example.org/");
+ EXPECT_EQ(OK, stream->InitializeStream(&request_info, DEFAULT_PRIORITY,
+ net_log_, CompletionCallback()));
- // Migration fails, and session is marked as going away.
+ // Ensure that session is alive and active.
+ QuicChromiumClientSession* session = GetActiveSession(host_port_pair_);
+ EXPECT_TRUE(QuicStreamFactoryPeer::IsLiveSession(factory_.get(), session));
+ EXPECT_TRUE(HasActiveSession(host_port_pair_));
+
+ // Set up second socket data provider that is used after
+ // migration. The request is rewritten to this new socket, and the
+ // response to the request is read on this new socket.
+ MockQuicData socket_data1;
+ socket_data1.AddWrite(
+ ConstructGetRequestPacket(1, kClientDataStreamId1, true, true));
+ socket_data1.AddRead(
+ ConstructOkResponsePacket(1, kClientDataStreamId1, false, false));
+ socket_data1.AddRead(SYNCHRONOUS, ERR_IO_PENDING);
+ socket_data1.AddWrite(client_maker_.MakeAckAndRstPacket(
+ 2, false, kClientDataStreamId1, QUIC_STREAM_CANCELLED, 1, 1, 1, true));
+ socket_data1.AddSocketDataToFactory(&socket_factory_);
+
+ // First queue a network change notification in the message loop.
+ if (disconnected) {
+ scoped_mock_network_change_notifier_->mock_network_change_notifier()
+ ->QueueNetworkDisconnected(kDefaultNetworkForTests);
+ } else {
+ scoped_mock_network_change_notifier_->mock_network_change_notifier()
+ ->QueueNetworkMadeDefault(kNewNetworkForTests);
+ }
+ // Send GET request on stream. This should cause a write error,
+ // which triggers a connection migration attempt. This will queue a
+ // migration attempt behind the notification in the message loop.
+ HttpResponseInfo response;
+ HttpRequestHeaders request_headers;
+ EXPECT_EQ(OK, stream->SendRequest(request_headers, &response,
+ callback_.callback()));
+
+ base::RunLoop().RunUntilIdle();
+ // The session should now be marked as going away. Ensure that
+ // while it is still alive, it is no longer active.
+ EXPECT_TRUE(QuicStreamFactoryPeer::IsLiveSession(factory_.get(), session));
EXPECT_FALSE(HasActiveSession(host_port_pair_));
+ EXPECT_EQ(1u, session->GetNumActiveStreams());
+
+ // Verify that response headers on the migrated socket were delivered to the
+ // stream.
+ EXPECT_EQ(OK, stream->ReadResponseHeaders(callback_.callback()));
+ EXPECT_EQ(200, response.headers->response_code());
+
+ stream.reset();
EXPECT_TRUE(socket_data.AllReadDataConsumed());
EXPECT_TRUE(socket_data.AllWriteDataConsumed());
+ EXPECT_TRUE(socket_data1.AllReadDataConsumed());
+ EXPECT_TRUE(socket_data1.AllWriteDataConsumed());
}
TEST_P(QuicStreamFactoryTest, MigrateSessionEarlyToBadSocket) {
« net/quic/chromium/quic_stream_factory.cc ('K') | « net/quic/chromium/quic_stream_factory.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698