Skip to content

Fix nil dereference in awaitResponse when the connection closes - #21

Merged
dank merged 1 commit into
dank:masterfrom
ABowlOfEleven:fix/await-response-closed-channel
Sep 17, 2026
Merged

dank merged 1 commit into
dank:masterfrom
ABowlOfEleven:fix/await-response-closed-channel

Conversation

@ABowlOfEleven

Copy link
Copy Markdown
Contributor

Fixes #20.

Problem

Close() closes the response channel of every pending request. A receive on a closed channel yields nil, and awaitResponse read response.Error from it with no check. Any request that was still waiting when the connection closed (explicit Close(), a read error such as PsyNet's DuplicateLogin drop, or the pong timeout) panicked the calling goroutine.

Change

  • awaitResponse checks the receive. If the channel was closed with no response, it returns ctx.Err() when the context has ended, and the new ErrConnectionClosed otherwise. The ctx.Err() branch covers the cleanup goroutine in sendRequestAsync, which closes the same channel on ctx.Done().
  • ErrConnectionClosed is exported so that callers can use errors.Is to tell "reconnect and retry" apart from a PsyNet error.
  • No other behavior changes. A request that gets its response, a PsyNet error, or a context timeout returns exactly what it did before.

Tests

Both new tests panic on master and pass with the fix:

  • TestPsyNetRPC_CloseWithPendingRequest: waits until the request is in pendingReqs, calls rpc.Close(), expects ErrConnectionClosed.
  • TestPsyNetRPC_ServerDropsConnectionWithPendingRequest: the mock server closes the connection when the request arrives, so Close() runs from readMessages. Expects ErrConnectionClosed and IsConnected() == false. This needed one small option on MockWSServer (SetCloseOnRequest).

Locally (Go 1.26.8, Windows): gofmt -l . is empty, go vet ./... is clean, the two new tests pass with -count=20, and the full suite passes with -count=3. I could not run -race on this machine (no C compiler for cgo), so that part is left to CI.

Close() closes the response channel of every pending request. A receive
on a closed channel yields nil, and awaitResponse read response.Error
from it without a check, so any request that was still waiting when the
connection closed panicked the calling goroutine. Close() runs on an
explicit call, on a read error (for example when PsyNet drops the
socket on DuplicateLogin) and on a pong timeout.

awaitResponse now returns ctx.Err() if the context ended, or the new
ErrConnectionClosed otherwise. Add two tests that panic without the fix:
Close() with a pending request, and the server dropping the connection.
@dank

dank commented Sep 17, 2026

Copy link
Copy Markdown
Owner

Nice, thanks!

@dank
dank merged commit 33c50cd into dank:master Sep 17, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Nil pointer dereference in awaitResponse when the connection closes with a request pending

2 participants