Skip to content

Atomic bRun in socket#3788

Open
softins wants to merge 2 commits into
jamulussoftware:mainfrom
softins:atomic-brun-in-socket
Open

Atomic bRun in socket#3788
softins wants to merge 2 commits into
jamulussoftware:mainfrom
softins:atomic-brun-in-socket

Conversation

@softins

@softins softins commented Jul 17, 2026

Copy link
Copy Markdown
Member

Short description of changes

Make the bRun boolean in the socket class atomic, for thread safety. Also remove the CSocket::Close method, as it is no longer needed with non-blocking socket reads.

CHANGELOG: Internal: Improve thread safety in socket handling

Context: Fixes an issue?

Replaces #3786 and expands on it, following on from #3774

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

No, bug fix and refactor only

Status of this Pull Request

Ready for review

What is missing until this pull request can be merged?

Checking on all main platforms

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

@softins
softins requested a review from ann0see July 17, 2026 15:27
@softins softins added the bug Something isn't working label Jul 17, 2026
@github-project-automation github-project-automation Bot moved this to Triage in Tracking Jul 17, 2026
@softins softins added this to the Release 4.0.0 milestone Jul 17, 2026
@softins softins changed the title Atomic brun in socket Atomic bRun in socket Jul 17, 2026
@softins softins mentioned this pull request Jul 17, 2026

@ann0see ann0see 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 good but I'll need to test it on ARM and x86.

We'll need to hunt for similar bugs then. Especially in the windows audio code.

Comment thread src/socket.cpp
@pljones pljones moved this from Triage to In Progress in Tracking Jul 17, 2026
@pljones

pljones commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

I'll give the Windows Client and Linux Server a test today.

@ann0see

ann0see commented Jul 17, 2026

Copy link
Copy Markdown
Member

Actually, testing on iOS would also be beneficial

@softins

softins commented Jul 17, 2026

Copy link
Copy Markdown
Member Author

Worked fine on arm64 Pi. I'll test on x86 Mac later.

@softins

softins commented Jul 17, 2026

Copy link
Copy Markdown
Member Author

Seems to behave fine as both server and client on Mac x86 10.15

@ann0see ann0see 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.

I am now back on the initial network where I saw the hang, and it seems good now.
Tested with macOS Apple Silicon

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

Labels

bug Something isn't working

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

3 participants