Skip to content

fix(bitswap/httpnet): idempotent Stop() #920

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
May 20, 2025
Merged

fix(bitswap/httpnet): idempotent Stop() #920

merged 1 commit into from
May 20, 2025

Conversation

lidel
Copy link
Member

@lidel lidel commented May 20, 2025

This PR aims to avoid closing channel twice (inside of of stopCleaner) on shutdown.

Found in Kubo 0.35.0-rc2 (ipfs/kubo#10760), shutdown of ipfs daemon sometimes produces noisy panic (not a big deal, since this is shutdown, but unnecessary noise):

panic: close of closed channel

goroutine 16518 [running]:
github.com/ipfs/boxo/bitswap/network/httpnet.(*cooldownTracker).stopCleaner(...)
	github.com/ipfs/boxo@v0.30.0/bitswap/network/httpnet/cooldown.go:50
github.com/ipfs/boxo/bitswap/network/httpnet.(*Network).Stop(0xc0014caa50)
	github.com/ipfs/boxo@v0.30.0/bitswap/network/httpnet/httpnet.go:322 +0x2e
github.com/ipfs/boxo/bitswap/network.(*router).Stop(0xc0014fbb00)
	github.com/ipfs/boxo@v0.30.0/bitswap/network/router.go:55 +0x35
github.com/ipfs/boxo/bitswap.(*Bitswap).Close(0xc0014fe100)
	github.com/ipfs/boxo@v0.30.0/bitswap/bitswap.go:152 +0x22
github.com/ipfs/kubo/core/node.Online.ProvidingExchange.func4.1({0x2edbc40?, 0x39f9d20?})
	github.com/ipfs/kubo/core/node/bitswap.go:188 +0x1c
go.uber.org/fx/internal/lifecycle.(*Lifecycle).runStopHook(0xc000000460, {0x3a11130, 0x4f74900}, {0x0, 0xc0014941f8, {0x0, 0x0}, {0x0, 0x0}, {{0xc001580340, ...}, ...}})
	go.uber.org/fx@v1.23.0/internal/lifecycle/lifecycle.go:341 +0x1f2
go.uber.org/fx/internal/lifecycle.(*Lifecycle).Stop(0xc000000460, {0x3a11130, 0x4f74900})
	go.uber.org/fx@v1.23.0/internal/lifecycle/lifecycle.go:303 +0x52e
go.uber.org/fx.(*App).Stop.func2({0x3a11130?, 0x4f74900?})
	go.uber.org/fx@v1.23.0/app.go:725 +0x7a
go.uber.org/fx.withTimeout.func1()
	go.uber.org/fx@v1.23.0/app.go:803 +0x6b
created by go.uber.org/fx.withTimeout in goroutine 42
	go.uber.org/fx@v1.23.0/app.go:791 +0xc5

cc @hsanjuan for visibility, if there is a better way

@lidel
fix(httpnet): idempotent Stop()
bc6882a 
avoid closing channel twice
@lidel lidel requested a review from a team as a code owner May 20, 2025 19:10
@hsanjuan
Copy link
Contributor

LGTM, but we close the network in two places then, which is probably unintended..

@lidel
Copy link
Member Author

lidel commented May 20, 2025

@hsanjuan could it be these two places? (once as bitswap, once as exchange)

I think its fine, my expectation for Close/Stop is to be idempotent anyway.

@lidel lidel merged commit 2f225c4 into main May 20, 2025
15 checks passed
@lidel lidel deleted the fix-httpnet-close-once branch May 20, 2025 22:52
@lidel lidel mentioned this pull request May 21, 2025
Closed
46 tasks
@BrewTestBot BrewTestBot mentioned this pull request Jul 14, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hsanjuan hsanjuan hsanjuan approved these changes

Assignees
No one assigned
Labels
skip/changelog
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lidel @hsanjuan