Rework the opening of UDP sockets for IPv4 and IPv6 by softins · Pull Request #3893 · jamulussoftware/jamulus

softins · 2026-08-12T14:15:40Z

Short description of changes

Reworks the socket opening logic introduced in #3774, which had potential shortcomings identified by AI.
Not for backporting, as #3774 was not also.

CHANGELOG: Client/Server: reworked and improved UDP socket opening logic

Context: Fixes an issue?

Fixes #3885

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

No, bug fixes only

Status of this Pull Request

Ready for review

What is missing until this pull request can be merged?

Checking that it satisfies CoPilot AI shortcomings that were flagged in #3885 for socket.cpp

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 · 2026-08-12T14:20:05Z

            // faulty router gets stuck and confused by a particular port (like
            // the starting port). Might work around frustrating "cannot connect"
            // problems (#568)
-            const quint16 startingPortNumber = iPortNumber + rand() % NUM_SOCKET_PORTS_TO_TRY;


Since this was the first call to rand() after program startup, and rand() always starts from the same seed, this was not providing any randomness at all. The same start port was chosen on each invocation, so this wouldn't actually address #568 as claimed. Verified on Linux and Mac.

pljones · 2026-08-12T17:29:41Z

    if ( !bSuccess )
    {
        // we cannot bind socket, throw error
        throw CGenErr ( "Cannot bind the socket (maybe "


Might be worth expanding this to explain more about what failed as there are more possible causes now. (Not due to this PR -- I've just read it to get more context and realised it's a bit weak.)

Actually, bSuccess only now applies to IPv4. If we fail to create and bind IPv4, bSuccess will be false and we throw the exception. If we succeed with IPv4 but fail with IPv6, it's just logged with a message, but bSuccess is still true. So there isn't really any more info to pass to the exception apart from what is already there.

🤖 AI: Yes, and one of the causes is new. Four distinct paths reach this exact sentence at 05b41038: EADDRINUSE (port already held), EADDRNOTAVAIL (--serverbindip4 naming an address not on the host), EACCES (server on port 80, unprivileged), and one where no bind was attempted at all: startingPortNumber is iPortNumber plus up to 99, so a client --port from 65437 up can start past 65535 and the new guard breaks on the first pass — 25 of 25 runs at --port 65535 made zero bind calls and printed this message.

So the errno is the information the text is missing. The fourth case is a separate matter: resetting the start to iPortNumber when it would overflow binds 65535 in 25 of 25 runs.

softins · 2026-08-13T11:49:04Z

I'd like to get this in before tagging beta3

softins · 2026-09-01T17:23:40Z

I'd like to get this in before tagging beta3

Ok, so missed that, despite it being functional and working. :(

pljones · 2026-09-01T18:03:31Z

We'll keep having betas until we're ready for a release candidate. I don't like having 3.12.x releases with bug fixes without a tagged release with those same bug fixes: it's hard to keep track of. In terms of bugs, 4.0.0beta3 = 3.12.5. In terms of features 3.12.0 = 3.12.5.

softins · 2026-09-01T18:41:58Z

We'll keep having betas until we're ready for a release candidate. I don't like having 3.12.x releases with bug fixes without a tagged release with those same bug fixes: it's hard to keep track of. In terms of bugs, 4.0.0beta3 = 3.12.5. In terms of features 3.12.0 = 3.12.5.

Ah, so you're saying 4.0.0beta3 was just tagged to correspond with the bug fixes that went into 3.12.5? And this PR just missed out because it's not a candidate for backporting?

Anyway, hope we can get it in soon. I'm about to resolve hopefully your remaining unresolved comment.

This corrects logic errors for bind() failure handling found by AI. bIPv6Available is now only set if bind() succeeds for IPv6.

Previously, rand() was used and is always seeded with 1, providing no randomness at program start.

pljones · 2026-09-01T19:31:12Z

Looks very smooth now.

ann0see · 2026-09-04T12:30:20Z

Will try to look at this later.

I'll let @coderabbitai review this first. I think it's more concise than Claude usually.

coderabbitai · 2026-09-04T12:30:33Z

@ann0see I will review the UDP socket opening changes in #3893.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai · 2026-09-04T12:30:48Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: QUIET

Plan: Advanced

Run ID: c5c2a415-439e-4ebf-b0d0-5424cbbb38c0

📥 Commits

Reviewing files that changed from the base of the PR and between c1ceb10 and f58e606.

📒 Files selected for processing (2)

src/socket.cpp
src/socket.h

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

Walkthrough

CSocket now treats IPv4 binding as the primary setup condition. Client port retries use randomized bounded ranges. IPv6 binding remains independent for both clients and servers.

Changes

Socket binding behavior

Layer / File(s)	Summary
Client socket initialization and port selection `src/socket.cpp`, `src/socket.h`	Client initialization resets IPv6 availability, binds IPv4 before IPv6, and uses randomized `quint32` port retries with overflow protection.
Server socket binding `src/socket.cpp`	Server setup retains successful IPv4 binding when IPv6 binding fails, then closes and disables the IPv6 socket.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to f58e6

UDP socket initialization now preserves IPv4 operation when IPv6 binding is unavailable and reports IPv6 capability independently. No concrete current-head merge risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: reworking UDP socket opening for IPv4 and IPv6.
Description check	✅ Passed	The description follows the template, explains the change, links issue `#3885`, states documentation needs, records status and remaining work, and includes a completed checklist.
Linked Issues check	✅ Passed	The changes address the socket.cpp portion of issue `#3885` by correcting IPv4 and IPv6 binding, retry, availability, and failure-handling logic. The issue also mentions buffer.cpp, but the PR objective…
Out of Scope Changes check	✅ Passed	The changes are limited to UDP socket initialization and binding behavior in src/socket.cpp and the required QRandomGenerator include in src/socket.h. These changes align with the linked issue and PR …

Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands.}

ann0see · 2026-09-04T12:34:27Z

Docstring Coverage

Not too important

softins self-assigned this Aug 12, 2026

softins added the bug Something isn't working label Aug 12, 2026

github-project-automation Bot added this to Tracking Aug 12, 2026

github-project-automation Bot moved this to Triage in Tracking Aug 12, 2026

softins added this to the Release 4.0.0 milestone Aug 12, 2026

softins commented Aug 12, 2026

View reviewed changes

Comment thread src/socket.cpp Outdated

softins force-pushed the socket-opening-correction branch from c56534e to 0d7b521 Compare August 12, 2026 14:52

pljones reviewed Aug 12, 2026

View reviewed changes

softins changed the title ~~Reword the opening of UDP sockets for IPv4 and IPv6~~ Rework the opening of UDP sockets for IPv4 and IPv6 Aug 12, 2026

softins force-pushed the socket-opening-correction branch from 0d7b521 to 05b4103 Compare August 13, 2026 11:50

ann0see moved this from Triage to In Progress in Tracking Aug 13, 2026

softins force-pushed the socket-opening-correction branch 2 times, most recently from 22ee1f4 to fb75de3 Compare August 21, 2026 19:06

pljones reviewed Aug 22, 2026

View reviewed changes

Comment thread src/socket.cpp

pljones reviewed Aug 22, 2026

View reviewed changes

Comment thread src/socket.cpp Outdated

pljones reviewed Aug 22, 2026

View reviewed changes

Comment thread src/socket.cpp Outdated

pljones reviewed Aug 22, 2026

View reviewed changes

Comment thread src/socket.cpp Outdated

mcfnord mentioned this pull request Aug 29, 2026

Add IPv6 support for Directories (still incomplete) #3809

Draft

5 tasks

softins added 5 commits September 1, 2026 19:53

Rework the binding of IPv4 and IPv6 sockets

98cfd88

This corrects logic errors for bind() failure handling found by AI. bIPv6Available is now only set if bind() succeeds for IPv6.

Use Qt random number generator for starting port in client

8e9b534

Previously, rand() was used and is always seeded with 1, providing no randomness at program start.

Clear bIPv6Available after closing sockets in Init()

304b6a6

Ensure binding is tried at least once, if high port given

0ac8c3a

Address review comment - no functional change

f58e606

pljones approved these changes Sep 1, 2026

View reviewed changes

softins force-pushed the socket-opening-correction branch from e703dd4 to f58e606 Compare September 1, 2026 19:35

softins requested review from ann0see and dingodoppelt September 1, 2026 19:36

Uh oh!

Conversation

softins commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

softins Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pljones Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

softins Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

mcfnord Aug 19, 2026

Choose a reason for hiding this comment

Uh oh!

softins commented Aug 13, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

softins commented Sep 1, 2026

Uh oh!

pljones commented Sep 1, 2026

Uh oh!

softins commented Sep 1, 2026

Uh oh!

pljones commented Sep 1, 2026

Uh oh!

ann0see commented Sep 4, 2026

Uh oh!

coderabbitai Bot commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

ann0see commented Sep 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

softins commented Aug 12, 2026 •

edited

Loading

pljones Aug 12, 2026 •

edited

Loading

coderabbitai Bot commented Sep 4, 2026 •

edited

Loading

coderabbitai Bot commented Sep 4, 2026 •

edited

Loading