Hacktoberfest 2026: as issues que os mantenedores marcaram para outubro, abertas e boas para iniciantes. Ver issues do Hacktoberfest

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

Aberta
  • Algo errado?
#3,933 2 comentários 0 reações 0 responsáveis Ver no GitHub

Mantenedores costumam responder em até 5 dias

Ninguém assumiu esta issue ainda.

Avaliação

Dificuldade
3/5
Tempo estimado
1-2 dias
Facilidade para iniciantes
76/100
Tipo de issue
Documentação
Clareza
Claramente especificada
Status de atividade
Ativa
Stack de tecnologia
cpp
Domínio
backend, documentation

Direção de pesquisa

Comece pela docs/THREADING.md proposta e verifique seu modelo em relação a src/socket.cpp, src/server.cpp, src/channel.h e às referências citadas a channel.cpp e Qt no commit 0545fddd. Confirme a propriedade documentada das threads e a cobertura dos locks e, em seguida, adicione a documentação acordada sem alterar o comportamento; o trabalho estará concluído quando o modelo de threading e locking de CChannel no servidor estiver registrado com precisão e o escopo de sua verificação estiver claro.

Escrita pelo modelo de indexação a partir do texto da issue.

Descrição

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

Linguagem predominante
C
Estrelas
1.1k
Forks
248
Merge médio
7d 4h
PRs com merge (30d)
5

Preparar o ambiente

Primeiros passos

  1. Leia a issue inteira e depois o guia de contribuição do projeto.
  2. Comente na issue dizendo que vai assumir — evita que duas pessoas façam o mesmo trabalho.
  3. Faça um fork do repositório e trabalhe em uma branch.
  4. Abra um pull request que referencie o número da issue.

Mais de jamulussoftware/jamulus

Todas as issues de jamulussoftware/jamulus

Issues semelhantes

Mais issues de C

Receba novas issues na sua caixa de entrada

Um resumo curto de issues do GitHub para quem está começando.