docs: write down the server's CChannel threading and locking model

[mcfnord]

🤖 AI: A server CChannel is shared between the main thread and the socket thread, and nothing in the tree says which lock covers which member - #3930 and #3932 were both instances of that gap. Below is that model written down as the code implements it, proposed as docs/THREADING.md beside docs/JAMULUS_PROTOCOL.md (or as a section of an existing file, if a new one is unwelcome). It adds no behaviour. The one finding in it that is more than bookkeeping: with the thread map measured, only two members are left unprotected, and one of them - InetAddr - fails the same way the #3930 channel-name bug did.

Proposed content:

---

Server threading and channel locking

Two threads touch a server CChannel concurrently, and this note writes down which lock covers which member. It describes the code as it is; it adds no behaviour. (Scope: the Linux server without --multithreading; see Verification at the end.)

The main thread does almost everything. CServer::OnTimer() runs here (the timer object emits from its own QThread, and the default Qt::AutoConnection queues the slot to the thread CServer lives on). Protocol handling runs here too: CSocket emits ProtocolMessageReceived from the socket thread, the queued connection delivers it to CServer::OnProtocolMessageReceived on the main thread, and that call - holding CServer::Mutex - drives every protocol slot of CChannel: SetChanInfo, SetGain/SetPan, OnNetTranspPropsReceived, OnVersionAndOSReceived, OnJittBufSizeChange. JSON-RPC handlers also run on the main thread and read channels through CServer::GetConCliParam().

The socket thread (CSocketThread) does exactly one thing: CServer::PutAudioData(), which takes CServer::Mutex, feeds each incoming audio packet to CChannel::PutAudioData(), and - when a packet arrives from a new address - initialises a channel in CServer::InitChannel(): SetAddress, ResetInfo, SetGain/SetPan.

The recorder (JamRecorder) receives AudioFrame over a queued connection with copied arguments and shares no channel state.

One timer tick

CServer::Mutex is the boundary between the two threads. OnTimer() holds it for the first half of the tick and releases it before the second:

 main thread   |== CServer::Mutex held ==========|== released =================|
 (one tick)    | collect connected channels      | channel levels              |
               | decode  (DecodeReceiveData)     | mix + send                  |
               |                                 |  (MixEncodeTransmitData,    |
               |                                 |   PrepAndSendPacket)        |
 socket thread | a packet arriving here blocks   | PutAudioData / InitChannel  |
               | on CServer::Mutex               | run IN PARALLEL with mix    |

So the racy question is always the same one: what does the socket thread write, and does the mix phase or an RPC handler read it without a common lock? Everything else is serialised - either both sides hold CServer::Mutex, or both sides are the main thread.

What a reader must hold, member by member

Member(s)	Writers (thread)	A reader on another thread must
ChannelInfo, incl. the channel name	ResetInfo (socket), SetChanInfo (main)	take Mutex - GetName/GetChanInfo do (since #3930)
bIsIdentified	same writers	nothing - std::atomic (since #3932)
vecfGains, vecfPannings	InitChannel (socket), protocol slots (main)	take Mutex - GetGain/GetPan do
SockBuf contents, iFadeInCnt	PutAudioData (socket)	take MutexSocketBuf, or read in the decode phase under CServer::Mutex (what GetFadeInGain relies on)
iConTimeOut	PutAudioData (socket)	nothing - std::atomic (IsConnected)
InetAddr	SetAddress (socket)	nothing exists - gap, see below
SignalLevelMeter	Reset in PutAudioData (socket)	nothing exists - gap, see below
transport properties (eAudioCompressionType, iNumAudioChannels, iNetwFrameSize, iNetwFrameSizeFact, iCeltNumCodedBytes, iAudioFrameSizeSamples, iFadeInCntMax), iCurSockBufNumFrames, bDoAutoSockBufSize, bUseSequenceNumber, ConvBuf, iSendSequenceNumber	main thread only	nothing extra today - the socket thread's few reads of them in PutAudioData are under CServer::Mutex, which the writers hold; the lock-free inline getters are safe because no second thread calls them
bIsServer, iConTimeOutStartVal	constructor only	nothing

The two gaps

• InetAddr - written lock-free by SetAddress (its only server-side caller is InitChannel); read lock-free in the mix phase (the level and recorder sends, and PrepAndSendPacket) and in GetConCliParam. CHostAddress is a QHostAddress plus a port, and QHostAddress is reference-counted, so a copy that overlaps operator= is the same shape as the QString copy Ensure ChannelInfo mutex taken #3930 fixed: of the two gaps, this is the one whose outcome is a use-after-free rather than a stale value. Written once per new connection.
• SignalLevelMeter - Reset() on a new connection (socket thread, under MutexSocketBuf) against Update() from the level pass in the mix phase (no lock). Two doubles.

Verification

Verified at commit 0545fddd (carries #3930 and #3932), on Linux, Qt 5.15.13. Thread attribution is measured, not inferred from the connect calls: in ThreadSanitizer runs under 8-client connection churn, OnTimer and OnProtocolMessageReceived appear only on the main thread, PutAudioData and InitChannel only on CSocketThread; gdb breakpoints on a live server show PutAudioData on thread 2 (CSocketThread) and OnTimer, SetChanInfo and the JSON-RPC path into GetConCliParam on thread 1. A QMutex-aware ThreadSanitizer build reports the InetAddr pair (once in 300 s of churn); the same churn under AddressSanitizer with recording enabled produced no report in 1508 connections over 600 s. Not measured, so not covered here: the client's use of CChannel, Windows, macOS, the GUI dialog, and --multithreading.

---

For review, the load-bearing anchors at 0545fddd: the queued socket-to-server connection that puts protocol slots on the main thread; the OnTimer lock scope (server.cpp:669-727, mix from :729); CServer::PutAudioData and InitChannel on the socket thread; the lock-free SetAddress/GetAddress pair (channel.h:109-110, read at server.cpp:748/:756/:1628; SignalLevelMeter at channel.cpp:636/:738; qhostaddress.h:160 is the QExplicitlySharedDataPointer).

If the mapping is right, a PR adding the file can follow. Whether InetAddr gets a fix, and of which shape, is the question the mapping raises.

---

🤖 This message was written by AI and reviewed by @mcfnord.

[ann0see]

I think that in general, this belongs into the code. A general high level overview of the threading model might be in some ARCHITECTURE.md file or just in src/README.md as subsection ## Threading

[mcfnord]

That sounds fine. I'd also like the coverage to be about half as long. (Saying almost as much in half the space is something LLMs do exceptionally well.) An agent would be fine with this detailed explanation, but a human would prefer a quicker explanation that of course leaves some details out. (I guess this would be an example of of "human-first, agent-second" writing.)