Skip to content

Commit

Permalink
HttpServerConnection: Don't spawn useless coroutines
Browse files Browse the repository at this point in the history
Currently, for each `Disconnect()` call, we spawn a coroutine, but every
one of them is just usesless, except the first one. However, since all
`Disconnect()` usages share the same asio strand and cannot interfere
with each other, spawning another coroutine within `Disconnect()` isn't
even necessary. When a coroutine calls `Disconnect()` now, it will
immediately initiate an async shutdown of the socket, potentially causing
the coroutine to yield and allowing the others to resume. Therefore, the
`m_ShuttingDown` flag is still required by the coroutines to be checked
regularly.
  • Loading branch information
yhabteab committed Nov 6, 2024
1 parent d894792 commit 64271cc
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 31 deletions.
58 changes: 28 additions & 30 deletions lib/remote/httpserverconnection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,44 +68,42 @@ void HttpServerConnection::Start()
IoEngine::SpawnCoroutine(m_IoStrand, [this, keepAlive](asio::yield_context yc) { CheckLiveness(yc); });
}

void HttpServerConnection::Disconnect()
void HttpServerConnection::Disconnect(boost::asio::yield_context yc)
{
namespace asio = boost::asio;

HttpServerConnection::Ptr keepAlive (this);
if (m_ShuttingDown) {
return;
}

IoEngine::SpawnCoroutine(m_IoStrand, [this, keepAlive](asio::yield_context yc) {
if (!m_ShuttingDown) {
m_ShuttingDown = true;
m_ShuttingDown = true;

Log(LogInformation, "HttpServerConnection")
<< "HTTP client disconnected (from " << m_PeerAddress << ")";

/*
* Do not swallow exceptions in a coroutine.
* https://github.com/Icinga/icinga2/issues/7351
* We must not catch `detail::forced_unwind exception` as
* this is used for unwinding the stack.
*
* Just use the error_code dummy here.
*/
boost::system::error_code ec;
Log(LogInformation, "HttpServerConnection")
<< "HTTP client disconnected (from " << m_PeerAddress << ")";

/*
* Do not swallow exceptions in a coroutine.
* https://github.com/Icinga/icinga2/issues/7351
* We must not catch `detail::forced_unwind exception` as
* this is used for unwinding the stack.
*
* Just use the error_code dummy here.
*/
boost::system::error_code ec;

m_CheckLivenessTimer.cancel();
m_CheckLivenessTimer.cancel();

m_Stream->lowest_layer().cancel(ec);
m_Stream->lowest_layer().cancel(ec);

m_Stream->next_layer().async_shutdown(yc[ec]);
m_Stream->next_layer().async_shutdown(yc[ec]);

m_Stream->lowest_layer().shutdown(m_Stream->lowest_layer().shutdown_both, ec);
m_Stream->lowest_layer().shutdown(m_Stream->lowest_layer().shutdown_both, ec);

auto listener (ApiListener::GetInstance());
auto listener (ApiListener::GetInstance());

if (listener) {
listener->RemoveHttpClient(this);
}
}
});
if (listener) {
listener->RemoveHttpClient(this);
}
}

void HttpServerConnection::StartStreaming()
Expand All @@ -126,7 +124,7 @@ void HttpServerConnection::StartStreaming()
m_Stream->async_read_some(readBuf, yc[ec]);
} while (!ec);

Disconnect();
Disconnect(yc);
}
});
}
Expand Down Expand Up @@ -563,7 +561,7 @@ void HttpServerConnection::ProcessMessages(boost::asio::yield_context yc)
}
}

Disconnect();
Disconnect(yc);
}

void HttpServerConnection::CheckLiveness(boost::asio::yield_context yc)
Expand All @@ -582,7 +580,7 @@ void HttpServerConnection::CheckLiveness(boost::asio::yield_context yc)
Log(LogInformation, "HttpServerConnection")
<< "No messages for HTTP connection have been received in the last 10 seconds.";

Disconnect();
Disconnect(yc);
break;
}
}
Expand Down
2 changes: 1 addition & 1 deletion lib/remote/httpserverconnection.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class HttpServerConnection final : public Object
HttpServerConnection(const String& identity, bool authenticated, const Shared<AsioTlsStream>::Ptr& stream);

void Start();
void Disconnect();
void Disconnect(boost::asio::yield_context yc);
void StartStreaming();

bool Disconnected();
Expand Down

0 comments on commit 64271cc

Please sign in to comment.