Skip to content
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.

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

don't use a context for closing the ObservedAddrManager #1175

Merged
merged 3 commits into from
Sep 7, 2021

Conversation

marten-seemann
Copy link
Contributor

This is part of the ongoing effort to stop using contexts to control shutdowns.
Most importantly, this PR removes a dependency on the swarm's Process in

hostClosing := oas.host.Network().Process().Closing()
.

@marten-seemann marten-seemann changed the title don't use a context for closing the ObservedAddressManager don't use a context for closing the ObservedAddrManager Sep 5, 2021
p2p/protocol/identify/obsaddr.go Outdated Show resolved Hide resolved

oas.mu.Lock()
oas.refreshTimer.Stop()
oas.refreshTimer = nil
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't work as the worker might be blocked on the timer channel, creating a race condition.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eh, we could have a "shutdown" boolean. That or have the worker keep a copy of the timer and/or channel locally instead of reading oas.refreshTimer.

@marten-seemann marten-seemann merged commit 0797df7 into master Sep 7, 2021
@marten-seemann marten-seemann deleted the id-service-shutdown branch September 17, 2021 16:25
@aschmahmann aschmahmann mentioned this pull request Dec 1, 2021
80 tasks
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.

2 participants