unbounded: keep consumer Source on close events (fix orphaned globe arcs + runaway helped counter)#512
Open
myleshorton wants to merge 1 commit into
Conversation
broflake's OnConnectionChange delivers the consumer addr on accept but a nil addr on close (the WebRTC session is already torn down, so the remote IP is gone). The close ConnectionEvent therefore carried an empty Source, and downstream consumers couldn't pair it with its accept: the Flutter globe matches the arc to remove by source IP, and the "people helped" counter decrements the same way. So closes were dropped — arcs orphaned and accumulated, and the live counter only ever grew (it equalled the lifetime total). Track each consumer slot's addr on accept (connSources, keyed by broflake's workerIdx — stable across a connection's accept->close) and restore it on close, so every -1 carries the same Source its +1 did. No event-shape change; downstream consumers already key off Source. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes Unbounded connection close events emitting an empty Source (when broflake provides nil addr on close), which prevents downstream consumers from matching close events to prior accepts—leading to orphaned globe arcs and a “People being helped right now” counter that never decrements.
Changes:
- Add
connSourcesto cache per-consumer-slot (workerIdx) source IPs on accept and restore them on close. - Update the broflake connection-change callback to backfill
Sourceon close events. - Add/adjust unit tests to pin the accept→close
Sourcebackfill behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| unbounded/unbounded.go | Introduces connSources and uses it to backfill Source on close events. |
| unbounded/unbounded_test.go | Updates the bridge test to assert close events preserve the accept’s Source. |
| unbounded/conn_sources_test.go | Adds unit coverage for connSources.resolve behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+94
to
+97
| case state > 0: | ||
| if addrStr != "" { | ||
| c.addrs[workerIdx] = addrStr | ||
| } |
Comment on lines
+50
to
+55
| // Slot reuse: broflake recycles a workerIdx; a fresh accept overwrites | ||
| // the prior addr even without an intervening close. | ||
| c.resolve(1, 12, "2.2.2.2") | ||
| c.resolve(1, 12, "3.3.3.3") | ||
| assert.Equal(t, "3.3.3.3", c.resolve(-1, 12, ""), | ||
| "reused slot restores the latest accept's addr") |
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.
Problem
The Unbounded donor tab's "People being helped right now" counter runs away (a tester saw 131 while the pill still said "Waiting for connections…"), and it equals "Total people helped to date" — the live count only ever grows. The globe also accumulates arcs that never disappear.
Root cause is here in
unbounded.go. broflake'sOnConnectionChangeFunchands us the consumer'saddron accept but a niladdron close (the WebRTC session is already torn down, so the remote IP is gone). We emitConnectionEvent{Source: addr.String()}, so a close goes out withSource == "".Downstream, both consumers of the event identify a connection by its Source IP:
So an empty-Source close can't be matched — the arc orphans and the counter never comes back down. broflake does fire the close (it has to, to recycle its small consumer table), so these aren't missing events; they just arrive un-attributable. (The 131 magnitude was amplified by a separate consumer-routing flood, throttled on the lantern-cloud side.)
Fix
Track each consumer slot's addr on accept and restore it on close, so every
-1carries the sameSourceits+1did:connSources— a smallworkerIdx → addrmap (broflake'sworkerIdxis the consumer slot, stable across a single connection's accept→close), concurrency-safe since broflake fires callbacks from per-worker goroutines.OnConnectionChangeFunccallssources.resolve(state, workerIdx, addrStr)— records on accept, restores on close.No change to the
ConnectionEventshape or to any consumer; they already key offSource. An accept that genuinely has no addr stays empty through its close (neither end is counted), which is the correct behavior when broflake can't surface the consumer IP at all.Tests
TestConnSources_resolve— unit-tests the backfill: accept stores, nil-addr close restores, freed slot restores nothing, slots independent, empty-addr accept stays empty, real-addr close passes through, slot reuse.TestConnectionEventBridge— updated to assert the installed callback now emits the accept's IP on the close (was asserting the old empty-Source bug). Passes with-race.