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

Allow Topics to close any handles they have #237

Open
aschmahmann opened this issue Dec 2, 2019 · 2 comments
Open

Allow Topics to close any handles they have #237

aschmahmann opened this issue Dec 2, 2019 · 2 comments

Comments

@aschmahmann
Copy link
Contributor

@Stebalien mentioned in libp2p/go-libp2p-pubsub-router#37 (comment) that the restrictions posed on closing a Topic handle shouldn't need to exist

// Close closes down the topic. Will return an error unless there are no active event handlers or subscriptions.

IIUC the reasoning is that since only one valid Topic handle can exist for a given topic that when that handler calls Close() it is willing to kill any existing Subscription or TopicEventHandler objects that are outstanding.

This may clean up some code, by removing a bunch of error checking/handling, but comes at the cost of adding complexity for a Topic with N Subscriptions that only wants to close when all N Subscriptions are closed.

Both options are by me. WDYT @vyzo?

@vyzo
Copy link
Collaborator

vyzo commented Dec 2, 2019

Maybe we can simply reference count the closures?

@Stebalien
Copy link
Member

As we decided in #198, topic handles are now owned by a single service. There shouldn't be any need to reference count unless I'm missing something.

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

No branches or pull requests

3 participants