From 80793b9e67e2d5f94a08cde7a84c80dd017270bb Mon Sep 17 00:00:00 2001 From: Hani Damlaj Date: Sun, 18 Oct 2026 10:36:38 -0700 Subject: [PATCH] remove `ingressPushId_` from `HQSession` Summary: removes `ingressPushId_`, its reads and writes, and the unused `onPushPromiseHeadersComplete` callback, continued efforts to remove all things http push related in proxygen/lib Reviewed By: Carl-Elkhoury, jbeshay Differential Revision: D124391059 fbshipit-source-id: 7c46f57264ee930f4ce6b9354930dc156ebae3e5 --- proxygen/lib/http/session/HQSession.cpp | 39 ++----------------------- proxygen/lib/http/session/HQSession.h | 19 ------------ 2 files changed, 3 insertions(+), 55 deletions(-) diff --git a/proxygen/lib/http/session/HQSession.cpp b/proxygen/lib/http/session/HQSession.cpp index 4c980d737..c61aa5673 100644 --- a/proxygen/lib/http/session/HQSession.cpp +++ b/proxygen/lib/http/session/HQSession.cpp @@ -2701,18 +2701,9 @@ void HQSession::HQStreamTransportBase::onHeadersComplete( // Tell the HTTPTransaction to start processing the message now // that the full ingress headers have arrived. - // Depending on the push promise latch, the message is delivered to - // the current transaction (no push promise) or to a freshly created - // pushed transaction. The latter is done via "onPushPromiseHeadersComplete" - // callback const bool isResp = msg->isResponse(); - if (ingressPushId_) { - onPushPromiseHeadersComplete(*ingressPushId_, streamID, std::move(msg)); - ingressPushId_ = folly::none; - } else { - txn_.onIngressHeadersComplete(std::move(msg)); - maybeScheduleWTStreamDrain(); - } + txn_.onIngressHeadersComplete(std::move(msg)); + maybeScheduleWTStreamDrain(); if (auto httpSessionActivityTracker = session_.getHTTPSessionActivityTracker()) { httpSessionActivityTracker->reportActivity(); @@ -3284,19 +3275,7 @@ size_t HQSession::HQStreamTransportBase::sendPadding( void HQSession::HQStreamTransportBase::onMessageBegin( HTTPCodec::StreamID streamID, HTTPMessage* /* msg */) { - VLOG(4) << __func__ << " txn=" << txn_ << " streamID=" << streamID - << " ingressPushId=" << ingressPushId_.value_or(-1); - - if (ingressPushId_) { - constexpr auto error = - "Received onMessageBegin in the middle of push promise"; - LOG(ERROR) << error << " streamID=" << streamID << " session=" << session_; - // TODO: Audit this error code - session_.dropConnectionAsync( - quic::QuicError(HTTP3::ErrorCode::HTTP_FRAME_ERROR, error), - kErrorDropped); - return; - } + VLOG(4) << __func__ << " txn=" << txn_ << " streamID=" << streamID; if (session_.infoCallback_) { session_.infoCallback_->onRequestBegin(session_); @@ -3305,10 +3284,6 @@ void HQSession::HQStreamTransportBase::onMessageBegin( // NOTE: for H2 this is where we create a new stream and transaction. // for HQ there is nothing to do here, except caching the codec streamID codecStreamId_ = streamID; - - // Reset the pending pushID, since the subsequent invocation of - // `onHeadersComplete` won't be associated with a push - ingressPushId_ = folly::none; } void HQSession::HQStreamTransportBase::trackEgressBodyOffset( @@ -3629,14 +3604,6 @@ void HQSession::expirePendingWTStreams(quic::StreamId sessionID) noexcept { } } -// Methods specific to StreamTransport subclasses -// -// - -void HQSession::HQStreamTransport::onPushPromiseHeadersComplete( - hq::PushId, HTTPCodec::StreamID, std::unique_ptr) { -} - void HQSession::onDatagramsAvailable() noexcept { auto result = sock_->readDatagramBufs(); if (result.hasError()) { diff --git a/proxygen/lib/http/session/HQSession.h b/proxygen/lib/http/session/HQSession.h index 6ba189c23..1eead7549 100644 --- a/proxygen/lib/http/session/HQSession.h +++ b/proxygen/lib/http/session/HQSession.h @@ -1124,13 +1124,6 @@ class HQSession HTTPCodec::StreamID /* parentTxnId */, HTTPMessage* /* msg */) override; - virtual void onPushPromiseHeadersComplete( - hq::PushId /* pushID */, - HTTPCodec::StreamID /* assoc streamID */, - std::unique_ptr /* msg */) { - LOG(ERROR) << "push promise: txn=" << txn_ << " TODO"; - } - void onHeadersComplete(HTTPCodec::StreamID streamID, std::unique_ptr msg) override; @@ -1739,13 +1732,6 @@ class HQSession // Track number of armed QUIC delivery callbacks. uint64_t numActiveDeliveryCallbacks_{0}; - // Used to store last seen ingress push ID between - // the invocations of onPushPromiseBegin / onHeadersComplete. - // It is being reset by - // - "onNewMessage" (in which case the push promise is being abandoned), - // - "onPushMessageBegin" (which may be abandonned / duplicate message id) - // - "onHeadersComplete" (not pending anymore) - folly::Optional ingressPushId_; /** * We asynchronously deliver datagrams to the HttpTxnHandler after upstream * txn receives 2xx or downstream txn sends a 2xx. @@ -1806,11 +1792,6 @@ class HQSession initIngress(__func__); } - void onPushPromiseHeadersComplete( - hq::PushId /* pushID */, - HTTPCodec::StreamID /* assoc streamID */, - std::unique_ptr /* promise */) override; - uint16_t getDatagramSizeLimit() const noexcept override; folly::Expected sendDatagram( std::unique_ptr datagram) override;