Modernize gfile: Go 1.26, pion/webrtc v4, framed protocol#12
Merged
Conversation
Closed
- Bump Go 1.12 → 1.26.2; migrate pion/webrtc v2 → v4
- Replace logrus with zerolog, urfave/cli with cobra/v1.10
- Introduce framed protocol (PROTOCOL.md) under internal/protocol with control/data frames and versioning
- Restructure into internal/ (session, stream, stats, output, protocol, utils) and pkg/transfer/{sender,receiver}
- Add klauspost/compress streaming compression; bandwidth tracking and bench harness
- Swap Travis+Makefile for GitHub Actions (ci, release) + Justfile + golangci-lint config
- Drop unused _client/web wasm experiment and legacy pkg/session, pkg/stats, pkg/utils trees
- Expand test coverage (protocol, session, transfer, stats, base45, sdp) and add Python e2e/bench scripts
4c72754 to
e3cd8a6
Compare
The three sites launching EmitProgressSamples only canceled its context on exit without waiting for the goroutine to stop, so it could still be reading output.Default after the caller returned. Tests swapping the emitter via output.SetDefault had no happens-before edge with that lingering read, triggering the race detector on Windows CI. Add StartProgressEmitter which returns a stop that cancels and blocks on goroutine exit, and migrate all three call sites.
The primary data channel's onFramesErr unconditionally called core.fail on any OnFrames exit error. When the sender closed first, pion's SCTP layer could surface teardown with "sending reset packet in non-established state" from rwc.Read instead of io.EOF. core.fail then raced verifyAndClose's doneOnce — if fail won, the receiver returned a spurious "protocol error" despite the transfer having succeeded, which flaked TestSendReceiveCompressed on CI. The multi-connection control channel had the same class of bug: after TRANSFER_COMPLETE, the sender's deferred PeerConnection.Close surfaces on the receiver as an SCTP "User Initiated Abort", and the control channel's onFramesErr logged it as ERR even though the transfer had succeeded. Gate both callbacks on a post-completion flag (eofSeen on single, completeSeen on multi) so post-completion transport teardown is silently ignored and verifyAndClose finalises the transfer.
5f00eff to
efebfed
Compare
Closed
--stun is now a comma-separated list with the Google STUN default applied
at the CLI layer. Empty ("") disables STUN so transfers fall back to
host/mDNS candidates, which is enough on a LAN and avoids a public reflexive
round-trip when none is needed.
Mirror browser behavior: ICE host candidates are emitted as `.local` hostnames instead of raw LAN IPs, so an SDP pasted between peers doesn't leak internal network topology and a LAN transfer works without STUN. Toggle with --mdns=false when the other peer can't resolve mDNS. The `bench` command and the in-process sendrecv test keep mDNS off for determinism.
Plumb a test-only ICELite flag through IOConfig → Session → multiRouter so loopback tests can short-circuit ICE connectivity checks. The CLI never sets it; production transfers leave it false. Also fix a race in receiver negotiateReceivePeer: the peer was published into r.peers before sess.CreateConnection had finished writing sess's internal state, so a concurrent cleanup() Close could race with that write. Now we build sess fully first, then publish, and dedupe sess.Close via sync.Once so either cleanup() or the failure-path defer may run.
This was referenced Apr 20, 2026
Closed
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
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.
Summary
This PR is a large overhaul of gfile after a long maintenance gap. The goal is to bring the project back to life on a modern Go / pion stack, give it a real wire protocol, and make same-host / cross-host transfers meaningfully faster.
PROTOCOL.md,internal/protocol): framed control/data channels with versioning, proper dispatch, and streaming zstd compression (off by default for already-compressed data).--connections N(1..16) stripes the transfer across N parallel peer connections, working around the per-datachannel throughput ceiling on high-bandwidth / high-latency links. See PROTOCOL.md — Multi-PC mode.internal/{session,stream,stats,output,protocol,utils}andpkg/transfer/{sender,receiver}. The old_client/webwasm experiment and the legacypkg/session,pkg/stats,pkg/utilstrees are removed.ci.yml,release.yml) +Justfile+golangci-lint. Expanded Go test coverage plus Pythonscripts/e2e.pyandscripts/bench.pyharnesses.Benchmarks
Methodology: same host (darwin 24.6, arm64, Apple Silicon), pion/webrtc, incompressible random payload. Master's
gfile benchis a 20-second synthetic throughput test over a single data channel with a 64 kB buffer-low threshold and 4 kB writes. This PR'sgfile benchis a real 500 MB file transfer through the fullpkg/transferpipeline. 3–5 runs per config, median reported.master(v2 webrtc, 20s synthetic)--loopback--connections 2,--loopback--connections 4,--loopback¹
--connections 4over loopback on this macOS version (Darwin 24.6) intermittently fails peer-open withabort: peer N open timeout— roughly 3/5 attempts succeed. The failure is environmental (ICE/DC open racing on loopback), not a protocol regression; successful runs are stable at the number above. Cross-host real-network runs don't exhibit this.Note
This PR was drafted with the help of Claude Code. The design decisions, review, and merge call are mine; flagging it here given this project has a few stars and has been mostly unmaintained for the last few years — want to be upfront about the assist.