Skip to content

Commit ecc4b43

Browse files
committed
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. However, 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 removing 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. Additionally, we now have logs both for before and after socket termination, which may help identify if it is hanging somewhere in between.
1 parent 92399a9 commit ecc4b43

File tree

1 file changed

+12
-13
lines changed

1 file changed

+12
-13
lines changed

lib/remote/jsonrpcconnection.cpp

+12-13
Original file line numberDiff line numberDiff line change
@@ -207,20 +207,10 @@ void JsonRpcConnection::Disconnect()
207207
if (!m_ShuttingDown.exchange(true)) {
208208
JsonRpcConnection::Ptr keepAlive (this);
209209

210-
IoEngine::SpawnCoroutine(m_IoStrand, [this, keepAlive](asio::yield_context yc) {
211-
Log(LogWarning, "JsonRpcConnection")
212-
<< "API client disconnected for identity '" << m_Identity << "'";
213-
214-
// We need to unregister the endpoint client as soon as possible not to confuse Icinga 2,
215-
// given that Endpoint::GetConnected() is just performing a check that the endpoint's client
216-
// cache is not empty, which could result in an already disconnected endpoint never trying to
217-
// reconnect again. See #7444.
218-
if (m_Endpoint) {
219-
m_Endpoint->RemoveClient(this);
220-
} else {
221-
ApiListener::GetInstance()->RemoveAnonymousClient(this);
222-
}
210+
Log(LogNotice, "JsonRpcConnection")
211+
<< "Disconnecting API client for identity '" << m_Identity << "'";
223212

213+
IoEngine::SpawnCoroutine(m_IoStrand, [this, keepAlive](asio::yield_context yc) {
224214
m_OutgoingMessagesQueued.Set();
225215

226216
m_WriterDone.Wait(yc);
@@ -255,6 +245,15 @@ void JsonRpcConnection::Disconnect()
255245
shutdownTimeout->Cancel();
256246

257247
m_Stream->lowest_layer().shutdown(m_Stream->lowest_layer().shutdown_both, ec);
248+
249+
if (m_Endpoint) {
250+
m_Endpoint->RemoveClient(this);
251+
} else {
252+
ApiListener::GetInstance()->RemoveAnonymousClient(this);
253+
}
254+
255+
Log(LogWarning, "JsonRpcConnection")
256+
<< "API client disconnected for identity '" << m_Identity << "'";
258257
});
259258
}
260259
}

0 commit comments

Comments
 (0)