Skip to content

Commit

Permalink
quic: fixup closing/draining period timing
Browse files Browse the repository at this point in the history 
When entering the closing or draining periods, servers should wait
three times the current probe timeout before releasing session
state.

PR-URL: #34283
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
@jasnell @cjihrig
jasnell authored and cjihrig committed Jul 22, 2020
1 parent 823fb00 commit 8e5220a
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 85 deletions.
8 changes: 6 additions & 2 deletions src/quic/node_quic_session-inl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -344,9 +344,13 @@ void QuicSession::InitApplication() {
// the peer. All existing streams are abandoned and closed.
void QuicSession::OnIdleTimeout() {
if (!is_destroyed()) {
if (state_->idle_timeout == 1) {
Debug(this, "Idle timeout");
Close(QuicSessionListener::SESSION_CLOSE_FLAG_SILENT);
return;
}
state_->idle_timeout = 1;
Debug(this, "Idle timeout");
Close(QuicSessionListener::SESSION_CLOSE_FLAG_SILENT);
UpdateClosingTimer();
}
}

Expand Down
156 changes: 74 additions & 82 deletions src/quic/node_quic_session.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1633,7 +1633,8 @@ BaseObjectPtr<QuicStream> QuicSession::CreateStream(int64_t stream_id) {

// Initiate a shutdown of the QuicSession.
void QuicSession::Close(int close_flags) {
CHECK(!is_destroyed());
if (is_destroyed())
return;
bool silent = close_flags & QuicSessionListener::SESSION_CLOSE_FLAG_SILENT;
bool stateless_reset = is_stateless_reset();

Expand Down Expand Up @@ -1735,13 +1736,16 @@ bool QuicSession::GetNewConnectionID(
}

void QuicSession::HandleError() {
if (is_destroyed() || (is_in_closing_period() && !is_server()))
if (is_destroyed())
return;

if (!SendConnectionClose()) {
set_last_error(QUIC_ERROR_SESSION, NGTCP2_ERR_INTERNAL);
Close();
}
// If the QuicSession is a server, send a CONNECTION_CLOSE. In either
// case, the closing timer will be set and the QuicSession will be
// destroyed.
if (is_server())
SendConnectionClose();
else
UpdateClosingTimer();
}

// The the retransmit libuv timer fires, it will call MaybeTimeout,
Expand All @@ -1751,20 +1755,25 @@ void QuicSession::MaybeTimeout() {
if (is_destroyed())
return;
uint64_t now = uv_hrtime();
bool transmit = false;

if (ngtcp2_conn_loss_detection_expiry(connection()) <= now) {
Debug(this, "Retransmitting due to loss detection");
CHECK_EQ(ngtcp2_conn_on_loss_detection_timer(connection(), now), 0);
IncrementStat(&QuicSessionStats::loss_retransmit_count);
transmit = true;
} else if (ngtcp2_conn_ack_delay_expiry(connection()) <= now) {
}

if (ngtcp2_conn_ack_delay_expiry(connection()) <= now) {
Debug(this, "Retransmitting due to ack delay");
ngtcp2_conn_cancel_expired_ack_delay_timer(connection(), now);
IncrementStat(&QuicSessionStats::ack_delay_retransmit_count);
transmit = true;
}
if (transmit)
SendPendingData();

int rv = ngtcp2_conn_handle_expiry(connection(), now);
if (rv != 0) {
Debug(this, "Error handling retransmit timeout: %s", ngtcp2_strerror(rv));
set_last_error(QUIC_ERROR_SESSION, rv);
HandleError();
}

SendPendingData();
}

bool QuicSession::OpenBidirectionalStream(int64_t* stream_id) {
Expand Down Expand Up @@ -1847,16 +1856,7 @@ bool QuicSession::Receive(
Debug(this, "Receiving QUIC packet");
IncrementStat(&QuicSessionStats::bytes_received, nread);

// Closing period starts once ngtcp2 has detected that the session
// is being shutdown locally. Note that this is different that the
// is_graceful_closing() function, which
// indicates a graceful shutdown that allows the session and streams
// to finish naturally. When is_in_closing_period is true, ngtcp2 is
// actively in the process of shutting down the connection and a
// CONNECTION_CLOSE has already been sent. The only thing we can do
// at this point is either ignore the packet or send another
// CONNECTION_CLOSE.
if (is_in_closing_period()) {
if (is_in_closing_period() && is_server()) {
Debug(this, "QUIC packet received while in closing period");
IncrementConnectionCloseAttempts();
// For server QuicSession instances, we serialize the connection close
Expand All @@ -1866,30 +1866,13 @@ bool QuicSession::Receive(
// every received packet, however, so we use an exponential
// backoff, increasing the ratio of packets received to connection
// close frame sent with every one we send.
if (!ShouldAttemptConnectionClose()) {
Debug(this, "Not sending connection close");
if (UNLIKELY(ShouldAttemptConnectionClose() &&
!SendConnectionClose())) {
Debug(this, "Failure trying to send another connection close");
return false;
}
Debug(this, "Sending connection close");
return SendConnectionClose();
}

// When is_in_draining_period is true, ngtcp2 has received a
// connection close and we are simply discarding received packets.
// No outbound packets may be sent. Return true here because
// the packet was correctly processed, even tho it is being
// ignored.
if (is_in_draining_period()) {
Debug(this, "QUIC packet received while in draining period");
return true;
}

// It's possible for the remote address to change from one
// packet to the next so we have to look at the addr on
// every packet.
remote_address_ = remote_addr;
QuicPath path(local_addr, remote_address_);

{
// These are within a scope to ensure that the InternalCallbackScope
// and HandleScope are both exited before continuing on with the
Expand All @@ -1901,38 +1884,28 @@ bool QuicSession::Receive(
Debug(this, "Processing received packet");
HandleScope handle_scope(env()->isolate());
InternalCallbackScope callback_scope(this);
remote_address_ = remote_addr;
QuicPath path(local_addr, remote_address_);
if (!ReceivePacket(&path, data, nread)) {
Debug(this, "Failure processing received packet (code %" PRIu64 ")",
last_error().code);
HandleError();
return false;
}
}

if (is_destroyed()) {
Debug(this, "Session was destroyed while processing the received packet");
// If the QuicSession has been destroyed but it is not
// in the closing period, a CONNECTION_CLOSE has not yet
// been sent to the peer. Let's attempt to send one.
if (!is_in_closing_period() && !is_in_draining_period()) {
set_last_error();
SendConnectionClose();
}
return true;
}

// Only send pending data if we haven't entered draining mode.
// We enter the draining period when a CONNECTION_CLOSE has been
// received from the remote peer.
if (is_in_draining_period()) {
Debug(this, "In draining period after processing packet");
// If processing the packet puts us into draining period, there's
// absolutely nothing left for us to do except silently close
// and destroy this QuicSession.
// and destroy this QuicSession, which we do by updating the
// closing timer.
GetConnectionCloseInfo();
Close(QuicSessionListener::SESSION_CLOSE_FLAG_SILENT);
UpdateClosingTimer();
return true;
}

Debug(this, "Sending pending data after processing packet");
SendPendingData();
UpdateIdleTimer();
Expand Down Expand Up @@ -1965,20 +1938,32 @@ bool QuicSession::ReceivePacket(
case NGTCP2_ERR_DRAINING:
case NGTCP2_ERR_RECV_VERSION_NEGOTIATION:
break;
case NGTCP2_ERR_RETRY:
// This should only ever happen on the server
CHECK(is_server());
socket()->SendRetry(scid_, dcid_, local_address_, remote_address_);
Close(QuicSessionListener::SESSION_CLOSE_FLAG_SILENT);
break;
case NGTCP2_ERR_DROP_CONN:
Close(QuicSessionListener::SESSION_CLOSE_FLAG_SILENT);
break;
default:
// Per ngtcp2: If NGTCP2_ERR_RETRY is returned,
// QuicSession must be a server and must perform
// address validation by sending a Retry packet
// then immediately close the connection.
if (err == NGTCP2_ERR_RETRY && is_server()) {
socket()->SendRetry(scid_, dcid_, local_address_, remote_address_);
Close(QuicSessionListener::SESSION_CLOSE_FLAG_SILENT);
break;
}
set_last_error(QUIC_ERROR_SESSION, err);
return false;
}
}

if (is_destroyed()) {
Debug(this, "Session was destroyed while processing the received packet");
// If the QuicSession has been destroyed but it is not
// in the closing period, a CONNECTION_CLOSE has not yet
// been sent to the peer. Let's attempt to send one. This
// will have the effect of setting the idle timer to the
// closing/draining period, after which the QuicSession
// will be destroyed.
return is_in_closing_period() ? true : SendConnectionClose();
}

return true;
}

Expand Down Expand Up @@ -2121,6 +2106,9 @@ bool QuicSession::SendConnectionClose() {
Debug(this, "Connection Close code: %" PRIu64 " (family: %s)",
error.code, error.family_name());

Debug(this, "Setting the connection/draining period timer");
UpdateClosingTimer();

// If initial keys have not yet been installed, use the alternative
// ImmediateConnectionClose to send a stateless connection close to
// the peer.
Expand All @@ -2135,11 +2123,12 @@ bool QuicSession::SendConnectionClose() {
return true;
}

UpdateIdleTimer();
switch (crypto_context_->side()) {
case NGTCP2_CRYPTO_SIDE_SERVER: {
if (!is_in_closing_period() && !StartClosingPeriod())
if (!is_in_closing_period() && !StartClosingPeriod()) {
Close(QuicSessionListener::SESSION_CLOSE_FLAG_SILENT);
return false;
}
CHECK_GT(conn_closebuf_->length(), 0);
return SendPacket(QuicPacket::Copy(conn_closebuf_));
}
Expand All @@ -2157,6 +2146,7 @@ bool QuicSession::SendConnectionClose() {
if (UNLIKELY(nwrite < 0)) {
Debug(this, "Error writing connection close: %d", nwrite);
set_last_error(QUIC_ERROR_SESSION, static_cast<int>(nwrite));
Close(QuicSessionListener::SESSION_CLOSE_FLAG_SILENT);
return false;
}
packet->set_length(nwrite);
Expand Down Expand Up @@ -2330,16 +2320,12 @@ bool QuicSession::StartClosingPeriod() {
if (is_in_closing_period())
return true;

StopRetransmitTimer();
UpdateIdleTimer();

QuicError error = last_error();
Debug(this, "Closing period has started. Error %d", error.code);

// Once the CONNECTION_CLOSE packet is written,
// is_in_closing_period will return true.
conn_closebuf_ = QuicPacket::Create(
"server connection close");
conn_closebuf_ = QuicPacket::Create("server connection close");
ssize_t nwrite =
SelectCloseFn(error.family)(
connection(),
Expand All @@ -2349,12 +2335,7 @@ bool QuicSession::StartClosingPeriod() {
error.code,
uv_hrtime());
if (nwrite < 0) {
if (nwrite == NGTCP2_ERR_PKT_NUM_EXHAUSTED) {
set_last_error(QUIC_ERROR_SESSION, NGTCP2_ERR_PKT_NUM_EXHAUSTED);
Close(QuicSessionListener::SESSION_CLOSE_FLAG_SILENT);
} else {
set_last_error(QUIC_ERROR_SESSION, static_cast<int>(nwrite));
}
set_last_error(QUIC_ERROR_SESSION, static_cast<int>(nwrite));
return false;
}
conn_closebuf_->set_length(nwrite);
Expand Down Expand Up @@ -2450,6 +2431,8 @@ void QuicSession::UpdateConnectionID(
// will be silently closed. It is important to update this as activity
// occurs to keep the idle timer from firing.
void QuicSession::UpdateIdleTimer() {
if (is_closing_timer_enabled())
return;
uint64_t now = uv_hrtime();
uint64_t expiry = ngtcp2_conn_get_idle_expiry(connection());
// nano to millis
Expand All @@ -2459,6 +2442,15 @@ void QuicSession::UpdateIdleTimer() {
idle_.Update(timeout, timeout);
}

void QuicSession::UpdateClosingTimer() {
set_closing_timer_enabled(true);
uint64_t timeout =
is_server() ? (ngtcp2_conn_get_pto(connection()) / 1000000ULL) * 3 : 0;
Debug(this, "Setting closing timeout to %" PRIu64, timeout);
retransmit_.Stop();
idle_.Update(timeout, 0);
idle_.Ref();
}

// Write any packets current pending for the ngtcp2 connection based on
// the current state of the QuicSession. If the QuicSession is in the
Expand Down
5 changes: 4 additions & 1 deletion src/quic/node_quic_session.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -696,7 +696,8 @@ class QuicApplication : public MemoryRetainer,
V(NGTCP2_CALLBACK, in_ngtcp2_callback) \
V(CONNECTION_CLOSE_SCOPE, in_connection_close_scope) \
V(SILENT_CLOSE, silent_closing) \
V(STATELESS_RESET, stateless_reset)
V(STATELESS_RESET, stateless_reset) \
V(CLOSING_TIMER_ENABLED, closing_timer_enabled)

// QUIC sessions are logical connections that exchange data
// back and forth between peer endpoints via UDP. Every QuicSession
Expand Down Expand Up @@ -1403,6 +1404,8 @@ class QuicSession final : public AsyncWrap,

void UpdateIdleTimer();

void UpdateClosingTimer();

inline void UpdateRetransmitTimer(uint64_t timeout);

inline void StopRetransmitTimer();
Expand Down

0 comments on commit 8e5220a

Please sign in to comment.