From a03819cd75435e3721f6d0b47b5822ecd7e7198a Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 21 Sep 2026 06:22:56 +0000 Subject: [PATCH 1/2] Bump sendspin-cpp pin to v0.8.0 Picks up the v0.8.0 release, whose connection-lifecycle work matters to the player: an established connection is now dropped after a bounded period of inbound silence so a blackholed socket reconnects instead of hanging forever, and role-thread stops became interruptible, cutting teardown from hundreds of milliseconds to roughly zero. The release also replaces start_server() with a synchronous start()/stop() lifecycle, so our call site moves to start() ahead of the v0.9.0 removal. Co-Authored-By: Claude Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EZYT7MkFLzBTfsZAXkur3B --- c_src/sendspin_player/CMakeLists.txt | 2 +- c_src/sendspin_player/src/main.cpp | 4 ++-- test/sendspin_player_contract_test.exs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/c_src/sendspin_player/CMakeLists.txt b/c_src/sendspin_player/CMakeLists.txt index 22d9d72..3997585 100644 --- a/c_src/sendspin_player/CMakeLists.txt +++ b/c_src/sendspin_player/CMakeLists.txt @@ -21,7 +21,7 @@ set(CMAKE_CXX_FLAGS_RELEASE "-O3 -DNDEBUG" CACHE STRING "" FORCE) # build dirs. This also means `-DSENDSPIN_CPP_REF=...` cannot override; # for scratch experiments against another ref use # `-DFETCHCONTENT_SOURCE_DIR_SENDSPIN-CPP=` instead. -set(SENDSPIN_CPP_REF "30514d5102c269a0c7fa6a13932d6bf7f2ae1abc" # v0.7.2 +set(SENDSPIN_CPP_REF "9331ace6428979982c934384702d289958ca125e" # v0.8.0 CACHE STRING "sendspin-cpp commit SHA to pin" FORCE) # Trim sendspin-cpp to just the player role. Other roles ship dead code on a diff --git a/c_src/sendspin_player/src/main.cpp b/c_src/sendspin_player/src/main.cpp index 97f858c..0bd3c5f 100644 --- a/c_src/sendspin_player/src/main.cpp +++ b/c_src/sendspin_player/src/main.cpp @@ -653,7 +653,7 @@ struct ClientListener : SendspinClientListener { // sendspin-cpp binds its WebSocket listener lazily — on the first // client.loop() tick after the network provider reports ready — so -// start_server() returning does NOT mean the port accepts connections +// start() returning does NOT mean the port accepts connections // yet. The Elixir side must not advertise the player over mDNS before // the listener is up: Music Assistant's discovery connect is one-shot // (aiosendspin `retry_initial_connection=False`), so a connection @@ -792,7 +792,7 @@ int main(int argc, char* argv[]) { emit_json(os.str()); } - if (!client.start_server()) { + if (!client.start()) { emit_json("{\"event\":\"error\",\"kind\":\"start_server\"," "\"msg\":\"failed to bind WebSocket listener\"}"); return 1; diff --git a/test/sendspin_player_contract_test.exs b/test/sendspin_player_contract_test.exs index 32ae32f..ffb55b6 100644 --- a/test/sendspin_player_contract_test.exs +++ b/test/sendspin_player_contract_test.exs @@ -7,7 +7,7 @@ defmodule SendspinPlayerContractTest do Spawns the host build of the binary, drives stdin/stdout, and asserts the documented event/command shapes. Does not require audio hardware — - the binary's `start_server()` binds a non-privileged WebSocket port and + the binary's `start()` binds a non-privileged WebSocket port and does not touch ALSA until a stream begins. """ From bf70763c2a788fc104256f3ca45c0d9fa73eedad Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 21 Sep 2026 09:05:02 +0000 Subject: [PATCH 2/2] Address review: stop() on shutdown, accurate start() diagnostics, doc pin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Shutdown now calls stop() instead of disconnect(), so the role threads are joined while the listeners they call into are still alive — `client` is declared before `audio_sink` and the listeners, and every PlayerListener callback dereferences the sink. Also corrects the start() failure event, which claimed a bind failure that start() cannot report, and updates the README pin note and the contract test's lifecycle description. Co-Authored-By: Claude Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EZYT7MkFLzBTfsZAXkur3B --- c_src/sendspin_player/README.md | 4 ++-- c_src/sendspin_player/src/main.cpp | 19 ++++++++++++++++--- test/sendspin_player_contract_test.exs | 5 +++-- 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/c_src/sendspin_player/README.md b/c_src/sendspin_player/README.md index 13e62d9..791bc2d 100644 --- a/c_src/sendspin_player/README.md +++ b/c_src/sendspin_player/README.md @@ -69,8 +69,8 @@ Differences: ## sendspin-cpp pin Pinned via `FetchContent` to a **commit SHA** in `CMakeLists.txt` -(`SENDSPIN_CPP_REF` cache var). Currently `30514d51...`, upstream -`v0.7.2`. We pin to a SHA rather +(`SENDSPIN_CPP_REF` cache var). Currently `9331ace6...`, upstream +`v0.8.0`. We pin to a SHA rather than a tag because git tags are mutable server-side; a retagged upstream would otherwise silently flow into firmware. The friendly tag name (or nearest-release note) lives in a comment beside the SHA for traceability. diff --git a/c_src/sendspin_player/src/main.cpp b/c_src/sendspin_player/src/main.cpp index 0bd3c5f..03518db 100644 --- a/c_src/sendspin_player/src/main.cpp +++ b/c_src/sendspin_player/src/main.cpp @@ -792,9 +792,13 @@ int main(int argc, char* argv[]) { emit_json(os.str()); } + // A false return means a role failed to start (thread spawn, ring + // buffer allocation); it does NOT mean the listener failed to bind, + // which cannot be known yet — start() only arms the server, and the + // bind happens on a later loop() tick (see the hook comment above). if (!client.start()) { - emit_json("{\"event\":\"error\",\"kind\":\"start_server\"," - "\"msg\":\"failed to bind WebSocket listener\"}"); + emit_json("{\"event\":\"error\",\"kind\":\"start\"," + "\"msg\":\"failed to start client roles\"}"); return 1; } @@ -874,7 +878,16 @@ int main(int argc, char* argv[]) { std::this_thread::sleep_for(std::chrono::milliseconds(10)); } - client.disconnect(SendspinGoodbyeReason::SHUTDOWN); + // stop() rather than disconnect(): it sends the shutdown goodbye AND + // joins the role threads before returning. That ordering is load- + // bearing here — `client` is declared before `audio_sink` and the + // listeners, so reverse destruction would otherwise tear those down + // while ~SendspinClient() has yet to join the threads that call into + // them. Every PlayerListener callback dereferences `sink` (including + // on_audio_write on the role thread's hot path), so a late callback + // would touch a destroyed stack object. Upstream states the contract + // directly: listeners must outlive the client. + client.stop(); emit_json("{\"event\":\"shutdown\"}"); return 0; } diff --git a/test/sendspin_player_contract_test.exs b/test/sendspin_player_contract_test.exs index ffb55b6..968c4c6 100644 --- a/test/sendspin_player_contract_test.exs +++ b/test/sendspin_player_contract_test.exs @@ -7,8 +7,9 @@ defmodule SendspinPlayerContractTest do Spawns the host build of the binary, drives stdin/stdout, and asserts the documented event/command shapes. Does not require audio hardware — - the binary's `start()` binds a non-privileged WebSocket port and - does not touch ALSA until a stream begins. + the binary listens on a non-privileged WebSocket port (`start()` arms + the server; the bind lands on a later `loop()` tick) and does not touch + ALSA until a stream begins. """ use ExUnit.Case, async: false