Repository navigation
Conversation
On Windows every socket call fails until WSAStartup() runs, and IXWebSocket leaves that to
the application through ix::initNetSystem(). Without it the client could neither dial a
server ("Either the application has not called WSAStartup") nor listen (setsockopt on the
listening socket fails).
SendspinWsServer::start() and SendspinClientConnection::start() now call
ix::initNetSystem() once, through a function-local static, before their first socket. It is
never undone, as Windows releases Winsock at process exit, and is a no-op on POSIX.
- Build PortAudio v19.7.0 from source as a static library on Windows, which has no pkg-config or system PortAudio; CMAKE_POLICY_VERSION_MINIMUM lets CMake 4 accept its cmake_minimum_required(VERSION 3.1) - Add a minimal getopt() for Windows (examples/common/win_getopt.h) - Stand in for SIGUSR1/SIGUSR2, which Windows lacks, with the console keys 'p' (pairing window gesture) and 'c' (cancel) - Persist through _open/_write/_commit and std::filesystem::rename(), which replaces an existing file where std::rename() does not, and fall back to %USERPROFILE% when HOME is unset The tui_client and source_client examples remain POSIX-only.
clang-tidy's readability-identifier-naming wants UPPER_CASE for static constants.
wait_for_start_acknowledgement() decided whether a COMMAND_START had arrived from the flags it read at the top of its loop, then cleared COMMAND_START separately. An acknowledgement landing between the two was cleared without being seen: when it was stale (an earlier stream than the header the task holds), the task skipped the idle note and went back to waiting for a start, while the main loop kept that stream's STREAM_END parked until the note arrived. Both sides waited forever. The decision now reads the bits clear() returns, which EventFlags captures atomically with the clear on both platforms. This restores the contract docs/internals.md describes under "Stream End and Start". Found as a rare hang of ClientLifecycle.StreamEventsDrainedTogetherStartEachStreamOnItsOwnStart in CI. A 2 ms sleep between the read and the clear hung that test on its first iteration; with this change it passed 40 of 40 with the sleep still in place.
This was referenced Oct 21, 2026
Member
Author
|
Split into three focused PRs:
Together they carry every change here, so this one can be closed. |
This was referenced Oct 27, 2026
kahrendt
pushed a commit
that referenced
this pull request
Oct 28, 2026
On Windows every socket call fails until `WSAStartup()` runs, and
IXWebSocket leaves that to the application through
`ix::initNetSystem()`. The client could neither dial a server ("Either
the application has not called WSAStartup") nor listen.
## Summary
- `SendspinWsServer::start()` and `SendspinClientConnection::start()`
call `ix::initNetSystem()` once, through a function-local static, before
their first socket. It is never undone; Windows releases it at process
exit.
- The two handle a failure differently on purpose. The server's
`start()` returns a bool, so it logs and returns false. The client's
`start()` has no failure return, so it logs and lets the attempt go
ahead; its first socket call then fails and IXWebSocket reports it as
the connection's `Error`, which already reports the close.
## Notes
- A no-op on POSIX (`initNetSystem()` returns true without doing
anything there).
- Follows #210, which made the library compile on Windows with clang-cl.
Split out of #211.
## Testing
- Windows 11, clang-cl 22.1.3 (from #211): `basic_client` dials Music
Assistant with `-u` and listens on its own port; before this change both
failed with the WSAStartup error.
- Linux, GCC 16: host build clean; `sendspin_tests` all pass. The
ASan/UBSan and TSan runs are this PR's CI (no sanitizer runtime on the
local machine).
kahrendt
pushed a commit
that referenced
this pull request
Oct 28, 2026
`SyncTask::wait_for_start_acknowledgement()` could erase a stale start acknowledgement without noting idle for it, leaving the main loop's parked `STREAM_END` waiting forever. ## Summary - **The race.** The loop read `flags` once at the top, then cleared `COMMAND_START` and decided whether an acknowledgement had arrived from that earlier snapshot. A stale acknowledgement (the start of a stream whose end overtook it) landing between the read and the clear was cleared but never seen, so the sync task skipped the `note_sync_idle()` the main loop's held `STREAM_END` waits for. Playback then deadlocked. - **The fix.** Decide from the bits `event_flags_.clear()` returns. `EventFlags::clear()` returns the pre-clear bits on both platforms, so whatever the clear took is what gets judged. ## Notes - Not platform-specific. It showed up as a rare CI hang of `ClientLifecycle.StreamEventsDrainedTogetherStartEachStreamOnItsOwnStart`, and was split out of #211. - No new test. The window sits between two statements inside one function, and a deterministic test would need a hook in production code to stop between them, which the test standards rule out. The existing lifecycle test covers the behaviour; the reproduction below shows it catches the race once the window is widened. - `docs/internals.md` "Stream End and Start" already describes the intended behaviour (a stale acknowledgement notes idle), so it is unchanged. ## Testing - Reproduced by adding a 2 ms sleep before the clear: without the fix the lifecycle test hung 3/3 runs; with the fix it passed 20/20 with the sleep in place (and 40/40 earlier, in #211). - Linux, GCC 16: host build clean; `sendspin_tests` 705/705. The ASan/UBSan build could not link on this machine (no sanitizer runtime installed), so the sanitizer and TSan runs are this PR's CI.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #210, which made the library compile on Windows with clang-cl. Running it there for the first time turned up problems a compile alone does not; this draft collects the library fixes and example changes found while bringing up a Windows player, starting with
basic_clientand then sendspin-cpp-cli.So far
WSAStartup()runs, and IXWebSocket leaves that to the application throughix::initNetSystem(). The client could neither dial a server ("Either the application has not called WSAStartup") nor listen.SendspinWsServer::start()andSendspinClientConnection::start()now call it once before their first socket; it is a no-op on POSIX.basic_clientbuilds and runs on Windows. PortAudio v19.7.0 is built from source as a static library (no pkg-config on Windows), a minimalgetopt()stands in for the missing one, the console keysp/cstand in forSIGUSR1/SIGUSR2as the pairing-window gesture, and the persistence provider writes through_open/_write/_commitandstd::filesystem::rename()(std::rename()will not replace an existing file on Windows), falling back to%USERPROFILE%whenHOMEis unset.SyncTask::wait_for_start_acknowledgement()decided whether aCOMMAND_STARThad arrived from a flag snapshot taken before clearing the bit, so a stale acknowledgement landing in between was erased without the idle note the main loop's parkedSTREAM_ENDwaits for. It now reads the bitsclear()returns. This was a rare CI hang ofClientLifecycle.StreamEventsDrainedTogetherStartEachStreamOnItsOwnStart, not Windows-specific; widening the window with a 2 ms sleep hung it on the first iteration, and with the fix it passed 40/40 with the sleep in place.Notes
%USERPROFILE%grants only the user, SYSTEM and Administrators.basic_clienthas no mDNS on Windows, so a server cannot discover it; it dials one with-u.strerror(errno), while Winsock reports throughWSAGetLastError(), so a failedbindon Windows logs "Unknown error". The one hit so far was a real port conflict (8928 held by another Sendspin client on the same machine).tui_client,source_clientand the tests remain POSIX-only.Testing
sendspinandbasic_clientbuild with no warnings from the library's own sources.basic_client.exe -p 8929 -u ws://<music-assistant>:8927/sendspinagainst Music Assistant: Noise handshake complete, connection admitted (unpaired), local server listening. Pairing and playback not yet exercised.