libp2p doesn't always clean up connections

[dirkmc]

I've created a minimal repo with a test case demonstrating an issue with libp2p cleaning up connections:
https://github.com/dirkmc/simultaneous-connection-test

It seems like when a connection is made between two peers, each dialing the other at the same time, libp2p loses track of some of the connections. Consequently, when one of the peers is shut down, only one half of the connection is closed. This means that any pull stream that is waiting on the end of the connection will never terminate.

Note: I'm not sure if this is an issue with libp2p, libp2p-switch or libp2p-mplex or websocket-star so I created a separate repo in which to test it out.

To try it out, clone the repo above, npm install and then run
DEBUG=conn-test,conn-test:* yarn test:node

Sample output:

[parkan]

@daviddias can you help us route this more appropriately? I think @dirkmc is right to not jump to conclusions as to the exact culprit repository, but I'm concerned that this might get overlooked without direct maintainer attention 😄

[jacobheun]

Thanks for the reproduction case @dirkmc, this is great! This is an issue with libp2p-switch. Currently switch keeps track of connections in a hash map with the peerId as the key, which creates this problem. Dialing out is easy to resolve because we know the peerid in advance and can check the existence of the connection. When we receiving an incoming connection, we don't know the peerid until crypto has completed. Right now, switch isn't properly managing those different connections. On incoming connections once switch knows the peerid it should be checking for an existing connection and then switching over to that. There's more we can be doing with that in the future to determine the best connection and switch over to that.

We need to be keeping track of more than 1 connection per peerid, so we can properly handle this scenario. In the future we can look at improving how we handle multiple connections to a single peer.

I've created https://github.com/libp2p/js-libp2p-switch/issues/288 to track this there since I can't move the issue.

[dirkmc]

Great, thanks @jacobheun!

[jacobheun]

@dirkmc this should be resolved in the latest version of libp2p-switch@0.41.3. I ran it against your test repo with a clean npm install and the tests are passing. Let me know if you see any issues.

[dirkmc]

@jacobheun yes it works, thanks for the quick work!