Skip to content

tests: fix stopping race in http client harness#6566

Merged
algorandskiy merged 2 commits intoalgorand:masterfrom
algorandskiy:pavel/p2p-fix-http-client-tests
Feb 25, 2026
Merged

tests: fix stopping race in http client harness#6566
algorandskiy merged 2 commits intoalgorand:masterfrom
algorandskiy:pavel/p2p-fix-http-client-tests

Conversation

@algorandskiy
Copy link
Copy Markdown
Contributor

Summary

Fix for p2p-related failures as seen in this build

There is a race in libp2phttp.go

	select {
	case <-h.httpTransport.closeListeners:
	case err = <-errCh:
		expectedErrCount--
	}

h.httpTransport.closeListeners is closd in httphost.Close() and errCh gets it error from Serve loop.
Because notifyAll is now async, host.Close() finishes somitimes faster so that errCh gets a chance to be ready before h.httpTransport.closeListeners

The fix is very the same as WebsocketNetwork.httpdThread - is to handle http.ErrServerClosed explicitly

Test Plan

This is a test fix

There is a race in libp2phttp.go

```
	select {
	case <-h.httpTransport.closeListeners:
	case err = <-errCh:
		expectedErrCount--
	}
```
h.httpTransport.closeListeners is closd in httphost.Close()
and `errCh` gets it error from Serve loop.
Because notifyAll is now async, host.Close() finishes somitimes faster
so that errCh gets a chance to be ready before `h.httpTransport.closeListeners`

The fix is very the same as WebsocketNetwork.httpdThread - is to handle
`http.ErrServerClosed` explicitly
Comment thread network/p2p/testing/httpNode.go Outdated
gmalouf
gmalouf previously approved these changes Feb 24, 2026
@algorandskiy algorandskiy requested a review from cce February 24, 2026 20:52
@codecov
Copy link
Copy Markdown

codecov bot commented Feb 24, 2026

Codecov Report

❌ Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 47.68%. Comparing base (791c5ae) to head (b8f7fa7).
⚠️ Report is 3 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
network/p2p/testing/httpNode.go 0.00% 2 Missing ⚠️
network/p2pNetwork.go 0.00% 0 Missing and 2 partials ⚠️

❗ There is a different number of reports uploaded between BASE (791c5ae) and HEAD (b8f7fa7). Click for more details.

HEAD has 4 uploads less than BASE
Flag BASE (791c5ae) HEAD (b8f7fa7)
full_coverage 4 0
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #6566       +/-   ##
===========================================
- Coverage   63.66%   47.68%   -15.99%     
===========================================
  Files         484      645      +161     
  Lines       67688    87972    +20284     
===========================================
- Hits        43095    41946     -1149     
- Misses      21077    43264    +22187     
+ Partials     3516     2762      -754     
Flag Coverage Δ
full_coverage ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@algorandskiy algorandskiy merged commit c197379 into algorand:master Feb 25, 2026
39 checks passed
@algorandskiy algorandskiy deleted the pavel/p2p-fix-http-client-tests branch March 16, 2026 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants