Skip to content

Commit a751da1

Browse files
sandarshmeta-codesync[bot]
authored andcommitted
Guard unset ingress source during CONNECT teardown
Summary: HTTPConnectStream constructs the transport before awaiting sendRequest(), but ingressSource_ is assigned only after that await completes. If CONNECT setup is cancelled or throws first, destruction calls close() -> shutdownRead() -> canRead() and dereferences the unset shared_ptr. Guard ingressSource_ in canRead() so partially constructed streams report that they are not readable. Apply the change to both the fbcode and xplat mirrors. Keep the existing ConnectCancelledBeforeIngressAssigned regression test. It reproduces the production destructor stack without the guard and passes once the guard is present. This fixes the recurring crash tracked by T277272947, T283037317, and T284870008. Reviewed By: jbeshay Differential Revision: D114422937 fbshipit-source-id: e064f038a3034a51f8c220b61a207d9c90fdb9f3
1 parent 9b8c221 commit a751da1

2 files changed

Lines changed: 20 additions & 1 deletion

File tree

proxygen/lib/http/coro/transport/HTTPConnectStream.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ void HTTPConnectStream::close() {
167167
}
168168

169169
bool HTTPConnectStream::canRead() const {
170-
return *ingressSource_;
170+
return ingressSource_ && *ingressSource_;
171171
}
172172

173173
bool HTTPConnectStream::canWrite() const {

proxygen/lib/http/coro/transport/test/HTTPConnectTransportTest.cpp

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,25 @@ TEST_F(HTTPConnectTransportTest, ConnectCancelled) {
289289
});
290290
}
291291

292+
TEST_F(HTTPConnectTransportTest, ConnectCancelledBeforeIngressAssigned) {
293+
run([&]() -> Task<> {
294+
auto session = co_await HTTPCoroConnector::connect(
295+
&evb, *srv->address(), 0ms, getConnParams());
296+
auto reservation = session->reserveRequest();
297+
// Cancel after the session exists so connectUnique constructs the stream.
298+
cancelSource.requestCancellation();
299+
auto result = co_await folly::coro::co_awaitTry(co_withCancellation(
300+
cancelSource.getToken(),
301+
HTTPConnectStream::connectUnique(session,
302+
std::move(*reservation),
303+
kAuthority,
304+
std::chrono::seconds(1),
305+
{{"Foo", "Bar"}},
306+
egressBufferSize)));
307+
EXPECT_TRUE(result.hasException());
308+
});
309+
}
310+
292311
TEST_F(HTTPConnectTransportTest, SimpleReadWrite) {
293312
run([&]() -> Task<> {
294313
// Should fill window

0 commit comments

Comments
 (0)