Skip to content

Commit a116f4b

Browse files
authored
fix(native): Ensure txn hasn't been detached before attempting to use… (#26633)
… for http client ``` == NO RELEASE NOTE == ```
1 parent 6e0a8cf commit a116f4b

File tree

1 file changed

+16
-13
lines changed

1 file changed

+16
-13
lines changed

presto-native-execution/presto_cpp/main/http/HttpClient.cpp

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -212,12 +212,12 @@ class ResponseHandler : public proxygen::HTTPTransactionHandler {
212212
}
213213

214214
void setTransaction(proxygen::HTTPTransaction* txn) noexcept override {
215-
if (txn) {
216-
protocol_ = txn->getTransport().getCodec().getProtocol();
217-
}
215+
txn_ = CHECK_NOTNULL(txn);
216+
protocol_ = txn_->getTransport().getCodec().getProtocol();
218217
}
219218

220219
void detachTransaction() noexcept override {
220+
txn_ = nullptr;
221221
self_.reset();
222222
}
223223

@@ -269,12 +269,14 @@ class ResponseHandler : public proxygen::HTTPTransactionHandler {
269269
self_.reset();
270270
}
271271

272-
void sendRequest(proxygen::HTTPTransaction* txn) {
273-
txn->sendHeaders(request_);
274-
if (!body_.empty()) {
275-
txn->sendBody(folly::IOBuf::wrapBuffer(body_.c_str(), body_.size()));
272+
void sendRequest() {
273+
if (txn_) {
274+
txn_->sendHeaders(request_);
275+
if (!body_.empty()) {
276+
txn_->sendBody(folly::IOBuf::wrapBuffer(body_.c_str(), body_.size()));
277+
}
278+
txn_->sendEOM();
276279
}
277-
txn->sendEOM();
278280
}
279281

280282
private:
@@ -288,6 +290,7 @@ class ResponseHandler : public proxygen::HTTPTransactionHandler {
288290
std::shared_ptr<ResponseHandler> self_;
289291
std::shared_ptr<HttpClient> client_;
290292
std::optional<proxygen::CodecProtocol> protocol_;
293+
proxygen::HTTPTransaction* txn_{nullptr};
291294
};
292295

293296
// Responsible for making an HTTP request. The request will be made in 2
@@ -354,10 +357,8 @@ class ConnectionHandler : public proxygen::HTTPConnector::Callback {
354357
http2InitialStreamWindow_, http2StreamWindow_, http2SessionWindow_);
355358
session->setMaxConcurrentOutgoingStreams(maxConcurrentStreams_);
356359
}
357-
auto txn = session->newTransaction(responseHandler_.get());
358-
if (txn) {
359-
responseHandler_->sendRequest(txn);
360-
}
360+
session->newTransaction(responseHandler_.get());
361+
responseHandler_->sendRequest();
361362

362363
sessionPool_->putSession(session);
363364
delete this;
@@ -535,7 +536,9 @@ void HttpClient::sendRequest(std::shared_ptr<ResponseHandler> responseHandler) {
535536
auto doSend = [this, responseHandler = std::move(responseHandler)](
536537
proxygen::HTTPTransaction* txn) {
537538
if (txn) {
538-
responseHandler->sendRequest(txn);
539+
// txn may no longer be valid, ::onError and ::detachTransaction would
540+
// have been called. In such case, ::sendRequest will be no-op
541+
responseHandler->sendRequest();
539542
return;
540543
}
541544
VLOG(2) << "Create new connection to " << address_.describe();

0 commit comments

Comments
 (0)