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

Side by Side Diff: net/quic/chromium/quic_chromium_client_session.cc

Issue 2319343004: Makes migration on write error asynchronous to avoid reentrancy issues (Closed)
Patch Set: synced and rebased 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 unified diff | Download patch
OLDNEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "net/quic/chromium/quic_chromium_client_session.h" 5 #include "net/quic/chromium/quic_chromium_client_session.h"
6 6
7 #include <openssl/ssl.h> 7 #include <openssl/ssl.h>
8 8
9 #include <utility> 9 #include <utility>
10 10
(...skipping 223 matching lines...) Expand 10 before | Expand all | Expand 10 after
234 logger_(new QuicConnectionLogger(this, 234 logger_(new QuicConnectionLogger(this,
235 connection_description, 235 connection_description,
236 std::move(socket_performance_watcher), 236 std::move(socket_performance_watcher),
237 net_log_)), 237 net_log_)),
238 going_away_(false), 238 going_away_(false),
239 port_migration_detected_(false), 239 port_migration_detected_(false),
240 disabled_reason_(QUIC_DISABLED_NOT), 240 disabled_reason_(QUIC_DISABLED_NOT),
241 token_binding_signatures_(kTokenBindingSignatureMapSize), 241 token_binding_signatures_(kTokenBindingSignatureMapSize),
242 streams_pushed_count_(0), 242 streams_pushed_count_(0),
243 streams_pushed_and_claimed_count_(0), 243 streams_pushed_and_claimed_count_(0),
244 error_code_from_rewrite_(OK), 244 migration_pending_(false),
245 use_error_code_from_rewrite_(false),
246 weak_factory_(this) { 245 weak_factory_(this) {
247 sockets_.push_back(std::move(socket)); 246 sockets_.push_back(std::move(socket));
248 packet_readers_.push_back(base::MakeUnique<QuicChromiumPacketReader>( 247 packet_readers_.push_back(base::MakeUnique<QuicChromiumPacketReader>(
249 sockets_.back().get(), clock, this, yield_after_packets, 248 sockets_.back().get(), clock, this, yield_after_packets,
250 yield_after_duration, net_log_)); 249 yield_after_duration, net_log_));
251 crypto_stream_.reset( 250 crypto_stream_.reset(
252 crypto_client_stream_factory->CreateQuicCryptoClientStream( 251 crypto_client_stream_factory->CreateQuicCryptoClientStream(
253 server_id, this, base::MakeUnique<ProofVerifyContextChromium>( 252 server_id, this, base::MakeUnique<ProofVerifyContextChromium>(
254 cert_verify_flags, net_log_), 253 cert_verify_flags, net_log_),
255 crypto_config)); 254 crypto_config));
(...skipping 706 matching lines...) Expand 10 before | Expand all | Expand 10 after
962 961
963 void QuicChromiumClientSession::OnSuccessfulVersionNegotiation( 962 void QuicChromiumClientSession::OnSuccessfulVersionNegotiation(
964 const QuicVersion& version) { 963 const QuicVersion& version) {
965 logger_->OnSuccessfulVersionNegotiation(version); 964 logger_->OnSuccessfulVersionNegotiation(version);
966 QuicSpdySession::OnSuccessfulVersionNegotiation(version); 965 QuicSpdySession::OnSuccessfulVersionNegotiation(version);
967 } 966 }
968 967
969 int QuicChromiumClientSession::HandleWriteError( 968 int QuicChromiumClientSession::HandleWriteError(
970 int error_code, 969 int error_code,
971 scoped_refptr<StringIOBuffer> packet) { 970 scoped_refptr<StringIOBuffer> packet) {
971 if (stream_factory_ == nullptr ||
972 !stream_factory_->migrate_sessions_on_network_change()) {
973 return error_code;
974 }
972 DCHECK(packet != nullptr); 975 DCHECK(packet != nullptr);
973 use_error_code_from_rewrite_ = false; 976 DCHECK_NE(ERR_IO_PENDING, error_code);
974 if (stream_factory_) { 977 DCHECK_GT(0, error_code);
975 stream_factory_->MaybeMigrateSingleSession(this, WRITE_ERROR, packet); 978 DCHECK(!migration_pending_);
979 DCHECK(packet_ == nullptr);
980
981 // Post a task to migrate the session onto a new network.
982 task_runner_->PostTask(
983 FROM_HERE,
984 base::Bind(&QuicChromiumClientSession::MigrateSessionOnWriteError,
985 weak_factory_.GetWeakPtr()));
986
987 // Store packet in the session since the actual migration and packet rewrite
988 // can happen via this posted task or via an async network notification.
989 packet_ = packet;
990 migration_pending_ = true;
991
992 // Cause the packet writer to return ERR_IO_PENDING and block so
993 // that the actual migration happens from the message loop instead
994 // of under the call stack of QuicConnection::WritePacket.
995 return ERR_IO_PENDING;
996 }
997
998 void QuicChromiumClientSession::MigrateSessionOnWriteError() {
999 // If migration_pending_ is false, an earlier task completed migration.
1000 if (!migration_pending_)
1001 return;
1002
1003 if (stream_factory_ != nullptr &&
1004 stream_factory_->MaybeMigrateSingleSession(this, WRITE_ERROR) ==
1005 MigrationResult::SUCCESS) {
1006 return;
976 } 1007 }
977 return use_error_code_from_rewrite_ ? error_code_from_rewrite_ : error_code; 1008
1009 // Close the connection if migration failed. Do not cause a
1010 // connection close packet to be sent since socket may be borked.
1011 connection()->CloseConnection(QUIC_PACKET_WRITE_ERROR,
1012 "Write and subsequent migration failed",
1013 ConnectionCloseBehavior::SILENT_CLOSE);
1014 }
1015
1016 void QuicChromiumClientSession::WriteToNewSocket() {
1017 // Prevent any pending migration from executing.
1018 migration_pending_ = false;
1019
1020 static_cast<QuicChromiumPacketWriter*>(connection()->writer())
1021 ->set_write_blocked(false);
1022 DCHECK(!connection()->writer()->IsWriteBlocked());
1023
1024 if (packet_ == nullptr) {
1025 // Unblock the connection before sending a PING packet, since it
1026 // may have been blocked before the migration started.
1027 connection()->OnCanWrite();
1028 connection()->SendPing();
1029 return;
1030 }
1031
1032 // Set packet_ to null first before calling WritePacketToSocket since
1033 // that method may set packet_ if there is a write error.
1034 scoped_refptr<StringIOBuffer> packet = packet_;
1035 packet_ = nullptr;
1036
1037 // The connection is waiting for the original write to complete
1038 // asynchronously. The new writer will notify the connection if the
1039 // write below completes asynchronously, but a synchronous competion
1040 // must be propagated back to the connection here.
1041 WriteResult result =
1042 static_cast<QuicChromiumPacketWriter*>(connection()->writer())
1043 ->WritePacketToSocket(packet);
1044
1045 if (result.error_code == ERR_IO_PENDING)
1046 return;
1047 // All write errors should be mapped into ERR_IO_PENDING by
1048 // HandleWriteError.
1049 DCHECK_LT(0, result.error_code);
1050 connection()->OnCanWrite();
978 } 1051 }
979 1052
980 void QuicChromiumClientSession::OnWriteError(int error_code) { 1053 void QuicChromiumClientSession::OnWriteError(int error_code) {
981 DCHECK_NE(ERR_IO_PENDING, error_code); 1054 DCHECK_NE(ERR_IO_PENDING, error_code);
982 DCHECK_GT(0, error_code); 1055 DCHECK_GT(0, error_code);
983 connection()->OnWriteError(error_code); 1056 connection()->OnWriteError(error_code);
984 } 1057 }
985 1058
986 void QuicChromiumClientSession::OnWriteUnblocked() { 1059 void QuicChromiumClientSession::OnWriteUnblocked() {
987 connection()->OnCanWrite(); 1060 connection()->OnCanWrite();
988 } 1061 }
989 1062
990 void QuicChromiumClientSession::OnPathDegrading() { 1063 void QuicChromiumClientSession::OnPathDegrading() {
991 if (stream_factory_) { 1064 if (stream_factory_) {
992 stream_factory_->MaybeMigrateSingleSession(this, EARLY_MIGRATION, nullptr); 1065 stream_factory_->MaybeMigrateSingleSession(this, EARLY_MIGRATION);
993 } 1066 }
994 } 1067 }
995 1068
996 bool QuicChromiumClientSession::HasOpenDynamicStreams() const { 1069 bool QuicChromiumClientSession::HasOpenDynamicStreams() const {
997 return QuicSession::HasOpenDynamicStreams() || 1070 return QuicSession::HasOpenDynamicStreams() ||
998 GetNumDrainingOutgoingStreams() > 0; 1071 GetNumDrainingOutgoingStreams() > 0;
999 } 1072 }
1000 1073
1001 void QuicChromiumClientSession::OnProofValid( 1074 void QuicChromiumClientSession::OnProofValid(
1002 const QuicCryptoClientConfig::CachedState& cached) { 1075 const QuicCryptoClientConfig::CachedState& cached) {
(...skipping 186 matching lines...) Expand 10 before | Expand all | Expand 10 after
1189 going_away_ = true; 1262 going_away_ = true;
1190 DCHECK_EQ(0u, GetNumActiveStreams()); 1263 DCHECK_EQ(0u, GetNumActiveStreams());
1191 // Will delete |this|. 1264 // Will delete |this|.
1192 if (stream_factory_) 1265 if (stream_factory_)
1193 stream_factory_->OnSessionClosed(this); 1266 stream_factory_->OnSessionClosed(this);
1194 } 1267 }
1195 1268
1196 bool QuicChromiumClientSession::MigrateToSocket( 1269 bool QuicChromiumClientSession::MigrateToSocket(
1197 std::unique_ptr<DatagramClientSocket> socket, 1270 std::unique_ptr<DatagramClientSocket> socket,
1198 std::unique_ptr<QuicChromiumPacketReader> reader, 1271 std::unique_ptr<QuicChromiumPacketReader> reader,
1199 std::unique_ptr<QuicChromiumPacketWriter> writer, 1272 std::unique_ptr<QuicChromiumPacketWriter> writer) {
1200 scoped_refptr<StringIOBuffer> packet) {
1201 DCHECK_EQ(sockets_.size(), packet_readers_.size()); 1273 DCHECK_EQ(sockets_.size(), packet_readers_.size());
1202 if (sockets_.size() >= kMaxReadersPerQuicSession) { 1274 if (sockets_.size() >= kMaxReadersPerQuicSession)
1203 return false; 1275 return false;
1204 } 1276
1205 // TODO(jri): Make SetQuicPacketWriter take a scoped_ptr. 1277 // TODO(jri): Make SetQuicPacketWriter take a scoped_ptr.
1206 packet_readers_.push_back(std::move(reader)); 1278 packet_readers_.push_back(std::move(reader));
1207 sockets_.push_back(std::move(socket)); 1279 sockets_.push_back(std::move(socket));
1208 StartReading(); 1280 StartReading();
1209 QuicChromiumPacketWriter* raw_writer = writer.get(); 1281 // Block the writer to prevent is being used until WriteToNewSocket
1282 // completes.
1283 writer->set_write_blocked(true);
1210 connection()->SetQuicPacketWriter(writer.release(), /*owns_writer=*/true); 1284 connection()->SetQuicPacketWriter(writer.release(), /*owns_writer=*/true);
1211 if (packet == nullptr) { 1285
1212 connection()->SendPing(); 1286 // Post task to write the pending packet or a PING packet to the new
1213 return true; 1287 // socket. This avoids reentrancy issues if there is a write error
1214 } 1288 // on the write to the new socket.
1215 // Packet rewrite after migration on socket write error. 1289 task_runner_->PostTask(
1216 error_code_from_rewrite_ = raw_writer->WritePacketToSocket(packet.get()); 1290 FROM_HERE, base::Bind(&QuicChromiumClientSession::WriteToNewSocket,
1217 use_error_code_from_rewrite_ = true; 1291 weak_factory_.GetWeakPtr()));
1292 // Migration completed.
1293 migration_pending_ = false;
1218 return true; 1294 return true;
1219 } 1295 }
1220 1296
1221 void QuicChromiumClientSession::PopulateNetErrorDetails( 1297 void QuicChromiumClientSession::PopulateNetErrorDetails(
1222 NetErrorDetails* details) { 1298 NetErrorDetails* details) {
1223 details->quic_port_migration_detected = port_migration_detected_; 1299 details->quic_port_migration_detected = port_migration_detected_;
1224 } 1300 }
1225 1301
1226 const DatagramClientSocket* QuicChromiumClientSession::GetDefaultSocket() 1302 const DatagramClientSocket* QuicChromiumClientSession::GetDefaultSocket()
1227 const { 1303 const {
(...skipping 27 matching lines...) Expand all
1255 } 1331 }
1256 1332
1257 void QuicChromiumClientSession::DeletePromised( 1333 void QuicChromiumClientSession::DeletePromised(
1258 QuicClientPromisedInfo* promised) { 1334 QuicClientPromisedInfo* promised) {
1259 if (IsOpenStream(promised->id())) 1335 if (IsOpenStream(promised->id()))
1260 streams_pushed_and_claimed_count_++; 1336 streams_pushed_and_claimed_count_++;
1261 QuicClientSessionBase::DeletePromised(promised); 1337 QuicClientSessionBase::DeletePromised(promised);
1262 } 1338 }
1263 1339
1264 } // namespace net 1340 } // namespace net
OLDNEW
« no previous file with comments | « net/quic/chromium/quic_chromium_client_session.h ('k') | net/quic/chromium/quic_chromium_client_session_test.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698