Skip to content

[release/3_12] Backport of #3800 - #3853

Merged
pljones merged 2 commits into
release/3_12from
r3_12-mcfnord_atomic-flags-3798-ab
Aug 2, 2026
Merged

[release/3_12] Backport of #3800#3853
pljones merged 2 commits into
release/3_12from
r3_12-mcfnord_atomic-flags-3798-ab

Conversation

@pljones

@pljones pljones commented Aug 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Short description of changes

Backport of #3800

CHANGELOG: SKIP

Context: Fixes an issue?

git cherry-pick needed a small, manual patch on the first commit, to add in bEnableIPv6, which has been removed on main.

Does this change need documentation? What needs to be documented and how?

N/A

Status of this Pull Request

Needs @mcfnord and @softins to check.

What is missing until this pull request can be merged?

...getting all the targets clean would be enough, I hope...
https://github.com/jamulussoftware/jamulus/actions/runs/30757002725 ✅
(manually triggered as release/* doesn't work as a trigger for builds on jamulussoftware/jamulus)

Checklist

  • I've verified that this Pull Request follows the general code principles
  • I tested my code and it does what I want
  • My code follows the style guide
  • I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
  • I've filled all the content above

AUTOBUILD: Please build all targets

jrd and others added 2 commits August 2, 2026 17:30
jrd @claude @pljones
Make cross-thread run/status flags std::atomic
58f2c70 
The socket, high-precision timer and sound threads are stopped by a
plain bool written from another thread, and the jitter-buffer status
flags are written and read/reset from different threads. Convert these
six flags to std::atomic<bool>:

- CSocketThread::bRun
- CHighPrecisionTimer::bRun (Mac/Linux variant)
- CSoundBase::bRun
- CSoundBase::bCallbackEntered
- CSocket::bJitterBufferOK
- CClient::bJitterBufferOK

The read-and-reset functions now use exchange() so a "not OK" written
between a separate read and reset can no longer be lost.

Addresses batches A and B of #3798.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
jrd @claude @pljones
Drop CSocketThread::bRun conversion, superseded by #3788
20697a7 
softins already makes this member atomic in #3788 (which also removes
CSocket::Close in favour of non-blocking reads). Keep only the
bJitterBufferOK conversion in this file; the <atomic> include moves to
match #3788 so the two branches merge cleanly in either order.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@pljones
pljones requested a review from softins August 2, 2026 16:34
@pljones pljones self-assigned this Aug 2, 2026
@pljones pljones added this to the Release 3.12.4 milestone Aug 2, 2026
@pljones pljones added this to Tracking Aug 2, 2026
@github-project-automation github-project-automation Bot moved this to Triage in Tracking Aug 2, 2026
softins
softins approved these changes Aug 2, 2026
View reviewed changes

@softins softins left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine to me. Compared, compiled and smoke-tested.

@github-project-automation github-project-automation Bot moved this from Triage to Waiting on Team in Tracking Aug 2, 2026
@pljones
pljones merged commit 982504a into release/3_12 Aug 2, 2026
12 checks passed
@github-project-automation github-project-automation Bot moved this from Waiting on Team to Done in Tracking Aug 2, 2026
@mcfnord

mcfnord commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

MY LLM WROTE:

20697a7e drops the CSocketThread::bRun conversion as superseded by #3788 — but #3788 is not on this branch.

git merge-base --is-ancestor d3ac7e12 main          -> yes
git merge-base --is-ancestor d3ac7e12 release/3_12  -> no

main socket.h:238 has std::atomic<bool> bRun;. release/3_12 socket.h:212 still has bool bRun; — Stop() writes it, run()'s while ( bRun ) reads it on the socket thread. That is the case this backport exists to fix.

Either backport #3788 as well, or restore that one line here. #3788 also changed OnDataReceived to take std::atomic<bool>&, but this branch's loop calls OnDataReceived() with no argument, so the type change alone covers it. Not compiled.

@pljones
pljones deleted the r3_12-mcfnord_atomic-flags-3798-ab branch August 2, 2026 17:43
@softins

softins commented Aug 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

MY LLM WROTE:

20697a7e drops the CSocketThread::bRun conversion as superseded by #3788 — but #3788 is not on this branch.

git merge-base --is-ancestor d3ac7e12 main          -> yes
git merge-base --is-ancestor d3ac7e12 release/3_12  -> no

main socket.h:238 has std::atomic<bool> bRun;. release/3_12 socket.h:212 still has bool bRun; — Stop() writes it, run()'s while ( bRun ) reads it on the socket thread. That is the case this backport exists to fix.

Either backport #3788 as well, or restore that one line here. #3788 also changed OnDataReceived to take std::atomic<bool>&, but this branch's loop calls OnDataReceived() with no argument, so the type change alone covers it. Not compiled.

Yes, missed that. #3788 is dependent on the change to two separate sockets for IPv4 and IPv6, which is not in 3.12.x. That would have slipped through the compile and smoke-test.

So the change of CSocketThread::bRun to be atomic needs specifically to be here.

In fact, you just need to revert 20697a7

@pljones

pljones commented Aug 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

OK, thanks, I'll get the revert pushed.

https://github.com/jamulussoftware/jamulus/actions/runs/30760042638

dc55ccc3 2026-08-02 (HEAD -> release/3_12, origin/release/3_12) Revert "Drop CSocketThread::bRun conversion, superseded by #3788" [Peter L Jones]

@pljones

pljones commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator Author

Current backport view for the project is here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@softins softins softins approved these changes

Assignees

@pljones pljones

Labels

None yet

Projects

Tracking
Status: Done

Milestone

Release 3.12.4

Development

Successfully merging this pull request may close these issues.

3 participants

@pljones @mcfnord @softins