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

Pubsub appears to be opening multiple streams and not closing them #668

Closed
Stebalien opened this issue Jun 11, 2020 · 7 comments
Closed
Labels
kind/bug A bug in existing code (including security flaws)

Comments

@Stebalien
Copy link
Member

On the preload nodes, I'm noticing ~500 connections to js-libp2p peers, but 22K open streams to these peers, most of them inbound pubsub streams. It looks like js-libp2p-pubsub is opening multiple streams per connection and not closing them.

@Stebalien Stebalien added the kind/bug A bug in existing code (including security flaws) label Jun 11, 2020
@Stebalien
Copy link
Member Author

Hm. This behavior appears to be limited to a few peers. But we should still verify that we're closing streams when done (@jacobheun couldn't find where this was happening).

@vasco-santos
Copy link
Member

I did a small experiment with a test 9f07273
It seems that the stream is being closed after sending messages in the current code for js <-> js with floodsub and gossipsub.

Having js older peers with bidirectional streams talking with js with unidirectional streams could cause issues, but on the preload nodes I still did not understand what might be the issue

@Stebalien
Copy link
Member Author

It looks like it's just a few peers with a ton of streams. I'm not sure what's going on there. Could be an old bug, could be a malicious peer, could be anything.

@vasco-santos
Copy link
Member

@Stebalien do you have any logs or any other information that I can look at?

@jacobheun
Copy link
Contributor

@vasco-santos my best guess is that for some reason we're getting multiple connect calls to a peer, https://github.com/libp2p/js-libp2p-pubsub/blob/v0.5.2/src/index.js#L194-L201, and instead of checking for an existing Peer/stream, we're creating a new one to them.

The particular version of libp2p might be hard to track down but we should make sure that pubsub doesn't create new streams for a peer if it already has one.

@vasco-santos
Copy link
Member

FYI, I created a PR to guarantee that we do not open a new stream if we already have one. However, we should guarantee that the onConnect is not called multiple times.

@jacobheun
Copy link
Contributor

This should be fixed in the latest pubsub.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug A bug in existing code (including security flaws)
Projects
None yet
Development

No branches or pull requests

3 participants