From 2eb0c806aeea9fb7f6f8d19035ad9bb1e4e0cfd1 Mon Sep 17 00:00:00 2001 From: absurdfarce Date: Thu, 12 Mar 2026 14:29:25 -0500 Subject: [PATCH 1/2] Do not make on_close() a pure virtual void fn. Also avoid the callback in shutdown situations in order to avoid taking longer than we need to. --- src/connection.hpp | 2 +- src/prepare_host_handler.cpp | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/connection.hpp b/src/connection.hpp index 512967011..e82af8307 100644 --- a/src/connection.hpp +++ b/src/connection.hpp @@ -87,7 +87,7 @@ class ConnectionListener { * * @param connection The closing connection. */ - virtual void on_close(Connection* connection) = 0; + virtual void on_close(Connection* connection) {}; }; /** diff --git a/src/prepare_host_handler.cpp b/src/prepare_host_handler.cpp index 8a03661c7..8616fe08c 100644 --- a/src/prepare_host_handler.cpp +++ b/src/prepare_host_handler.cpp @@ -69,8 +69,6 @@ void PrepareHostHandler::prepare(uv_loop_t* loop, const ConnectionSettings& sett } void PrepareHostHandler::on_close(Connection* connection) { - callback_(this); - dec_ref(); // The event loop is done with this handler } From 674c0459fb131d113c98046b1c16cfa5e7d7d9f6 Mon Sep 17 00:00:00 2001 From: absurdfarce Date: Mon, 30 Mar 2026 15:35:02 -0500 Subject: [PATCH 2/2] Code review feedback --- src/cluster.cpp | 13 +++++++++++++ src/connection.hpp | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/cluster.cpp b/src/cluster.cpp index 2f6720641..49b5b6b08 100644 --- a/src/cluster.cpp +++ b/src/cluster.cpp @@ -663,6 +663,19 @@ void Cluster::notify_or_record(const ClusterEvent& event) { } } +// TODO: This function takes a callback because it's called from both the host up and host add handlers. +// The usage there is to send appropriate notifications to all other nodes whether query prep succeeds or +// not. But that processing isn't really part of what _this_ function is trying to accomplish; that +// responsibility really relies with the host up or host add handler itself. +// +// A more robust implementation would be to implement the host up and host add handlers as a combined +// sequence of ops and then wait on that whole sequence to either complete or fail. That's a fairly +// significant change to the current impl, though, so for now just moving the notifications into +// the host up/add handlers will be sufficient. +// +// Why does any of this matter? Originally we were triggering this callback on socket close even when +// we were closing a connection. This link has already been severed (see CASSCPP-3) but that effort +// exposed the unnecessary entanglement described above. bool Cluster::prepare_host(const Host::Ptr& host, const PrepareHostHandler::Callback& callback) { if (connection_ && settings_.prepare_on_up_or_add_host) { PrepareHostHandler::Ptr prepare_host_handler( diff --git a/src/connection.hpp b/src/connection.hpp index e82af8307..8ed41fbc7 100644 --- a/src/connection.hpp +++ b/src/connection.hpp @@ -87,7 +87,7 @@ class ConnectionListener { * * @param connection The closing connection. */ - virtual void on_close(Connection* connection) {}; + virtual void on_close(Connection* connection) {} }; /**