Skip to content

Commit ca270cd

Browse files
authored
fix: do not block identify on SSE client shutdown completion (#384)
It appears that the `async_shutdown` method in Foxy client hangs indefinitely. It's not clear whether this is because the FD servers are misbehaving, or the client is somehow misbehaving. In any case, this is causing Identify to hang because it waits for `async_shutdown` completion handler to be invoked before creating a new EventSource client. One solution would be to not wait for that, and just make the new client immediately. That works, but it still leaves the old client sitting in memory waiting forever. Instead, I've changed the shutdown logic to simply close the TCP socket.
1 parent 03449b3 commit ca270cd

File tree

1 file changed

+31
-16
lines changed

1 file changed

+31
-16
lines changed

libs/server-sent-events/src/client.cpp

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -280,14 +280,10 @@ class FoxyClient : public Client,
280280
void on_read_body(boost::system::error_code ec, std::size_t amount) {
281281
boost::ignore_unused(amount);
282282
if (ec) {
283+
if (shutting_down_) {
284+
return;
285+
}
283286
if (ec == boost::asio::error::operation_aborted) {
284-
// operation_aborted can occur if the read timeout is reached or
285-
// if we're shutting down, so shutting_down_ is needed to
286-
// disambiguate.
287-
if (shutting_down_) {
288-
// End the chain of async operations.
289-
return;
290-
}
291287
errors_(Error::ReadTimeout);
292288
return async_backoff(
293289
"aborting read of response body (timeout)");
@@ -339,18 +335,37 @@ class FoxyClient : public Client,
339335
}
340336

341337
void do_shutdown(std::function<void()> completion) {
338+
// Signal to the body reader operation that if it completes,
339+
// it should return instead of starting another async read.
342340
shutting_down_ = true;
341+
// If any backoff is taking place, cancel that as well.
343342
backoff_timer_.cancel();
344-
session_->async_shutdown(beast::bind_front_handler(
345-
&FoxyClient::on_shutdown, std::move(completion)));
346-
}
347343

348-
static void on_shutdown(std::function<void()> completion,
349-
boost::system::error_code ec) {
350-
// Because do_shutdown doesn't use shared_from_this() when initiating
351-
// the async_shutdown op, the client may already be destroyed - hence
352-
// this static method.
353-
boost::ignore_unused(ec);
344+
// Cancels the outstanding read.
345+
if (session_->stream.is_ssl()) {
346+
session_->stream.ssl().next_layer().cancel();
347+
} else {
348+
session_->stream.plain().cancel();
349+
}
350+
351+
// Ideally we would call session_->async_shutdown() here to gracefully
352+
// terminate the SSL session. For unknown reasons, this call appears to
353+
// hang indefinitely and never complete until the SDK client is
354+
// destroyed.
355+
//
356+
// A workaround is to set a timeout on the operation, say 1 second. This
357+
// gives the opportunity to shutdown gracefully and then if that fails,
358+
// we could close the socket directly. But that also doesn't seem to
359+
// work: even with the timeout, the operation still doesn't complete.
360+
//
361+
// So the most robust solution appears to be closing the socket
362+
// directly. This is not ideal because it doesn't send a close_notify to
363+
// the server.
364+
boost::system::error_code ec;
365+
session_->stream.plain().shutdown(
366+
boost::asio::ip::tcp::socket::shutdown_both, ec);
367+
session_->stream.plain().close(ec);
368+
354369
if (completion) {
355370
completion();
356371
}

0 commit comments

Comments
 (0)