Ensure QuicChromiumPacketWriter isn't still tracked when deleted. (#4691

)

Attempt to address increased crashes in the QuicChromiumPacketWriter
destructor.

The QuicChromiumPacketWriter destructor crashes seem to indicate that
there is a double free.  The destructor is called from both the
QuicConnection destructor and during QuicConnection::MigratePath.

A double free appears to only be possible after MigratePath is called
with the same writer as currently used, but on a connection that is
not connected.

The increase possibly means a race between a connection teardown and
migration was made more likely to occur as a result of recent
performance improvements.

This adds a check and pointer reset that should avoid having a stale
QuicChromiumPacketWriter pointer in the QuicConnection.

This also moves the WeakPtr instantiation that detects object
destruction
closer to the evaluation.

b/388316262
b/388616504

net/quic/quic_chromium_packet_reader.cc

@@ -127,14 +127,14 @@ bool QuicChromiumPacketReader::ProcessMultiplePacketReadResult(int result) {
   quic::QuicSocketAddress quick_peer_address =
       ToQuicSocketAddress(peer_address);
 
-  auto self = weak_factory_.GetWeakPtr();
   struct Socket::ReadPacketResult* read_packet = read_results_.packets;
   for (int p = 0; p < read_results_.result; ++p, ++read_packet) {
     if (read_packet->result <= 0) {
       continue;
     }
     quic::QuicReceivedPacket packet(read_packet->buffer, read_packet->result,
                                     clock_->ApproximateNow());
+    auto self = weak_factory_.GetWeakPtr();
     if (!(visitor_->OnPacket(packet, quick_local_address, quick_peer_address) &&
           self)) {
       return false;

net/third_party/quiche/src/quiche/quic/core/quic_connection.cc

@@ -411,7 +411,7 @@ void QuicConnection::InstallInitialCrypters(QuicConnectionId connection_id) {
 
 QuicConnection::~QuicConnection() {
   QUICHE_DCHECK_GE(stats_.max_egress_mtu, long_term_mtu_);
-  if (owns_writer_) {
+  if (writer_ != nullptr && owns_writer_) {
     delete writer_;
   }
   ClearQueuedPackets();
@@ -2439,7 +2439,7 @@ QuicPacketNumber QuicConnection::GetLeastUnacked() const {
 }
 
 bool QuicConnection::HandleWriteBlocked() {
-  if (!writer_->IsWriteBlocked()) {
+  if (writer_ != nullptr && !writer_->IsWriteBlocked()) {
     return false;
   }
 
@@ -2808,12 +2808,14 @@ void QuicConnection::ProcessUdpPacket(const QuicSocketAddress& self_address,
 }
 
 void QuicConnection::OnBlockedWriterCanWrite() {
-  writer_->SetWritable();
-  OnCanWrite();
+  if (writer_) {
+    writer_->SetWritable();
+    OnCanWrite();
+  }
 }
 
 void QuicConnection::OnCanWrite() {
-  if (!connected_) {
+  if (!connected_ || !writer_) {
     return;
   }
   if (writer_->IsWriteBlocked()) {
@@ -6911,6 +6913,10 @@ bool QuicConnection::MigratePath(const QuicSocketAddress& self_address,
   QUICHE_DCHECK(perspective_ == Perspective::IS_CLIENT);
   if (!connected_) {
     if (owns_writer) {
+      if (writer_ == writer) {
+        writer_ = nullptr;
+        owns_writer_ = false;
+      }
       delete writer;
     }
     return false;
@@ -6921,6 +6927,10 @@ bool QuicConnection::MigratePath(const QuicSocketAddress& self_address,
   if (connection_migration_use_new_cid_) {
     if (!UpdateConnectionIdsOnMigration(self_address, peer_address)) {
       if (owns_writer) {
+        if (writer_ == writer) {
+          writer_ = nullptr;
+          owns_writer_ = false;
+        }
         delete writer;
       }
       return false;