basecamp / basecamp/basecamp-sdk

Event feed: staleness is armed after Observer.Connected, leaving a socket-open state with no staleness window

Open
#763 0 comments 0 reactions 0 assignees View on GitHub
bug go spec
Dominant language
Go
Stars
49
Forks
12
Avg merge
20h 47m
Merged PRs (30d)
89

Description

Raised by Codex on #705 (`loop.go`, the `Observer.Connected` call site).

The dial returns, `Observer.Connected` fires, and only afterwards does
`newLiveConn` create the frame pump and arm the `staleness` timer. So for the
duration of that callback the socket is **open** while neither its pump nor its
required 7.5s staleness window exists. A callback that blocks between 7.5s and
10s cannot observe a staleness firing on return, and a peer that went silent
immediately after the upgrade stays connected until the 10s handshake deadline
mops it up.

Two reasons this is a spec question rather than a one-line reorder.

**It contradicts the socket-open timer contract as written.** §23's timer table
says `staleness` is armed "at socket open", and the per-state exact-timer-set
invariants list `staleness` in every socket-open state. Between the dial
returning and `newLiveConn`, the connector is in a socket-open state with an
empty staleness slot. Either the arming moves before the callback, or §23 says
the window opens at pump start rather than at socket open — and the second is a
real option, since "socket open" and "the connector is reading" genuinely
differ.

**The reorder is fixture-visible.** Moving the arming before the callback
changes the outstanding-timer set at the instant `Connected` fires, and the
tier-2 fixtures assert exact timer sets. Any scenario that observes timers
around `connected` would need re-blessing, across six SDKs rather than in the
Go reference alone.

Severity, stated honestly: this is **bounded**, unlike the hangs in #758 and
#760. The handshake deadline still fires at 10s, so the worst case is a dead
socket surviving ~2.5s longer than the staleness policy intends, and it needs a
consumer whose `Connected` callback blocks for 7.5s or more. That is a real
misconfiguration but not a stall, which is why it is filed rather than patched
into #705 alongside the unbounded ones.

Related: #759 is the same callback-blocking surface from the preemption angle
(what runs *after* a callback that called `Close`); this one is about what has
not been *set up yet* when the callback runs. Both would be simplified by a
decision about what a consumer callback is allowed to assume about connector
state, and they may want deciding together.

Follows #606, #614, #645, #696; siblings #753, #758, #759, #760, #761, #762.

Contributor guide

Open the contributing guide

Research direction

Start at loop.go and the Observer.Connected call site, then read §23's socket-open timer contract and the tier-2 fixture expectations around connected. Resolve whether staleness begins at socket open or pump start, document the decision, and update the affected exact-timer fixtures across the SDKs.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
networking
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.