Hacktoberfest 2026: những issue maintainer đã đánh dấu cho tháng Mười, đang mở và phù hợp người mới. Xem issue Hacktoberfest

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

Đang mở
  • Có gì không ổn?
#3,933 2 bình luận 0 reaction 0 người được giao Xem trên GitHub

Maintainer thường phản hồi trong vòng 3 ngày

Chưa có ai nhận issue này.

Đánh giá

Độ khó
3/5
Thời gian dự kiến
1-2 ngày
Mức phù hợp với người mới
76/100
Loại issue
Tài liệu
Độ rõ ràng
Đặc tả rõ ràng
Mức độ hoạt động
Sôi nổi
Công nghệ
cpp
Lĩnh vực
backend, documentation

Hướng nghiên cứu

Bắt đầu với docs/THREADING.md được đề xuất và kiểm tra mô hình của tài liệu này đối chiếu với src/socket.cpp, src/server.cpp, src/channel.h, cùng các tham chiếu đã dẫn đến channel.cpp và Qt tại commit 0545fddd. Xác nhận quyền sở hữu thread và phạm vi bao phủ của lock đã được ghi lại, sau đó thêm tài liệu đã thống nhất mà không thay đổi hành vi; công việc được coi là hoàn tất khi mô hình threading và locking của CChannel phía server được ghi lại chính xác và phạm vi kiểm chứng của nó rõ ràng.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Mô tả

🤖 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 #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.

Ngôn ngữ chính
C
Star
1.1k
Fork
248
Merge trung bình
2 ngày 22 giờ
Pull request đã merge (30 ngày)
6

Chuẩn bị môi trường

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của jamulussoftware/jamulus

Tất cả issue của jamulussoftware/jamulus

Issue tương tự

Thêm issue về C

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.