contexts: always call returned cancelFunc

[cryptix]

Hi,

according to this upstream change, we can't ignore the returned cancelFunc of a context otherweise cleanup of goroutines blocking on ctx.Done() can't happend.

I found at one such case in routing/dht/records.go and started a branch (dontIgnoreCancelFuncs) with a fix but wanted to wait with a PR until we are sure there arn't more easy cases lying around.

[whyrusleeping]

Is this the case even if the parent context cancels? The only time ive thrown away cancel funcs is if im confident my parent will cancel for me

[jbenet]

that's a bit error prone-- if others change the code above you it will break things. probably safer to cancel always.

[jbenet]

• https://github.com/jbenet/go-ipfs/pull/860/files#r25924773

[cryptix]

The returned context's Done channel is closed when the returned cancel function is called or when the parent context's Done channel is closed, whichever happens first. - godoc

so @whyrusleeping's assumptions holds but I agree with @jbenet that it is error prone. If the parent changes to a non-canceling one for instance.

[cryptix]

pushed fix for the ones in PR860 that @jbenet linked to. (df4d510)

here is a list of outstanding ones

[cryptix@higgs ~/go/src/github.com/jbenet/go-ipfs] grep -R '_ := context.' | grep -v Godeps | grep -v _test.go  | sort
core/commands/id.go:            ctx, _ := context.WithTimeout(context.TODO(), time.Second*5)
core/commands/ping.go:                  ctx, _ := context.WithTimeout(ctx, kPingTimeout)
core/commands/ping.go:                  ctx, _ := context.WithTimeout(ctx, kPingTimeout)
core/corenet/net.go:    ctx, _ := context.WithTimeout(nd.Context(), time.Second*30)
diagnostics/diag.go:    ctx, _ := context.WithTimeout(context.TODO(), timeout)
exchange/bitswap/bitswap.go:                    child, _ := context.WithTimeout(ctx, providerRequestTimeout)
exchange/bitswap/bitswap.go:            hasBlockCtx, _ := context.WithTimeout(ctx, hasBlockTimeout)
merkledag/merkledag.go: ctx, _ := context.WithTimeout(context.TODO(), time.Minute)
namesys/publisher.go:   timectx, _ := context.WithDeadline(ctx, time.Now().Add(time.Second*10))
p2p/net/swarm/swarm_dial.go:            ctxT, _ := context.WithTimeout(ctx, s.dialT)
pin/pin.go:     ctx, _ := context.WithTimeout(context.Background(), time.Second*60)
routing/dht/dht.go:                             ctx, _ := context.WithTimeout(dht.Context(), time.Second*5)
unixfs/tar/reader.go:           ctx, _ := context.WithTimeout(context.TODO(), time.Second*60)

[jbenet]

awesome. might try grepping just _ := context.

[cryptix]

agreed and updated :)

rebased btw

[jbenet]

@cryptix which PR?

[cryptix]

it's still a branch only - I guess I can open one now.

[cryptix]

closing in favor of #878.