Skip to content

Commit

Permalink
JsonRpcConnection: Don't drop client from cache prematurely
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
yhabteab committed Oct 31, 2024
1 parent 92399a9 commit 99be923
Showing 1 changed file with 10 additions and 11 deletions.
21 changes: 10 additions & 11 deletions lib/remote/jsonrpcconnection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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 << "'";
});
}
}
Expand Down

0 comments on commit 99be923

Please sign in to comment.