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.

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

Drain channel on unsubscribe-all #1

Merged
merged 3 commits into from
May 11, 2022
Merged

Drain channel on unsubscribe-all #1

merged 3 commits into from
May 11, 2022

Conversation

Stebalien
Copy link

And:

  • Make this into a go module.
  • Enable tests.
  • Fix tests.

@Stebalien
Copy link
Author

This makes it easier to use this module without accidentally deadlocking. It's not perfect, but it's better than the current state of things.

Stebalien added a commit to filecoin-project/lotus that referenced this pull request May 10, 2022
@Stebalien
fix: use unsub-all when unsubscribing from pubsub
799402a 
When unsubscribing all topics, pubsub will drain the subscription
channel to avoid deadlocks. See
whyrusleeping/pubsub#1.

fixes #7803
Stebalien added a commit to filecoin-project/lotus that referenced this pull request May 10, 2022
@Stebalien
fix: mpool: avoid deadlock on unsubscribe
f5af563 
When unsubscribing _all_ topics, pubsub will drain the subscription
channel to avoid deadlocks. See
whyrusleeping/pubsub#1.

fixes #7803
@Stebalien Stebalien mentioned this pull request May 10, 2022
Closed
5 tasks
whyrusleeping
whyrusleeping approved these changes May 10, 2022
View reviewed changes
Copy link
Owner

@whyrusleeping whyrusleeping left a comment

Choose a reason for hiding this comment

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

Lgtm, seems like we might be covering up some bad usage patterns with this, but less footguns is good

@Stebalien
Copy link
Author

Lgtm, seems like we might be covering up some bad usage patterns with this, but less footguns is good

Well, the only other way to do this is to spawn a goroutine to unsubscribe while we continue to read from the channel.

@arajasek arajasek mentioned this pull request May 10, 2022
Merged
arajasek pushed a commit to filecoin-project/lotus that referenced this pull request May 10, 2022
@Stebalien @arajasek
fix: mpool: avoid deadlock on unsubscribe
9da58c5 
When unsubscribing _all_ topics, pubsub will drain the subscription
channel to avoid deadlocks. See
whyrusleeping/pubsub#1.

fixes #7803
@arajasek arajasek mentioned this pull request May 10, 2022
Merged
5 tasks
@whyrusleeping whyrusleeping merged commit cb4b00a into whyrusleeping:master May 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@whyrusleeping whyrusleeping whyrusleeping approved these changes

@arajasek arajasek arajasek approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Stebalien @whyrusleeping @arajasek