Shutdown should be an out-of-band signal selected at the task boundary, not a queued command

[jadamcrain]

Channel::shutdown() and ServerHandle::shutdown() (#194, unreleased) deliver shutdown as a Command::Shutdown / ServerCommand::Shutdown sent over the same bounded queue that carries normal work. This makes shutdown strictly cooperative: it is only observed at points already polling that queue, so it can be delayed for a long time or blocked indefinitely.

Delayed: with an unresponsive peer, default max_queued_requests (16) and a 5s response timeout, shutdown().await first blocks waiting for a queue slot, then the command is processed only after the backlog drains — the task lives ~80s past the call.

Blocked: the loop can park in the unbounded write at client/task.rs:273 (see #195), in which case shutdown never takes effect and, once the queue fills, shutdown().await itself never returns.

Blind spots beyond the request loop

Several paths never poll the command queue at all:

• client TLS handshake — tcp/client.rs:159 awaits the connection handler, reaching tcp/tls/client.rs:174
• server response writes — server/task.rs:81 and :226 run inside handle_frame, outside the command-aware select at :123
• server TLS handshake — tcp/server.rs:248 → tcp/tls/server.rs:58
• server fanout — tcp/server.rs:151 awaits each session's bounded queue; while inside apply_command the outer select at :157 isn't running
• user-supplied listener futures, e.g. tcp/client.rs:183

Adding a select branch at each site is not worth it, and would still miss the next one added. The signal should be selected once around the entire task future.

Proposed change

Replace the queued variants with a tokio::sync::watch<bool>, held as Arc<watch::Sender<bool>> in the handle since both handles are Clone. (tokio-util is not currently a dependency, so CancellationToken would add one for no gain. Notify is the wrong primitive: it is an event rather than persistent state, and notify_waiters() retains no permit, so a session spawned during the race would miss shutdown.)

Select it once, biased, against the whole task future:

• TcpChannelTask::run / SerialChannelTask::run — one select covers connect, TLS handshake, request write, response wait, reconnect backoff and listener callbacks. The terminal listener.update(ClientState::Shutdown) must run after the select, not inside the cancelled future.
• ServerTask::run for the accept loop, plus one select around each run_session().

Keep the existing mpsc-drop path for "task completes when every handle is dropped" — unchanged.

Delete Command::Shutdown and ServerCommand::Shutdown. That also allows reverting the ServerSetting → ServerCommand rename: nothing outside rodbus/src/ references either name, and the rename existed only to host the new variant. It removes the silent no-op ServerCommand::Shutdown => return arm at tcp/server.rs:145 as well, which today does nothing and depends on the caller at :161 intercepting the variant first.

Net effect is a smaller change than what merged: two enum variants, a rename, and the per-site plumbing all go away.

JoinSet for sessions

Track sessions in a JoinSet instead of the detached tokio::spawn at tcp/server.rs:236, and drain it after the select. Joining the server task then guarantees every session task has finished, which it does not today.

Sender-drop is not sufficient on its own. A session does treat a closed command channel as terminal (None => Err(RequestError::Shutdown), server/task.rs:130), but only while it is sitting in that select — one wedged in the response write at :226 never reaches it and outlives the server indefinitely. Sessions therefore need the watch receiver directly.

API

Nothing has shipped, so shutdown() can become pub fn shutdown(&self) — sync, infallible, idempotent. A failed watch::Sender::send means there are no receivers, i.e. the task already exited, which is success for a shutdown call.

No FFI impact: the C bindings shut down by destroying the handle (ffi/rodbus-schema/src/client.rs:136, server.rs:199), and #194 changed no files under ffi/.

Dropping the work future is safe

• Promises resolve. Promise::drop calls failure(RequestError::Shutdown) (client/message.rs:315), covering both requests buffered in the mpsc and the in-flight one held as a local in execute_request. No caller is left waiting on a dead oneshot.
• No lock can be stranded. The only locks on these paths are the guards at server/task.rs:222 and :234, both statement-temporaries released before the following await. This is structural, not incidental: these futures are tokio::spawned and so must be Send, and std::sync::MutexGuard is not, so the compiler already rejects holding one across an await.
• Partial writes don't matter here. The transport drops with the future and nothing reuses it. This is precisely why cancelling at the task boundary avoids the cancel-safety hazard that an in-place write timeout has to handle (Request write is outside the response timeout, leaving requests unbounded and blocking reconnect #195).
• TLS skips close_notify on drop, so peers may log an unclean close. Every existing error path already behaves this way.

One pre-existing wrinkle, not a blocker: Promise::complete invokes PromiseInner::Boxed(callback) (client/message.rs:308), so dropping the future runs user callbacks synchronously inside drop, and a panicking callback becomes a panic-in-drop. This is already reachable from every current shutdown and error path; cancellation adds one more.

Tests

The tests added in #194 largely survive — they assert that post-shutdown sends fail and that queued requests resolve to RequestError::Shutdown, both still true. The ones that drive shutdown through the command queue need rewriting against the new signal. Worth adding: shutdown while parked in a write to an unresponsive peer, and a server-side assertion that sessions have joined once the server task completes.