Remove connection close-related QUIC_BUGs. Remove QUIC_BUG(quic_bug_10360_3) from QuicSpdySession::SendHttp3GoAway(), and QUIC_BUG(quic_send_multiple_connection_closes) from QuicConnection::SendConnectionClosePacket(). Among other changes, this CL reverts parts of cl/437010467. PiperOrigin-RevId: 442601892
diff --git a/quiche/quic/core/http/quic_spdy_session.cc b/quiche/quic/core/http/quic_spdy_session.cc index b90fba9..003929f 100644 --- a/quiche/quic/core/http/quic_spdy_session.cc +++ b/quiche/quic/core/http/quic_spdy_session.cc
@@ -776,21 +776,12 @@ stream_id = QuicUtils::GetMaxClientInitiatedBidirectionalStreamId( transport_version()); - if (last_sent_http3_goaway_id_.has_value()) { - if (last_sent_http3_goaway_id_.value() == stream_id) { - // Do not send GOAWAY twice. - return; - } - if (last_sent_http3_goaway_id_.value() < stream_id) { - // A previous GOAWAY frame was sent with smaller stream ID. This is not - // possible, because the only time a GOAWAY frame with non-maximal - // stream ID is sent is right before closing connection. - QUIC_BUG(quic_bug_10360_3) - << "Not sending GOAWAY frame with " << stream_id - << " because one with " << last_sent_http3_goaway_id_.value() - << " already sent on connection " << connection()->connection_id(); - return; - } + if (last_sent_http3_goaway_id_.has_value() && + last_sent_http3_goaway_id_.value() <= stream_id) { + // Do not send GOAWAY frame with a higher id, because it is forbidden. + // Do not send one with same stream id as before, since frames on the + // control stream are guaranteed to be processed in order. + return; } send_control_stream_->SendGoAway(stream_id); @@ -1533,18 +1524,9 @@ } if (last_sent_http3_goaway_id_.has_value() && last_sent_http3_goaway_id_.value() <= stream_id) { - // A previous GOAWAY frame was sent with smaller stream ID. This is not - // possible, because this is the only method sending a GOAWAY frame with - // non-maximal stream ID, and this must only be called once, right - // before closing connection. - QUIC_BUG(QuicGoawayFrameAlreadySent) - << "Not sending GOAWAY frame with " << stream_id << " because one with " - << last_sent_http3_goaway_id_.value() << " already sent on connection " - << connection()->connection_id(); - - // MUST not send GOAWAY with identifier larger than previously sent. - // Do not bother sending one with same identifier as before, since GOAWAY - // frames on the control stream are guaranteed to be processed in order. + // Do not send GOAWAY frame with a higher id, because it is forbidden. + // Do not send one with same stream id as before, since frames on the + // control stream are guaranteed to be processed in order. return; }
diff --git a/quiche/quic/core/quic_connection.cc b/quiche/quic/core/quic_connection.cc index de739d6..2a3ceb5 100644 --- a/quiche/quic/core/quic_connection.cc +++ b/quiche/quic/core/quic_connection.cc
@@ -4543,16 +4543,6 @@ auto* frame = new QuicConnectionCloseFrame( transport_version(), error, ietf_error, details, framer_.current_received_frame_type()); - if (level == ENCRYPTION_FORWARD_SECURE) { - if (connection_close_frame_sent_.has_value()) { - QUIC_BUG(quic_send_multiple_connection_closes) - << ENDPOINT << "Already sent connection close: " - << connection_close_frame_sent_.value() - << ", going to send connection close: " << *frame; - } else { - connection_close_frame_sent_ = *frame; - } - } packet_creator_.ConsumeRetransmittableControlFrame(QuicFrame(frame)); packet_creator_.FlushCurrentPacket(); }
diff --git a/quiche/quic/core/quic_connection.h b/quiche/quic/core/quic_connection.h index 22c2c50..a186386 100644 --- a/quiche/quic/core/quic_connection.h +++ b/quiche/quic/core/quic_connection.h
@@ -1295,6 +1295,11 @@ // |ietf_error| may optionally be be used to directly specify the wire // error code. Otherwise if |ietf_error| is NO_IETF_QUIC_ERROR, the // QuicErrorCodeToTransportErrorCode mapping of |error| will be used. + // Caller may choose to call SendConnectionClosePacket() directly instead of + // CloseConnection() to notify peer that the connection is going to be closed, + // for example, when the server is tearing down. Given + // SendConnectionClosePacket() does not close connection, multiple connection + // close packets could be sent to the peer. virtual void SendConnectionClosePacket(QuicErrorCode error, QuicIetfTransportErrorCodes ietf_error, const std::string& details); @@ -2243,10 +2248,6 @@ const bool flush_after_coalesce_higher_space_packets_ = GetQuicReloadableFlag(quic_flush_after_coalesce_higher_space_packets); - // Records the 1-RTT connection close frame sent. - // TODO(b/180103273): remove this after the bug gets fixed. - absl::optional<QuicConnectionCloseFrame> connection_close_frame_sent_; - // If true, send connection close packet on INVALID_VERSION. bool send_connection_close_for_invalid_version_ = false;
diff --git a/quiche/quic/core/quic_connection_test.cc b/quiche/quic/core/quic_connection_test.cc index 50ae2f1..2770f5a 100644 --- a/quiche/quic/core/quic_connection_test.cc +++ b/quiche/quic/core/quic_connection_test.cc
@@ -15747,16 +15747,16 @@ SendStreamDataToPeer(1, "foo", 0, NO_FIN, nullptr); ASSERT_TRUE(connection_.BlackholeDetectionInProgress()); - // Verify BeforeConnectionCloseSent gets called twice while OnConnectionClosed - // is called once. + // Verify that BeforeConnectionCloseSent() gets called twice, + // while OnConnectionClosed() is called only once. EXPECT_CALL(visitor_, BeforeConnectionCloseSent()).Times(2); EXPECT_CALL(visitor_, OnConnectionClosed(_, _)); // Send connection close w/o closing connection. QuicConnectionPeer::SendConnectionClosePacket( &connection_, INTERNAL_ERROR, QUIC_INTERNAL_ERROR, "internal error"); - // Fire blackhole detection alarm. - EXPECT_QUIC_BUG(connection_.GetBlackholeDetectorAlarm()->Fire(), - "Already sent connection close"); + // Fire blackhole detection alarm. This will invoke + // SendConnectionClosePacket() a second time. + connection_.GetBlackholeDetectorAlarm()->Fire(); } // Regression test for b/157895910.