From 9b00f62a7bcb77a90143606f0d7ce80448daf8b2 Mon Sep 17 00:00:00 2001
From: Alan Frindell <afrind@meta.com>
Date: Thu, 5 Dec 2024 09:11:40 -0800
Subject: [PATCH] Refresh session timeout on stream activity

Summary:
As long as data is being sent and received within the session, we don't want it to time out.

Note:

1) New streams already refresh the timeout in HTTPTransaction so a call in that case is not required.
2) I decided not to refresh on send or receive STOP_SENDING, but in practice either new stream or RESET_STREAM should keep the timeout fresh.

Reviewed By: jordicenzano

Differential Revision: D66508543

fbshipit-source-id: 916831d521b97288079505320a1556654a5e0481
---
 proxygen/lib/http/session/HTTPTransaction.h         | 2 +-
 proxygen/lib/http/webtransport/WebTransportImpl.cpp | 8 ++++++++
 proxygen/lib/http/webtransport/WebTransportImpl.h   | 3 +++
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/proxygen/lib/http/session/HTTPTransaction.h b/proxygen/lib/http/session/HTTPTransaction.h
index 816f673b3f..a897d8d4a9 100644
--- a/proxygen/lib/http/session/HTTPTransaction.h
+++ b/proxygen/lib/http/session/HTTPTransaction.h
@@ -1582,7 +1582,7 @@ class HTTPTransaction
   /**
    * Schedule or refresh the idle timeout for this transaction
    */
-  void refreshTimeout() {
+  void refreshTimeout() override {
     // TODO(T121147568): Remove the zero-check after the experiment is complete.
     if (timer_ && hasIdleTimeout() &&
         idleTimeout_.value() != std::chrono::milliseconds::zero()) {
diff --git a/proxygen/lib/http/webtransport/WebTransportImpl.cpp b/proxygen/lib/http/webtransport/WebTransportImpl.cpp
index 39ff3d16fc..c10058ac6d 100644
--- a/proxygen/lib/http/webtransport/WebTransportImpl.cpp
+++ b/proxygen/lib/http/webtransport/WebTransportImpl.cpp
@@ -60,6 +60,7 @@ WebTransportImpl::newWebTransportUniStream() {
   auto res = wtEgressStreams_.emplace(std::piecewise_construct,
                                       std::forward_as_tuple(*id),
                                       std::forward_as_tuple(*this, *id));
+  sp_.refreshTimeout();
   return &res.first->second;
 }
 
@@ -76,6 +77,7 @@ WebTransportImpl::newWebTransportBidiStream() {
                                 std::forward_as_tuple(*this, *id));
   auto readHandle = &ingressRes.first->second;
   tp_.initiateReadOnBidiStream(*id, readHandle);
+  sp_.refreshTimeout();
   auto egressRes = wtEgressStreams_.emplace(std::piecewise_construct,
                                             std::forward_as_tuple(*id),
                                             std::forward_as_tuple(*this, *id));
@@ -114,6 +116,7 @@ WebTransportImpl::sendWebTransportStreamData(HTTPCodec::StreamID id,
   if (eof || res.hasError()) {
     wtEgressStreams_.erase(id);
   }
+  sp_.refreshTimeout();
   return res;
 }
 
@@ -122,6 +125,7 @@ WebTransportImpl::resetWebTransportEgress(HTTPCodec::StreamID id,
                                           uint32_t errorCode) {
   auto res = tp_.resetWebTransportEgress(id, errorCode);
   wtEgressStreams_.erase(id);
+  sp_.refreshTimeout();
   return res;
 }
 
@@ -130,6 +134,7 @@ WebTransportImpl::stopReadingWebTransportIngress(HTTPCodec::StreamID id,
                                                  uint32_t errorCode) {
   auto res = tp_.stopReadingWebTransportIngress(id, errorCode);
   wtIngressStreams_.erase(id);
+  sp_.refreshTimeout();
   return res;
 }
 
@@ -142,6 +147,7 @@ WebTransportImpl::StreamWriteHandle::writeStreamData(
         folly::make_exception_wrapper<WebTransport::Exception>(
             *stopSendingErrorCode_));
   }
+  impl_.sp_.refreshTimeout();
   auto fcState =
       impl_.sendWebTransportStreamData(id_, std::move(data), fin, this);
   if (fcState.hasError()) {
@@ -213,6 +219,7 @@ WebTransportImpl::StreamReadHandle::readStreamData() {
 
 void WebTransportImpl::StreamReadHandle::readAvailable(
     quic::StreamId id) noexcept {
+  impl_.sp_.refreshTimeout();
   auto readRes = impl_.tp_.readWebTransportData(id, 65535);
   if (readRes.hasError()) {
     LOG(ERROR) << "Got synchronous read error=" << uint32_t(readRes.error());
@@ -258,6 +265,7 @@ WebTransportImpl::StreamReadHandle::dataAvailable(
 void WebTransportImpl::StreamReadHandle::readError(
     quic::StreamId id, quic::QuicError error) noexcept {
   // Do I need to setReadCallback(id, nullptr);
+  impl_.sp_.refreshTimeout();
   auto quicAppErrorCode = error.code.asApplicationErrorCode();
   if (quicAppErrorCode) {
     auto appErrorCode =
diff --git a/proxygen/lib/http/webtransport/WebTransportImpl.h b/proxygen/lib/http/webtransport/WebTransportImpl.h
index b3861ea1f2..c8483a9d17 100644
--- a/proxygen/lib/http/webtransport/WebTransportImpl.h
+++ b/proxygen/lib/http/webtransport/WebTransportImpl.h
@@ -70,6 +70,9 @@ class WebTransportImpl : public WebTransport {
    public:
     virtual ~SessionProvider() = default;
 
+    virtual void refreshTimeout() {
+    }
+
     virtual folly::Expected<folly::Unit, WebTransport::ErrorCode> closeSession(
         folly::Optional<uint32_t> /*error*/) = 0;
   };