From: Arran Cudbard-Bell Date: Sun, 26 Jul 2026 22:13:49 +0000 (-0600) Subject: connection: expose is_closed so a closed connection's handle can't be used X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=bb579aa5ab7e6a8a7483e2e471e095d774942f1e;p=thirdparty%2Ffreeradius-server.git connection: expose is_closed so a closed connection's handle can't be used The close callback frees the handle, but is_closed lived in the private struct so drivers had no way to tell a live connection from a dead one. A mysql connection dying mid-query aborted on talloc_get_type_abort() in sql_error(), reached from sql_get_map_list_resume() after the trunk had already closed the connection. rlm_sql_print_error() is the only caller of driver->sql_error, and every driver reads the errors off the handle, so the check goes there once and reports "Connection closed" rather than in each driver. --- diff --git a/src/lib/server/connection.c b/src/lib/server/connection.c index a26000308af..c44d83d4584 100644 --- a/src/lib/server/connection.c +++ b/src/lib/server/connection.c @@ -90,7 +90,6 @@ struct connection_s { void *uctx; //!< User data. void *in_handler; //!< Connection is currently in a callback. - bool is_closed; //!< The close callback has previously been called. bool processing_signals; //!< Processing deferred signals, don't let the deferred ///< signal processor be called multiple times. @@ -683,11 +682,11 @@ static void connection_state_enter_closed(connection_t *conn) * the FAILED state. Eventually the connection is * shutdown, and enter_shutdown calls this function. */ - if (conn->close && !conn->is_closed) { + if (conn->close && !conn->pub.is_closed) { HANDLER_BEGIN(conn, conn->close); DEBUG4("Calling close(el=%p, h=%p, uctx=%p)", conn->pub.el, conn->pub.h, conn->uctx); conn->close(conn->pub.el, conn->pub.h, conn->uctx); - conn->is_closed = true; /* Ensure close doesn't get called twice if the connection is freed */ + conn->pub.is_closed = true; /* Ensure close doesn't get called twice if the connection is freed */ HANDLER_END(conn); /* @@ -697,7 +696,7 @@ static void connection_state_enter_closed(connection_t *conn) */ if (conn->pub.state != CONNECTION_STATE_CLOSED) return; } else { - conn->is_closed = true; + conn->pub.is_closed = true; } WATCH_POST(conn); } @@ -953,7 +952,7 @@ static void connection_state_enter_timeout(connection_t *conn) */ static void connection_state_enter_halted(connection_t *conn) { - fr_assert(conn->is_closed); + fr_assert(conn->pub.is_closed); switch (conn->pub.state) { case CONNECTION_STATE_INIT: @@ -1135,13 +1134,13 @@ static void connection_state_enter_init(connection_t *conn) switch (ret) { case CONNECTION_STATE_CONNECTING: - conn->is_closed = false; /* We now have a handle */ + conn->pub.is_closed = false; /* We now have a handle */ WATCH_POST(conn); /* Only call if we successfully initialised the handle */ connection_state_enter_connecting(conn); return; case CONNECTION_STATE_CONNECTED: - conn->is_closed = false; /* We now have a handle */ + conn->pub.is_closed = false; /* We now have a handle */ WATCH_POST(conn); /* Only call if we successfully initialised the handle */ connection_state_enter_connected(conn); return; @@ -1326,7 +1325,7 @@ void connection_signal_shutdown(connection_t *conn) case CONNECTION_STATE_TIMEOUT: case CONNECTION_STATE_FAILED: connection_state_enter_closed(conn); - fr_assert(conn->is_closed); + fr_assert(conn->pub.is_closed); FALL_THROUGH; case CONNECTION_STATE_CLOSED: @@ -1380,8 +1379,8 @@ void connection_signal_halt(connection_t *conn) case CONNECTION_STATE_SHUTDOWN: case CONNECTION_STATE_TIMEOUT: case CONNECTION_STATE_FAILED: - if (!conn->is_closed) connection_state_enter_closed(conn); - fr_assert(conn->is_closed); + if (!conn->pub.is_closed) connection_state_enter_closed(conn); + fr_assert(conn->pub.is_closed); connection_state_enter_halted(conn); break; @@ -1589,7 +1588,8 @@ connection_t *connection_alloc(TALLOC_CTX *ctx, fr_event_list_t *el, .pub = { .id = id, .state = CONNECTION_STATE_HALTED, - .el = el + .el = el, + .is_closed = true /* Starts closed */ }, .reconnection_delay = conf->reconnection_delay, .connection_timeout = conf->connection_timeout, @@ -1598,7 +1598,6 @@ connection_t *connection_alloc(TALLOC_CTX *ctx, fr_event_list_t *el, .close = funcs->close, .failed = funcs->failed, .shutdown = funcs->shutdown, - .is_closed = true, /* Starts closed */ .triggers = conf->triggers, .trigger_args = conf->trigger_args, .trigger_cs = conf->trigger_cs, diff --git a/src/lib/server/connection.h b/src/lib/server/connection.h index 8bf24101eea..f28c66dd1eb 100644 --- a/src/lib/server/connection.h +++ b/src/lib/server/connection.h @@ -75,6 +75,10 @@ struct connection_pub_s { void * _CONST h; //!< Connection handle fr_event_list_t * _CONST el; //!< Event list for timers and I/O events. + bool _CONST is_closed; //!< The close callback has previously been called, + ///< so the handle is gone. Check this before using + ///< anything that lives on the connection. + uint64_t _CONST reconnected; //!< How many times we've attempted to establish or ///< re-establish this connection. uint64_t _CONST timed_out; //!< How many times has this connection timed out when diff --git a/src/modules/rlm_sql/sql.c b/src/modules/rlm_sql/sql.c index 06199e730f9..bc30fc0e35c 100644 --- a/src/modules/rlm_sql/sql.c +++ b/src/modules/rlm_sql/sql.c @@ -124,7 +124,20 @@ void rlm_sql_print_error(rlm_sql_t const *inst, request_t *request, fr_sql_query char const *driver = inst->driver_submodule->name; sql_log_entry_t log[20]; size_t num, i; - TALLOC_CTX *log_ctx = talloc_new(NULL); + TALLOC_CTX *log_ctx; + + /* + * Every driver reads the errors off the connection handle, which the + * trunk frees when it closes the connection. A query that failed + * because its connection died is resumed after that has happened, + * so there is nothing left to ask the driver for. + */ + if (!query_ctx->tconn || query_ctx->tconn->conn->is_closed) { + ROPTIONAL(RERROR, ERROR, "%s: Connection closed", driver); + return; + } + + log_ctx = talloc_new(NULL); num = (inst->driver->sql_error)(log_ctx, log, (NUM_ELEMENTS(log)), query_ctx); if (num == 0) {