HttpServerConnection: Don't spawn useless coroutines

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.
This commit is contained in:
Yonas Habteab 2024-11-04 11:26:10 +01:00
parent d68ee3fcf8
commit 5c0f9bfdaa
2 changed files with 36 additions and 30 deletions

View File

@ -68,44 +68,49 @@ void HttpServerConnection::Start()
IoEngine::SpawnCoroutine(m_IoStrand, [this, keepAlive](asio::yield_context yc) { CheckLiveness(yc); }); IoEngine::SpawnCoroutine(m_IoStrand, [this, keepAlive](asio::yield_context yc) { CheckLiveness(yc); });
} }
void HttpServerConnection::Disconnect() /**
* Tries to asynchronously shut down the SSL stream and underlying socket.
*
* It is important to note that this method should only be called from within a coroutine that uses `m_IoStrand`.
*
* @param yc boost::asio::yield_context The coroutine yield context which you are calling this method from.
*/
void HttpServerConnection::Disconnect(boost::asio::yield_context yc)
{ {
namespace asio = boost::asio; namespace asio = boost::asio;
HttpServerConnection::Ptr keepAlive (this); if (m_ShuttingDown) {
return;
}
IoEngine::SpawnCoroutine(m_IoStrand, [this, keepAlive](asio::yield_context yc) { m_ShuttingDown = true;
if (!m_ShuttingDown) {
m_ShuttingDown = true;
Log(LogInformation, "HttpServerConnection") Log(LogInformation, "HttpServerConnection")
<< "HTTP client disconnected (from " << m_PeerAddress << ")"; << "HTTP client disconnected (from " << m_PeerAddress << ")";
/* /*
* Do not swallow exceptions in a coroutine. * Do not swallow exceptions in a coroutine.
* https://github.com/Icinga/icinga2/issues/7351 * https://github.com/Icinga/icinga2/issues/7351
* We must not catch `detail::forced_unwind exception` as * We must not catch `detail::forced_unwind exception` as
* this is used for unwinding the stack. * this is used for unwinding the stack.
* *
* Just use the error_code dummy here. * Just use the error_code dummy here.
*/ */
boost::system::error_code ec; 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) { if (listener) {
listener->RemoveHttpClient(this); listener->RemoveHttpClient(this);
} }
}
});
} }
void HttpServerConnection::StartStreaming() void HttpServerConnection::StartStreaming()
@ -126,7 +131,7 @@ void HttpServerConnection::StartStreaming()
m_Stream->async_read_some(readBuf, yc[ec]); m_Stream->async_read_some(readBuf, yc[ec]);
} while (!ec); } while (!ec);
Disconnect(); Disconnect(yc);
} }
}); });
} }
@ -563,7 +568,7 @@ void HttpServerConnection::ProcessMessages(boost::asio::yield_context yc)
} }
} }
Disconnect(); Disconnect(yc);
} }
void HttpServerConnection::CheckLiveness(boost::asio::yield_context yc) void HttpServerConnection::CheckLiveness(boost::asio::yield_context yc)
@ -582,7 +587,7 @@ void HttpServerConnection::CheckLiveness(boost::asio::yield_context yc)
Log(LogInformation, "HttpServerConnection") Log(LogInformation, "HttpServerConnection")
<< "No messages for HTTP connection have been received in the last 10 seconds."; << "No messages for HTTP connection have been received in the last 10 seconds.";
Disconnect(); Disconnect(yc);
break; break;
} }
} }

View File

@ -28,7 +28,6 @@ public:
HttpServerConnection(const String& identity, bool authenticated, const Shared<AsioTlsStream>::Ptr& stream); HttpServerConnection(const String& identity, bool authenticated, const Shared<AsioTlsStream>::Ptr& stream);
void Start(); void Start();
void Disconnect();
void StartStreaming(); void StartStreaming();
bool Disconnected(); bool Disconnected();
@ -45,6 +44,8 @@ private:
HttpServerConnection(const String& identity, bool authenticated, const Shared<AsioTlsStream>::Ptr& stream, boost::asio::io_context& io); HttpServerConnection(const String& identity, bool authenticated, const Shared<AsioTlsStream>::Ptr& stream, boost::asio::io_context& io);
void Disconnect(boost::asio::yield_context yc);
void ProcessMessages(boost::asio::yield_context yc); void ProcessMessages(boost::asio::yield_context yc);
void CheckLiveness(boost::asio::yield_context yc); void CheckLiveness(boost::asio::yield_context yc);
}; };