docs: write down the server's CChannel threading and locking model
维护者通常 3 天内回复
还没有人认领这个 Issue。
评估
- 难度
- 3/5
- 预计耗时
- 1-2 天
- 新手友好度
- 76/100
- Issue 类型
- 文档
- 描述清晰度
- 描述清楚
- 活跃度
- 活跃
- 技术栈
- cpp
- 领域
- backend, documentation
调研方向
从提议的 docs/THREADING.md 开始,并根据 src/socket.cpp、src/server.cpp、src/channel.h,以及 commit 0545fddd 中引用的 channel.cpp 和 Qt 参考资料验证其模型。确认文档中记录的线程所有权和锁覆盖范围,然后在不改变行为的情况下添加已达成一致的文档;完成的标准是,服务器端 CChannel 的 threading 和 locking 模型已被准确记录,并且其验证范围清晰明确。
由索引模型根据 Issue 内容生成。
描述
🤖 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 bySetAddress(its only server-side caller isInitChannel); read lock-free in the mix phase (the level and recorder sends, andPrepAndSendPacket) and inGetConCliParam.CHostAddressis aQHostAddressplus a port, andQHostAddressis reference-counted, so a copy that overlapsoperator=is the same shape as theQStringcopy #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, underMutexSocketBuf) againstUpdate()from the level pass in the mix phase (no lock). Twodoubles.
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.
- 主要语言
- C
- 星标
- 1.1k
- 派生
- 248
- 平均合并
- 2 天 22 小时
- 30 天内合并 PR
- 6
环境准备
- 没有 Dockerfile 或 Docker Compose 文件
- 有 Pull Request 模板
- 阅读贡献指南
从这里开始
- 先读完整个 Issue,再读项目的贡献指南。
- 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
- Fork 仓库,在一个分支上完成修改。
- 提交 Pull Request,并在描述里引用这个 Issue 编号。
jamulussoftware/jamulus 的其他 Issue
-
Move translation checker (and potentially other runners) to ARM runner可能已有人在做 @ann0see 于 21 天前认领。 未关闭
难度 2/5 1-3 小时 新手友好度 75/100
jamulussoftware/jamulus#3953 · 2 条评论 ·
维护者通常 3 天内回复
-
Reused channel slot records the previous occupant's audio可能已有人在做 关联的 PR 仍在进行中或已合并。 未关闭AI bug
难度 2/5 1-3 小时 新手友好度 82/100
jamulussoftware/jamulus#3901 · 4 条评论 · 1 个 reaction ·
维护者通常 3 天内回复
-
AI
难度 2/5 1-3 小时 新手友好度 78/100
jamulussoftware/jamulus#3846 ·
维护者通常 3 天内回复
-
feature request
难度 5/5 一周以上 新手友好度 25/100
jamulussoftware/jamulus#3964 · 3 条评论 ·
维护者通常 3 天内回复
-
难度 3/5 1-2 天 新手友好度 65/100
jamulussoftware/jamulus#3961 ·
维护者通常 3 天内回复
查看 jamulussoftware/jamulus 的全部 Issue
相似的 Issue
-
bug
难度 2/5 1-3 小时 新手友好度 74/100
EchoTools/nevr-runtime#117 · 2 条评论 ·
维护者通常 1 天内回复
-
难度 2/5 1-3 小时 新手友好度 67/100
DarkFlippers/qUnleashed#240 ·
维护者通常 1 天内回复
-
enhancement
难度 2/5 1-3 小时 新手友好度 72/100
维护者通常 1 天内回复
-
难度 1/5 1 小时以内 新手友好度 84/100
AFLplusplus/AFLplusplus#2899 ·
-
难度 2/5 1-3 小时 新手友好度 78/100
HarbourMasters/Shipwright#7320 ·
维护者通常 1 天内回复