Skip to content

A replacement request can close the connection despite respecting the concurrency limit #2

Description

@foundev

Problem

Connection.handle releases its worker slot only in a defer after c.send returns. The peer can receive the completed response and submit its replacement before the writer reports completion. readLoop still sees all 32 slots occupied and closes the entire connection.

Reproduction

Connect a net.Pipe peer to a handler and send requests with numeric IDs 0 through 31. Hold requests 1 through 31 until cancellation. Let request 0 return, decode its response at the peer, then immediately send request 32. For deterministic scheduling, wrap the connection's output stream so Write transfers the response bytes but delays returning until Close. At replacement submission, the peer has only 31 unanswered requests. The added investigation_repro_test.go implements this sequence.

Expected behavior

The replacement starts, keeping the number of unanswered requests within the documented limit of 32.

Actual behavior

The connection closes with too many simultaneous ACP requests and cancels the remaining requests.

Affected source

connection.go

Evidence

Ran env GOCACHE=/tmp/acp-go-bugscan-cache go test -race . -run '^TestInvestigationReplacementAfterDeliveredResponse$' -count=1 -timeout=10s -v. It failed with: valid replacement closed the connection: too many simultaneous ACP requests.

The ordinary net.Pipe reproduction also failed without the delayed writer: TestBugscanReplacementAtConcurrencyLimit reported iteration 5: replacement after response rejected with only 31 active handlers: too many simultaneous ACP requests.

Connection.handle removes the inbound request ID before sending its response, but retains the workers token through c.send. readLoop rejects a full workers channel immediately.

Independent review

Independently inspected commit eeaeb28. connection.go:281 retains the worker slot through send at line 307; readLoop closes the connection when all slots remain occupied (251–255). Both reproductions failed under -race with 'too many simultaneous ACP requests': the deterministic test confirmed response 0 was received before replacement submission, and ordinary net.Pipe reproduced at iteration 4 with only 31 active handlers. Existing connection tests passed. This validates a connection-closing race within the documented 32-request limit. No issues or comments were supplied for comparison.

Investigated at commit eeaeb28967219eaa95a28b82655ce618d503bb88 by bug-bot.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions