Audit: non-atomic booleans and ints shared across threads (follow-up to #3786)

**Background.** In #3786 the discussion converged on two points: `CSocketThread::bRun` is written by one thread and read by another, so it should be `std::atomic`; and the same pattern probably exists elsewhere in the codebase. I audited `main` (55e0d62d) file by file, tracing each shared flag against the threads that actually touch it. Below is everything I found, with file/line references only (no patch — that can follow separately if there's interest).

One data point up front: there are currently **zero** uses of `std::atomic` or `QAtomic*` anywhere in `src/`.

**Why these matter, briefly.** Each item is a data race in the C++ memory-model sense (concurrent access, at least one write, no synchronization), which is formally undefined behavior. In practice the realistic failure modes are: (a) the compiler legally caching or hoisting a repeated read (more likely under LTO/aggressive optimization), and (b) delayed or reordered visibility on weakly-ordered CPUs — which today includes Apple Silicon Macs, Android (Oboe), and ARM Linux servers such as Raspberry Pi. For the non-`bool` items, torn reads of multi-byte values are also possible. On x86 most of these "work" today, which is why they've been harmless-looking for years.

**Thread model as audited.**
- Client: main/Qt event thread (GUI, protocol — protocol messages arrive via queued signal, so parsing runs on main), socket thread (`CSocketThread`, time-critical), audio-driver callback thread (JACK/CoreAudio/ASIO/Oboe).
- Server: main thread (including `OnTimer` mixing, reached via queued signal from the timer thread, and the protocol slots which lock `Mutex`), `CHighPrecisionTimer` thread (non-Windows), socket thread, `CThreadPool` workers, recorder thread.

---

### A. Thread-lifecycle `bRun` flags (the exact #3786 class)

**A1. `CSocketThread::bRun` — `src/socket.h:241`.**
Written `false` by the main thread in `Stop()` (socket.h:213); read in the thread loop (socket.h:231) on the socket thread. This is the #3786 case, unchanged on `main`.

**A2. `CHighPrecisionTimer::bRun` (non-Windows) — `src/util.h:1355`.**
Written by the main thread in `Start()`/`Stop()` (src/util.cpp:315, 339); read in the `run()` loop on the timer thread (src/util.cpp:348). Additionally read lock-free by a *third* thread: the socket thread calls `pServer->IsRunning()` (src/socket.cpp:744) → `CServer::IsRunning()` (src/server.h:136) → `isActive()` (src/util.h:1350) when deciding whether to wake a sleeping server. A missed `bRun = false` means the server's time-critical timer thread never exits its loop (`Stop()` then hits the 5 s `wait()` timeout with the thread still running). This affects every Linux/macOS server; the Windows variant is a `QTimer` on the main thread and is fine.

**A3. `CSoundBase::bRun` — `src/sound/soundbase.h:196`.**
Written by the main thread in `Start()` (soundbase.h:98) and `Stop()` (src/sound/soundbase.cpp:71); read on the realtime audio callback thread in every backend that checks it:
- `src/sound/jack/sound.cpp:368` (read inside `MutexAudioProcessCallback`, but the writers don't hold that mutex, so the race remains),
- `src/sound/coreaudio-mac/sound.cpp:1026` and `:1086`,
- `src/sound/oboe/sound.cpp:225` (no lock at all at the point of the read).
This flag is the only gate between the driver callback and the whole client audio processing chain.

**A4. `CSoundBase::bCallbackEntered` — `src/sound/soundbase.h:197`.**
Written `true` by the audio callback thread (soundbase.h:192), reset `false` by the main thread in `Start()` (soundbase.h:99), read by the GUI at `src/clientdlg.cpp:1192` to detect a dead audio device. Unsynchronized write/reset/read across three call sites and two threads; a stale read here produces a false "sound card not working" verdict (or masks a real one).

### B. Jitter-buffer status flags

**B1. `CSocket::bJitterBufferOK` — `src/socket.h:127`.**
Written `false` by the socket thread (src/socket.cpp:714); read-and-reset by the main thread in `GetAndResetbJitterBufferOKFlag()` (src/socket.cpp:519–526) with no lock — note the class *has* a `Mutex` (used in `SendPacket`, socket.cpp:443), it just isn't used here. Lost updates show a green jitter-buffer LED during dropouts.

**B2. `CClient::bJitterBufferOK` — `src/client.h:434`.**
Same pattern one level up: written `false` by the sound thread (src/client.cpp:1558), reset/read by the main thread (client.cpp:634, 641, 1084).

### C. GUI-thread parameters read by the audio callback (client)

All written by plain setters on the main thread, read every frame by `ProcessAudioDataIntern()` on the sound thread, no atomics or locks:

| Member | Declared | Written | Read (sound thread) |
|---|---|---|---|
| `bMuteOutStream` | src/client.h:393 | client.h:281 | client.cpp:1504, 1516, 1534, 1584 |
| `iInputBoost` | src/client.h:409 | client.h:290 | client.cpp:1427–1433 |
| `iReverbLevel` | src/client.h:407 | client.h:192 | client.cpp:1443–1445 |
| `bReverbOnLeftChan` | src/client.h:406 | client.h:197 | client.cpp:1445 |
| `iAudioInFader` | src/client.h:405 | client.h:189 | client.cpp:1449+ |

One of these is worse than a flag race: **`SetReverbOnLeftChan()` (src/client.h:195–199) calls `AudioReverb.Clear()` on the GUI thread** while the sound thread may be inside `AudioReverb.Process()` (client.cpp:1445). That is a concurrent mutation of non-trivial filter state, not just a stale boolean.

(For contrast: parameters whose setters stop/re-init/restart the sound — `SetAudioChannels`, `SetAudioQuality`, `SetEnableOPUS64`, `SetSndCrdDev`, `OnRawAudioSupported`, etc. — are structurally fine, though their "is the callback really stopped" handshake itself rides on the non-atomic `bRun` of A3.)

### D. Channel state (client side)

**D1. `CChannel::bIsEnabled` — `src/channel.h:239`.**
Written under `Mutex` by `SetEnable()` (src/channel.cpp:152) on the main thread at connect/disconnect (client.cpp:1039, 1056); but read *without* the mutex via `IsEnabled()` (channel.h:106) by the socket thread in `PutAudioData()` (channel.cpp:561). A mutex only on the writer side synchronizes nothing. (Server side is safe by accident: channels are enabled once in the constructor, server.cpp:241, before any thread exists.)

**D2. `CChannel::iConTimeOut` — the connection-liveness counter.**
`Disconnect()` writes `iConTimeOut = 1` with **no lock** (src/channel.cpp:536, main thread), while the socket thread resets it under `MutexSocketBuf` (channel.cpp:613 via channel.h:101) and the audio path decrements it under `MutexSocketBuf` (channel.cpp:635–641). `IsConnected()` (channel.h:102) reads it with no lock from several threads. A lost update here delays or cancels a disconnect — possibly related to the comment in `CClient::Stop()` that "disconnect is still not working reliably" (client.cpp:1065).

**D3. `CChannel::iNetwFrameSize` / `iNetwFrameSizeFact` — mismatched mutexes.**
Written under `Mutex` in `SetAudioStreamProperties()` (src/channel.cpp:201–224); read under `MutexSocketBuf` — a *different* mutex — by the socket thread in `PutAudioData()` (channel.cpp:566). Two different mutexes provide no mutual exclusion; the packet-size check can see a torn/stale value while transport properties change.

### E. Server

**E1. `CServer::bChannelIsNowDisconnected` — `src/server.h:260`.**
Written by `CThreadPool` workers (src/server.cpp:950, enqueued at server.cpp:710) — potentially by several workers in the same tick. The read at server.cpp:722 *is* synchronized (it happens after `future.wait()`), but two workers storing concurrently is still formally a data race. This site carries the exact comment discussed in #3786 (server.cpp:947–949):

> // note that no mutex is needed for this shared resource since it is not a
> // read-modify-write operation but an atomic write …

A plain `bool` store is not an atomic write in the C++ memory model; making the member `std::atomic` would make the comment true at zero practical cost.

### F. Shutdown path

**F1. `CSocket::UdpSocket4` / `UdpSocket6` in `Close()` — `src/socket.cpp:389+`.**
`Close()` (main thread) shuts the sockets and overwrites the fds with `INVALID_SOCKET`, with no lock, while the socket thread is concurrently using those same fds in the blocking `poll`/`recvfrom` loop of `OnDataReceived()`, and while `SendPacket()` (which *does* take `Mutex`, socket.cpp:443) may be sending from the sound thread. Close-while-in-use is the intended unblock mechanism, but the unsynchronized fd overwrite adds a stale-fd/fd-reuse hazard on top of the `bRun` race of A1.

---

### Checked and found properly synchronized (for completeness)

- `CServer::PutAudioData()` and both protocol slots lock the server `Mutex` (server.cpp:1562, 1570, 1585), so the server's put/get audio path and channel protocol state are serialized with `OnTimer`.
- `OnTimer` holds `Mutex` for the whole decode phase and joins all workers via `future.wait()` before reading their results (server.cpp:667–719).
- `CChannel::SetAudioStreamProperties()` re-initializes `SockBuf`/`ConvBuf` under `MutexSocketBuf`/`MutexConvBuf` respectively (channel.cpp:226–239); `SetGain`/`GetGain` both lock `Mutex`.
- `CThreadPool` (src/threadpool.h) guards its `stop` flag and queue with `queue_mutex` + condition variable.
- The recorder runs in its own `QThread` and communicates via queued signals only.

### Suggested direction (no patch here by design)

The `bool` cases are one-line conversions to `std::atomic` using the overloaded operators (per the conclusion in #3786, the default sequentially-consistent operations are negligible at these rates). `iConTimeOut` and the C-class ints would become `std::atomic`; D3 wants either one mutex or atomics for the two size fields; C's `AudioReverb.Clear()` needs to move onto the audio thread or under `MutexAudioProcessCallback`. Happy to follow up with a PR if the direction is agreed.

Start with #3786, then trace the shared members and entry points listed in src/socket.h, src/util.h, src/sound/soundbase.h, src/client.h, src/channel.h, src/server.h, and src/socket.cpp. Compare each access with the existing mutexes and thread boundaries, paying special attention to AudioReverb and socket shutdown. Done means the listed races and cross-thread state hazards are addressed without disturbing the paths already identified as synchronized.

Written by the indexing model from the issue text.