Proposal: Closable Transport

[Jorropo]

Use Case

While writing a tor transport I noticed that some transports runs some node beyond the listener (mainly overlay networks).
For example my tor transport runs a tor node and I need to close it at one point. I can't rely on the listeners to close it because it's not because a node stop listening on an address it will not then dial someone.

Additions

I propose to add this in go-libp2p-core/transport :

type ClosableTransport interface {
  Transport

  // Close is called when the libp2p node doesn't need the transport anymore (the is node closing).
  // After that the transport can safely assume no more Listen() or Dial() are gonna be done.
  // This is usefull for transport running overlay network.
  Close() error
}

Then in host.Close() the host would use reflection to terminate the transport when closing (after closing all listeners) :

// Not actual working code, just an idea
var wg sync.WaitGroup
for _, v := range h.transports {
  ct, ok := v.(tpt.ClosableTransport)
  if ok {
    wg.Add(1)
    go func (t tpt.ClosableTransport){ // Needed because range is not thread safe
      defer wg.Done()
      t.Close()
    }(ct)
  }
}
wg.Wait()

(Also I can't rely on the garbage collector to do this work because I have some CGO code (or a system fork) and the garbage collector doesn't deal with that.)

In the Future

If in the future a method to unhook transport from a libp2p is added the transport will be closable that way.

[aschmahmann]

Looks like this is related to #992. WDYT @Stebalien @vyzo @raulk

[Stebalien]

It seems reasonable, but I'm not sure if we even need an interface change for this. We can probably just add a io.Closer. See libp2p/go-libp2p-swarm#227.

[vyzo]

I don't think we need an interface change for this; we can soft check if the transport implements io.Closer and just call it.

[Jorropo]

@vyzo @Stebalien yes thx, I didn't even though of that, that certainly prettier.
With a small comment aside of go-libp2p-core/transport#Transport this would be perfect (this shouldn't stay an hidded feature it must be in the doc somewhere).

[Jorropo]

Unexpected close (I was merging this in my repo for some mod replace rules and this somehow closed this).
I guess Closes #xxx works cross repo.

[marten-seemann]

Should the libp2p-quic-transport implement this?

[Jorropo]

Should the libp2p-quic-transport implement this?

@marten-seemann
That up to the transport, for quic yes it must :),
That would solve the issue of quic not closing the shared socket when libp2p close (#992).