Centralize client session ownership release

docs/CHANGELOG.md

@@ -48,6 +48,9 @@
 - Interactive client writes now pass through a bounded per-client outbox and
   flush against the remote SSH window from that client's session loop.  Exec
   sessions still write synchronously to preserve script output ordering.
+- Session callback refs are now owned and released through `client.c`, so
+  bootstrap and interactive cleanup no longer need to manually mirror the
+  main-ref / callback-ref release sequence.
 - The two-user lifecycle test now covers opening `:inbox` before a private
   message arrives, matching the way users often leave an inbox page open.
 - Private-message inbox access now uses its own mutex instead of sharing the

docs/Development-Guide.md

@@ -131,6 +131,7 @@ typedef struct client {
     int ref_count;                  // Reference counting
     pthread_mutex_t ref_lock;
     pthread_mutex_t io_lock;        // Own SSH channel writes only
+    bool channel_callback_ref;      // Ref held while callbacks are installed
 } client_t;
 ```
 
@@ -284,6 +285,9 @@ void room_broadcast(chat_room_t *room, const message_t *msg) {
 - Cross-client lookups, such as mentions and private messages, must call
   `client_addref()` before using a client pointer outside `g_room->lock`, then
   `client_release()` when done.  Do not increment `ref_count` directly.
+- Session callback lifetime is owned by `client.c`: `client_install_channel_callbacks()`
+  takes the callback ref, and `client_release_session()` removes callbacks and
+  releases both the callback ref and the session main ref.
 
 ### 3. Message Persistence (message.c)
 

docs/ROADMAP.md

@@ -38,7 +38,8 @@ Goal: make TNT predictable for operators, scripts, and package maintainers.
 
 Goal: make long-running operation boring and reliable.
 
-- move client state to a clearer ownership model with one release path
+- ✅ move session callback ownership into `client.c` and release sessions
+  through one `client_release_session()` path
 - ✅ remove cross-client SSH channel writes from mention and private-message
   notifications
 - continue replacing ad hoc cross-thread UI mutation with per-client event
@@ -103,6 +104,5 @@ Goal: make regressions harder to introduce.
 
 These are the next changes that should happen before new feature work expands the surface area.
 
-1. Finish untangling client-state ownership into a clearer release path.
-2. Replace remaining release placeholders with real maintainer metadata and
+1. Replace remaining release placeholders with real maintainer metadata and
    source-archive checksums when cutting a public package release.

include/client.h

@@ -32,20 +32,18 @@ int client_printf(client_t *client, const char *fmt, ...);
 /* Reference counting for safe cross-thread cleanup.
  *
  * Lifecycle: bootstrap_run() creates the client_t with ref_count = 1
- * (the "main" ref), then adds a second ref before installing the channel
- * callbacks (the "callback" ref) so the client outlives any in-flight
- * eof / close / window-change callback invocation.  The interactive
- * session releases both refs in its cleanup path; the final release
- * frees the SSH session, channel, callback struct, and the client_t. */
+ * (the "main" ref).  client_install_channel_callbacks() takes a second
+ * ref owned by client.c while channel callbacks are installed, so the
+ * client outlives in-flight eof / close / window-change callbacks.
+ * input_run_session() ends ownership with client_release_session(). */
 void client_addref(client_t *client);
 void client_release(client_t *client);
+void client_release_session(client_t *client);
 
-/* Install the post-bootstrap channel callbacks (window-change, eof, close)
- * that target this client_t.  Caller MUST have already added one
- * client_addref() to keep the client alive across in-flight callback
- * invocations; the matching client_release() happens during cleanup in
- * input_run_session().  Returns 0 on success, -1 on failure (in which
- * case the caller still owns both refs and must release them). */
+/* Install the post-bootstrap channel callbacks (window-change, eof, close).
+ * On success this function takes the callback reference described above.
+ * On failure no callback reference remains and the caller still owns only
+ * its original main reference. */
 int client_install_channel_callbacks(client_t *client);
 
 #endif /* CLIENT_H */

include/ssh_server.h

@@ -74,6 +74,7 @@ typedef struct client {
     pthread_mutex_t ref_lock;        /* Lock for ref_count */
     pthread_mutex_t io_lock;         /* Serialize SSH channel writes */
     pthread_mutex_t whisper_lock;    /* Serialize whisper inbox access */
+    bool channel_callback_ref;       /* client.c owns one ref while callbacks are installed */
     struct ssh_channel_callbacks_struct *channel_cb;
 } client_t;
 

scripts/release_check.sh

@@ -109,6 +109,10 @@ step "checking client I/O ownership boundaries"
     fail "cross-client target-array writes must be queued through client_queue_bell"
 ! grep -n "pthread_mutex_lock(&.*->io_lock)" src/commands.c >/dev/null ||
     fail "commands.c must not use SSH io_lock for in-memory command state"
+! grep -n "client_addref(client)" src/bootstrap.c >/dev/null ||
+    fail "bootstrap.c must let client_install_channel_callbacks own callback refs"
+grep -q "client_release_session(client)" src/input.c ||
+    fail "input.c must release session ownership through client_release_session"
 if grep -R "ssh_channel_write" src include | grep -v "^src/client.c:" >/dev/null; then
     fail "raw SSH channel writes must stay inside src/client.c"
 fi

src/bootstrap.c

@@ -476,17 +476,12 @@ void *bootstrap_run(void *arg) {
     }
     client->exec_command_too_long = ctx->exec_command_too_long;
 
-    /* Add a ref for the channel callbacks (eof/close/window_change) so the
-     * client_t outlives any in-flight callback invocation. */
-    client_addref(client);
-
     if (client_install_channel_callbacks(client) < 0) {
         /* Nullify session/channel ownership so client_release won't
          * double-free what cleanup_failed_session is about to free. */
         client->session = NULL;
         client->channel = NULL;
-        client_release(client);  /* drop the callback ref (2 → 1) */
-        client_release(client);  /* drop the main ref (1 → 0, frees client) */
+        client_release(client);
         cleanup_failed_session(session, ctx);
         return NULL;
     }

src/client.c

@@ -244,6 +244,25 @@ void client_release(client_t *client) {
     }
 }
 
+void client_release_session(client_t *client) {
+    if (!client) return;
+
+    if (client->channel && client->channel_cb) {
+        ssh_remove_channel_callbacks(client->channel, client->channel_cb);
+    }
+    if (client->channel_cb) {
+        free(client->channel_cb);
+        client->channel_cb = NULL;
+    }
+
+    if (client->channel_callback_ref) {
+        client->channel_callback_ref = false;
+        client_release(client);
+    }
+
+    client_release(client);
+}
+
 /* Send formatted string to client */
 int client_printf(client_t *client, const char *fmt, ...) {
     char buffer[2048];
@@ -315,8 +334,13 @@ int client_install_channel_callbacks(client_t *client) {
         return -1;
     }
 
+    client_addref(client);
+    client->channel_callback_ref = true;
+
     client->channel_cb = calloc(1, sizeof(struct ssh_channel_callbacks_struct));
     if (!client->channel_cb) {
+        client->channel_callback_ref = false;
+        client_release(client);
         return -1;
     }
 
@@ -330,6 +354,8 @@ int client_install_channel_callbacks(client_t *client) {
     if (ssh_set_channel_callbacks(client->channel, client->channel_cb) != SSH_OK) {
         free(client->channel_cb);
         client->channel_cb = NULL;
+        client->channel_callback_ref = false;
+        client_release(client);
         return -1;
     }
 

src/input.c

@@ -995,17 +995,7 @@ cleanup:
 
     ratelimit_release_ip(client->client_ip);
 
-    /* Remove channel callbacks before releasing refs to prevent use-after-free
-     * if a callback fires between the two releases. */
-    if (client->channel && client->channel_cb) {
-        ssh_remove_channel_callbacks(client->channel, client->channel_cb);
-    }
-
-    /* Release the callback reference (paired with addref before client_install_channel_callbacks) */
-    client_release(client);
-
-    /* Release the main reference - client will be freed when all refs are gone */
-    client_release(client);
+    client_release_session(client);
 
     /* Decrement connection count */
     ratelimit_decrement_total();