Client: add --connectdirectory for directory-assisted hole punch on --connect#3815
Draft
mcfnord wants to merge 4 commits into
Draft
Client: add --connectdirectory for directory-assisted hole punch on --connect#3815mcfnord wants to merge 4 commits into
mcfnord wants to merge 4 commits into
Conversation
This is an extract from jamulussoftware#2550 Co-authored-by: ann0see <20726856+ann0see@users.noreply.github.com>
Introduce EConnectionState (disconnected / connecting / connected) owned by CClient as the single source of truth. A connection is 'requested' when the audio stream starts (CS_CONNECTING) and 'established' once the server assigns our channel ID (CS_CONNECTED). Every transition emits ConnectionStateChanged. Rename the Connected(name) signal emitted from Start() to Connecting(name), since at that point the connection is only requested, not established. CClient::Connect() now terminates any current connection first, so connecting while connected behaves as a reconnect. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
5 tasks
- SIGTERM/SIGINT: route through Disconnect() instead of a raw Stop(), and guard Disconnect() on the connection state rather than IsRunning() (which tracks the audio device). IsRunning() is false while connecting and in headless mode, so the old guard could skip notifying the server on shutdown; the raw Stop() worked around that but also fired a spurious disconnect when idle. Now the server is notified iff a connection is pending or established, via the single Disconnect() path. - Move EConnectionState from util.h to client.h (it is client specific). - Add a doc-comment to SetConnectionState; reword two member comments. Addresses review feedback on jamulussoftware#3805. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
-c / --connect connects straight to the given address and, unlike selecting a server from the GUI directory list, never asks a directory to open the target's firewall. Servers behind a cloud firewall/NAT that the GUI reaches fine are therefore silently unreachable via -c. Give CClient::Connect an optional directory argument: when set it sends a CLM_REQ_SERVER_LIST to that directory (which makes the directory poke its registered servers towards our socket -- the same UDP hole punch the GUI relies on), waits HOLE_PUNCH_CONNECT_DELAY_MS, then connects. The server address is used verbatim and need not be listed by the directory. Expose it on the command line as --connectdirectory <host:port>, wired through both the GUI and headless connect-on-startup paths in main.cpp. Relates to jamulussoftware/jamuluswebsite#1122 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
mcfnord
force-pushed
the
connect-directory-holepunch
branch
from
July 21, 2026 06:01
1339edc to
1e9a6b3
Compare
Member
|
Best case |
Contributor
Author
The client would need to know which directory to use, and that will have to be specified by the CLI parameters. It would be too sloppy and nuts to guess all 7, and even if we do, it might be some other directory entirely. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Short description of changes
Adds an optional
--connectdirectory <host:port>client option and givesCClient::Connectan optional directory argument. When a directory is supplied, the client first asks that directory for its server list — which makes the directory tell its registered servers to send an "empty message" toward the client's socket (the same directory-assisted UDP hole punch the GUI directory list relies on) — waitsHOLE_PUNCH_CONNECT_DELAY_MS, then connects.Today
-c/--connectconnects straight to the address and never contacts a directory, so a server behind a cloud firewall/NAT that the GUI reaches fine is silently unreachable via-c. The server address is connected to verbatim and need not appear in the directory's server list, so this also lets you reach a server that isn't in any built-in directory by naming the (possibly custom/private) directory it is registered with.CHANGELOG: Client: Added
--connectdirectoryto hole-punch through a directory when connecting on startup with--connectContext: Fixes an issue?
Relates to jamulussoftware/jamuluswebsite#1122, which documents this
-creachability limitation. This is the code side of that gap.Stacked on #3805 — it uses the
CClient::Connectrefactor introduced there, so the diff currently includes #3805's commit; the single top commit is the change in this PR. Please review/merge #3805 first.Does this change need documentation? What needs to be documented and how?
Yes — a short website entry for
--connectdirectorynext to the-ctext (a companion to the-cnote added in jamuluswebsite#1156). A draft on the website repo will follow.Status of this Pull Request
Working implementation. Verified with a UDP capture: with
--connectdirectorythe client sendsCLM_REQ_SERVER_LISTto the directory and connects ~400 ms later; without it there is no directory traffic and behaviour is unchanged. Clean SIGTERM shutdown, including when interrupted during the hole-punch delay.What is missing until this pull request can be merged?
Depends on #3805 merging first. Otherwise ready for review.
Checklist