fix: emiting of peer:disconnect when a connection is still open (#301)

[nikor]

This should fix #301

[jacobheun]

Thanks for submitting a PR for this! Can you add tests to verify the fix?

[nikor]

Okay. I messed up. There is only a problem if there is two separate muxed connection to the same node (for instance two tcp connections with spdy). And that will only happen under the very specific test conditions I had.
Sorry about that.
The good news is that no patch is needed :)

[jacobheun]

Parallel dials will cause this as well, for example these new tests on switch: https://github.com/libp2p/js-libp2p-switch/pull/298/files#diff-063533a50438d54e519fdd9f9161bc82.

I think my main concern here is that the peer will always be in the peerBook, so we'll never fire the the disconnect event. I think we may need to actually check for connections with the PeerId via https://github.com/libp2p/js-libp2p-switch/blob/v0.41.4/src/connection/manager.js#L62.

[nikor]

The more i look at the connections management the more confused i get :)

From what i can see, the connections object (https://github.com/libp2p/js-libp2p-switch/blob/v0.41.4/src/connection/manager.js#L22) only contains the real connection that are being used for multiplexing.
So if libp2p is running without muxing, the connections object is always empty and hangup does not work (https://github.com/nikor/js-libp2p-test/blob/master/test1.js). Is that expected behavior?

It also seems like the connection is not removed from the object if the other end closes it (https://github.com/nikor/js-libp2p-test/blob/master/test2.js). But i think that is a seperate issue.

But about the current problem. I can only get libp2p to emit 'peer:disconnect' if i do a hangup, but that is expected behavior. How do i close only inbound or outbound connections?

[jacobheun]

The more i look at the connections management the more confused i get :)

Yeah, the connection logic is pretty confusing. It's getting better, but still has a ways to go.

So if libp2p is running without muxing, the connections object is always empty and hangup does not work (https://github.com/nikor/js-libp2p-test/blob/master/test1.js). Is that expected behavior?

No, hangup should work, it's just not in there yet as the focus has been primarily on muxed connections because they're a more efficient use of connections. We do need to do this though, to avoid leaking connections.

But about the current problem. I can only get libp2p to emit 'peer:disconnect' if i do a hangup, but that is expected behavior. How do i close only inbound or outbound connections?

That is being done here, libp2p/js-libp2p-switch#298. The work is done, but there is a bug in the libp2p tests I haven't been able to look at yet.

[jacobheun]

The connection manager in the switch now keeps track of the connections and will only emit when all connections to that peer have been removed, https://github.com/libp2p/js-libp2p-switch/blob/v0.42.11/src/connection/manager.js#L86.