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

Subscription.Cancel() hangs forever if PubSub exited #200

Closed
lukesolo opened this issue Sep 30, 2019 · 4 comments
Closed

Subscription.Cancel() hangs forever if PubSub exited #200

lukesolo opened this issue Sep 30, 2019 · 4 comments

Comments

@lukesolo
Copy link

Function Cancel() blocks forever because no one listens to sub.cancelCh if PubSub has exited.

func (sub *Subscription) Cancel() {
	sub.cancelCh <- sub
}

How to reproduce:

package main

import (
	"context"
	"fmt"
	"time"

	"github.com/libp2p/go-libp2p"
	pubsub "github.com/libp2p/go-libp2p-pubsub"
)

func main() {
	ctx, cancel := context.WithCancel(context.Background())

	host, _ := libp2p.New(ctx)
	gsub, _ := pubsub.NewGossipSub(ctx, host)
	sub, _ := gsub.Subscribe("topic")

	cancel()
	time.Sleep(time.Second)

	fmt.Println("cancelling subscription")
	sub.Cancel() // will hang forever
	fmt.Println("subscription cancelled")
}

I think that Subscription should have the same context as a GossipSub, so that you could check it in the Cancel function

func (sub *Subscription) Cancel() {
	select {
		case sub.cancelCh <- sub:
		case <-sub.ctx.Done():
	}
}
@vyzo
Copy link
Collaborator

vyzo commented Sep 30, 2019

Yes indeed, this is a bug; care for a patch?

@lukesolo
Copy link
Author

@vyzo This is not a critical bug for me (at least now), but I made a pull request with a simple fix, and now I think what to do with the other methods which get context as an argument. Should they check both inner and outer context for cancellation in select statement?

@vyzo
Copy link
Collaborator

vyzo commented Oct 1, 2019

If they are doing an operation with the event loop only, because that's only when they can deadlock on shutdown.

@vyzo
Copy link
Collaborator

vyzo commented Oct 1, 2019

Closing as the problem in the issue has been fixed by #201

@vyzo vyzo closed this as completed Oct 1, 2019
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

2 participants