From 99be9236258cadd6e7404288083cbedd87b5190c Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Thu, 31 Oct 2024 10:19:40 +0100 Subject: [PATCH] JsonRpcConnection: Don't drop client from cache prematurely PR #7445 incorrectly assumed that a peer that had already disconnected and never reconnected was due to the endpoint client being dropped after a successful socket shutdown. Howeever, the issue at that time was that there was not a single timeout guards that could cancel the `async_shutdown` call, petentially blocking indefinetely. Although remove the client from cache early might have allowed the endpoint to reconnect, it did not resolve the underlying problem. Now that we have a proper cancellation timeout, we can wait until the currently used socket is fully closed before dropping the client from our cache. When our socket termination works reliably, the `ApiListener` reconnect timer should attempt to reconnect this endpoint after the next tick. --- lib/remote/jsonrpcconnection.cpp | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/lib/remote/jsonrpcconnection.cpp b/lib/remote/jsonrpcconnection.cpp index 7f05f44985..715a60ce76 100644 --- a/lib/remote/jsonrpcconnection.cpp +++ b/lib/remote/jsonrpcconnection.cpp @@ -209,17 +209,7 @@ void JsonRpcConnection::Disconnect() IoEngine::SpawnCoroutine(m_IoStrand, [this, keepAlive](asio::yield_context yc) { Log(LogWarning, "JsonRpcConnection") - << "API client disconnected for identity '" << m_Identity << "'"; - - // We need to unregister the endpoint client as soon as possible not to confuse Icinga 2, - // given that Endpoint::GetConnected() is just performing a check that the endpoint's client - // cache is not empty, which could result in an already disconnected endpoint never trying to - // reconnect again. See #7444. - if (m_Endpoint) { - m_Endpoint->RemoveClient(this); - } else { - ApiListener::GetInstance()->RemoveAnonymousClient(this); - } + << "Disconnecting API client for identity '" << m_Identity << "'"; m_OutgoingMessagesQueued.Set(); @@ -255,6 +245,15 @@ void JsonRpcConnection::Disconnect() shutdownTimeout->Cancel(); m_Stream->lowest_layer().shutdown(m_Stream->lowest_layer().shutdown_both, ec); + + if (m_Endpoint) { + m_Endpoint->RemoveClient(this); + } else { + ApiListener::GetInstance()->RemoveAnonymousClient(this); + } + + Log(LogWarning, "JsonRpcConnection") + << "API client disconnected for identity '" << m_Identity << "'"; }); } }