mirror of
https://github.com/savoirfairelinux/opendht.git
synced 2026-11-08 04:21:51 +00:00
http: run resolver callbacks and connect on the io executor
Resolver::add_callback invoked the callback synchronously on the calling thread when resolution was already completed. Since Request objects are commonly created and sent from application threads while the io_context runs on a dedicated io thread, the whole continuation (Request::send -> Request::connect -> Connection::async_connect -> asio::async_connect start_connect_op) then executed on the caller's thread, mutating asio reactor state (kqueue/epoll per-descriptor op queues) concurrently with the io thread. This data race corrupts the heap and was observed crashing with SIGSEGV/EXC_BAD_ACCESS inside asio start_connect_op, both from the io thread and from the application main thread (e.g. a name lookup reusing a shared, already-completed Resolver), on macOS. Fix by always posting completed-resolution callbacks to the resolver's executor, so the connect chain consistently runs on the io_context thread regardless of the caller. Also make the Connection::async_connect completion handler hold a weak_ptr to the Connection instead of capturing raw `this` and a reference to the underlying socket, matching the existing async_handshake pattern, so a Connection destroyed while a connect operation is in flight (e.g. after a request timeout) can no longer be written to after free. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
1 parent
74b99453ce
commit
603a11494e
1 file changed
+32
-14
+32
-14
@@ -602,23 +602,31 @@ Connection::async_connect(std::vector<asio::ip::tcp::endpoint>&& endpoints, Conn
|
||||
cb(asio::error::operation_aborted, {});
|
||||
return;
|
||||
}
|
||||
auto& base = ssl_socket_ ? ssl_socket_->lowest_layer() : *socket_;
|
||||
|
||||
#pragma GCC diagnostic push
|
||||
#pragma GCC diagnostic ignored "-Wunused-variable"
|
||||
ConnectHandlerCb wcb = [this, &base, cb = std::move(cb)](const asio::error_code& ec,
|
||||
const asio::ip::tcp::endpoint& endpoint) {
|
||||
std::weak_ptr<Connection> wthis = shared_from_this();
|
||||
ConnectHandlerCb wcb = [wthis, cb = std::move(cb)](const asio::error_code& ec,
|
||||
const asio::ip::tcp::endpoint& endpoint) {
|
||||
if (!ec) {
|
||||
local_address_ = base.local_endpoint().address();
|
||||
// Once connected, set a keep alive on the TCP socket with 30 seconds delay
|
||||
// This will generate broken pipes as soon as possible.
|
||||
// Note this needs to be done once connected to have a valid native_handle()
|
||||
this->set_keepalive(30);
|
||||
if (auto sthis = wthis.lock()) {
|
||||
auto& this_ = *sthis;
|
||||
{
|
||||
std::lock_guard lock(this_.mutex_);
|
||||
if (this_.ssl_socket_ || this_.socket_) {
|
||||
auto& base = this_.ssl_socket_ ? this_.ssl_socket_->lowest_layer() : *this_.socket_;
|
||||
asio::error_code lec;
|
||||
auto le = base.local_endpoint(lec);
|
||||
if (!lec)
|
||||
this_.local_address_ = le.address();
|
||||
}
|
||||
}
|
||||
// Once connected, set a keep alive on the TCP socket with 30 seconds delay
|
||||
// This will generate broken pipes as soon as possible.
|
||||
// Note this needs to be done once connected to have a valid native_handle()
|
||||
this_.set_keepalive(30);
|
||||
}
|
||||
}
|
||||
if (cb)
|
||||
cb(ec, endpoint);
|
||||
};
|
||||
#pragma GCC diagnostic pop
|
||||
|
||||
if (ssl_socket_)
|
||||
asio::async_connect(ssl_socket_->lowest_layer(), std::move(endpoints), wrapCallback(std::move(wcb)));
|
||||
@@ -917,8 +925,18 @@ Resolver::add_callback(ResolverCb cb, sa_family_t family)
|
||||
else
|
||||
cb(ec, filter(endpoints, family));
|
||||
});
|
||||
else
|
||||
cb(ec_, family == AF_UNSPEC ? endpoints_ : filter(endpoints_, family));
|
||||
else {
|
||||
// Always dispatch the callback on the io_context thread. Invoking it
|
||||
// synchronously would run the caller's continuation (e.g. the whole
|
||||
// connect chain of Request::send) on the calling thread, mutating asio
|
||||
// objects concurrently with the io thread running the io_context.
|
||||
asio::post(resolver_.get_executor(),
|
||||
[cb = std::move(cb),
|
||||
ec = ec_,
|
||||
endpoints = family == AF_UNSPEC ? endpoints_ : filter(endpoints_, family)] {
|
||||
cb(ec, endpoints);
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
void
|
||||
|
||||
Reference in new issue
Block a user