Skip to content

Stop the inbound accept loop from spamming errors on shutdown - #575

Open
Bornoz wants to merge 1 commit into
canopy-network:mainfrom
Bornoz:fix/p2p-quiet-accept-loop-on-shutdown
Open

Stop the inbound accept loop from spamming errors on shutdown#575
Bornoz wants to merge 1 commit into
canopy-network:mainfrom
Bornoz:fix/p2p-quiet-accept-loop-on-shutdown

Conversation

@Bornoz

@Bornoz Bornoz commented Aug 31, 2026

Copy link
Copy Markdown

On shutdown the inbound listener is closed and listener.Accept() returns use of closed network connection. The accept loop logged that as ERROR, waited 5s, and retried forever, so a clean shutdown produced a wall of identical ERROR lines (exactly the output in #230).

The package already defines ErrListenerClosed for this string. When Accept fails with a closed listener, log a single debug line and return to end the loop; every other error keeps the existing back-off-and-retry path.

go build ./p2p/ and gofmt are clean, and go vet ./p2p/ reports nothing new on the changed file (the two pre-existing set_test.go warnings are unrelated).

Closes #230

When the node shuts down, the inbound listener is closed and listener.Accept() returns "use of closed network connection". The accept loop logged that as an ERROR, waited 5 seconds, and tried again — forever — so a clean shutdown produced a wall of identical ERROR lines that do not indicate anything wrong.

The p2p package already has a constant for this case (ErrListenerClosed). Use it: when Accept fails with a closed listener, log a single debug line and return, ending the loop. Every other Accept error keeps the existing back-off-and-retry behaviour.

Closes canopy-network#230
@ygd58

ygd58 commented Sep 4, 2026

Copy link
Copy Markdown

Looks like this overlaps with #521, which I opened earlier for the same root cause (#230) — flagging for whoever reviews these, not asking for either to be closed.

One technical difference worth noting: ErrListenerClosed here is a string constant matched via strings.Contains(err.Error(), ...), which predates net.ErrClosed (added in Go 1.16 specifically to replace string-matching this exact condition). errors.Is(err, net.ErrClosed) correctly traverses wrapped errors regardless of message text, so it's a bit more robust against future error-wrapping changes upstream (in netutil.LimitListener or Go's net package itself) than matching on message text — though both work correctly against the current Accept() behavior.

#521 also covers the other half of #491/#230: cmd/rpc.Server.updatePollResults panicking on a store-closed-during-shutdown race (via store.NewSMT()), plus test coverage for the accept-loop exit behavior specifically (asserting the goroutine actually returns instead of hanging, using a real net.Listener).

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.

[BUG] Make software kill and startup logs more graceful

2 participants