From 6e72f061dabbee53f7989a5ca785d290ec326e06 Mon Sep 17 00:00:00 2001 From: Kevin Ahrendt Date: Wed, 11 Nov 2026 08:16:46 -0500 Subject: [PATCH] Document that callbacks never call loop() loop() runs every main-loop callback and does not support being re-entered, so the public loop() doc, the integration guide's Thread Safety Summary and conventions.md now say no listener, role or provider callback calls it, and internals.md notes that the only drain nested beneath another is the one stop() runs. Closes #257. --- docs/conventions.md | 5 +++-- docs/integration-guide.md | 2 ++ docs/internals.md | 2 +- include/sendspin/client.h | 3 +++ 4 files changed, 9 insertions(+), 3 deletions(-) diff --git a/docs/conventions.md b/docs/conventions.md index f2c57ad2..6ce5e4c3 100644 --- a/docs/conventions.md +++ b/docs/conventions.md @@ -78,8 +78,9 @@ checklists in `.claude/skills/` apply these standards to a diff. other producers running cannot reset it, and a drain can take a slot on either side of a teardown. - Callback dispatch must tolerate re-entrant teardown: a listener callback may - call back into the client. See "Re-entrant Teardown During Callback - Dispatch" in `docs/internals.md` for the guard patterns in use. + call back into the client, except `loop()` (see `client.h`). See "Re-entrant + Teardown During Callback Dispatch" in `docs/internals.md` for the guard + patterns in use. - A message handler on the protocol task does the connection work the message calls for in place (an activation's admissibility check, admission and role ownership, a pairing step, a `noise/handshake` re-handshake, the RAM commit diff --git a/docs/integration-guide.md b/docs/integration-guide.md index 3b15a325..e1695cd9 100644 --- a/docs/integration-guide.md +++ b/docs/integration-guide.md @@ -1199,6 +1199,8 @@ Most listener callbacks fire on the main loop thread (the thread calling `client | `SendspinClientConfig::message_trace` (built with `SENDSPIN_ENABLE_MESSAGE_TRACE`) | Protocol task | | All other listener methods | Main loop thread | +Never call `loop()` from a listener, role or provider callback. A callback on the main loop already runs inside one of the client's own calls (`loop()`, `start()`, `stop()` or its destructor), and `loop()` does not support being re-entered; a callback the table lists on another thread is off the main loop, where `loop()` never runs. + `PlayerRole::notify_audio_played()` is thread-safe and is designed to be called from an audio output callback thread. `ControllerRole::send_command()` is callable from any thread. diff --git a/docs/internals.md b/docs/internals.md index 6e53ebef..a2790103 100644 --- a/docs/internals.md +++ b/docs/internals.md @@ -226,7 +226,7 @@ The source's `cleanup()` closes the stream gate without a message and clears the ### Re-entrant Teardown During Callback Dispatch -Requests a listener callback makes (`connect_to()`, `disconnect()`, ...) are queued or posted to the protocol task, so no callback tears a connection down under the drain that fired it. `stop()` is the exception: called from a callback, it joins the protocol task, tears every role down on the main loop and runs a drain of its own beneath the one that fired the callback, and a `start()` that follows it in the same callback begins a new run. Two generation counters keep the outer drain from acting on what it already took: +Requests a listener callback makes (`connect_to()`, `disconnect()`, ...) are queued or posted to the protocol task, so no callback tears a connection down under the drain that fired it. `stop()` is the exception: called from a callback, it joins the protocol task, tears every role down on the main loop and runs a drain of its own beneath the one that fired the callback, and a `start()` that follows it in the same callback begins a new run. No other drain runs beneath one, since a callback never calls `loop()` (`client.h`). Two generation counters keep the outer drain from acting on what it already took: - `drain_generation` (on the client's `EventState`) is bumped by `stop()` before its teardown. The outer drain snapshots it once and stops as soon as it changes: the event ring between events, the pairing-note dispatch between notes, and the role drains and the group update before each starts. - Each role's `cleanup_generation` is bumped by its `cleanup()` and stamped onto every event and slot payload it queues afterwards; the ring drain drops stream events whose stamp no longer matches (`event_is_current()`). Inside a role drain the same counter catches a callback that re-entered teardown: the player checks it after each server-command callback and around each `on_stream_start()` and `on_stream_end()`, metadata and color by ending their drain once the teardown's catch-up has forgotten the pending deadline (`ScheduledStateSlot::pending_at()`), and artwork after each display or clear, and each abandons the rest of its work when it moved on. diff --git a/include/sendspin/client.h b/include/sendspin/client.h index 7894637d..49a53406 100644 --- a/include/sendspin/client.h +++ b/include/sendspin/client.h @@ -412,6 +412,9 @@ class SendspinClient { /// Connection work (handshakes, time sync, sends, watchdogs) runs on the library's protocol /// task and does not wait for this call. Each call while running also polls the network /// provider, which is what starts a WebSocket server the network kept down at start(). + /// + /// Never call it from a listener, role or provider callback (docs/integration-guide.md, Thread + /// Safety Summary). void loop(); // ========================================