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

basic_host: ensure we close correctly when the context is canceled #656

Merged
merged 1 commit into from
Jun 13, 2019
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 7 additions & 12 deletions p2p/host/basic/basic_host.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,6 @@ type BasicHost struct {

proc goprocess.Process

ctx context.Context
cancel func()
mx sync.Mutex
lastAddrs []ma.Multiaddr
}
Expand Down Expand Up @@ -121,22 +119,21 @@ type HostOpts struct {

// NewHost constructs a new *BasicHost and activates it by attaching its stream and connection handlers to the given inet.Network.
func NewHost(ctx context.Context, net network.Network, opts *HostOpts) (*BasicHost, error) {
bgctx, cancel := context.WithCancel(ctx)

h := &BasicHost{
network: net,
mux: msmux.NewMultistreamMuxer(),
negtimeout: DefaultNegotiationTimeout,
AddrsFactory: DefaultAddrsFactory,
maResolver: madns.DefaultResolver,
ctx: bgctx,
cancel: cancel,
}

h.proc = goprocessctx.WithContextAndTeardown(ctx, func() error {
if h.natmgr != nil {
h.natmgr.Close()
}
if h.cmgr != nil {
h.cmgr.Close()
}
return h.Network().Close()
})

Expand All @@ -148,7 +145,7 @@ func NewHost(ctx context.Context, net network.Network, opts *HostOpts) (*BasicHo
h.ids = opts.IdentifyService
} else {
// we can't set this as a default above because it depends on the *BasicHost.
h.ids = identify.NewIDService(bgctx, h)
h.ids = identify.NewIDService(goprocessctx.WithProcessClosing(ctx, h.proc), h)
}

if uint64(opts.NegotiationTimeout) != 0 {
Expand Down Expand Up @@ -223,7 +220,7 @@ func New(net network.Network, opts ...interface{}) *BasicHost {

// Start starts background tasks in the host
func (h *BasicHost) Start() {
go h.background()
h.proc.Go(h.background)
}

// newConnHandler is the remote-opened conn handler for inet.Network
Expand Down Expand Up @@ -300,7 +297,7 @@ func (h *BasicHost) PushIdentify() {
}
}

func (h *BasicHost) background() {
func (h *BasicHost) background(p goprocess.Process) {
// periodically schedules an IdentifyPush to update our peers for changes
// in our address set (if needed)
ticker := time.NewTicker(1 * time.Minute)
Expand All @@ -318,7 +315,7 @@ func (h *BasicHost) background() {
case <-ticker.C:
h.PushIdentify()

case <-h.ctx.Done():
case <-p.Closing():
return
}
}
Expand Down Expand Up @@ -724,8 +721,6 @@ func (h *BasicHost) AllAddrs() []ma.Multiaddr {

// Close shuts down the Host's services (network, etc).
func (h *BasicHost) Close() error {
h.cancel()
h.cmgr.Close()
return h.proc.Close()
}

Expand Down