This repository has been archived by the owner on May 11, 2022. It is now read-only.
remove context from constructor, implement a proper Close method #109
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I wish we didn't have to make this change, but controlling shutdowns with a
context
simply doesn't work in Go: There's no way to know when a function listening onctx.Done()
has actually shut down. All you can hope for is that it eventually does.A reliable way of implementing a clean shutdown is by having a
Close
method, which explicitly blocks until all go routines have finished.Unfortunately, this is an API-breaking change, since we're 1. removing the
ctx
from the constructor, and (logically) we're adding aClose
method that users of this library will now have to call explicitly.The concrete reason why I'm sending this PR now is that this is blocking libp2p/go-libp2p#1168. In specific, we have a few tests there that modify
manet.Private4
. This global variable is accessed by thebackground
loop ingo-libp2p-autonat/autonat.go
Line 183 in 3723bd5