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

Error: Multiplexer is destroyed` on the server when libp2p circuit is doing something while the client crashes just after connecting #171

Closed
mkg20001 opened this issue Feb 27, 2018 · 14 comments
Assignees
Labels
exp/wizard Extensive knowledge (implications, ramifications) required help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws) P1 High: Likely tackled by core team if no one steps up

Comments

@mkg20001
Copy link
Member

  • Version: v18
  • Platform: Linux - 4.4.0-116-generic #140-Ubuntu SMP Mon Feb 12 21:23:04 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux, node -v: v8.9.4
  • Subsystem: muxer

Type: bug

Severity: high

Description: Error: Multiplexer is destroyed on the server when libp2p floodsub is doing something while the client crashes just after connecting

Steps to reproduce the error:

Run a server

Run a client

Connect the client to the server and dial a protocol

The server should send some data and the client should throw when it receives it.

The server will now crash with Error: Multiplexer is destroyed

(Once again discovered this while writing libp2p-nodetrust - btw, here is the server-log.txt)

Note that this error does not occur 100% of the time.

@mkg20001
Copy link
Member Author

Edit: Turns out circuit and not floodsub is causing this: at Dialer.canHop (/home/maciej/Projekte/libp2p/libp2p-nodetrust/server/node_modules/libp2p-circuit/src/circuit/dialer.js:81:7)

@mkg20001 mkg20001 changed the title Error: Multiplexer is destroyed` on the server when libp2p floodsub is doing something while the client crashes just after connecting Error: Multiplexer is destroyed` on the server when libp2p circuit is doing something while the client crashes just after connecting Feb 27, 2018
@daviddias
Copy link
Member

@dryajov ^^

@dryajov
Copy link
Member

dryajov commented Feb 27, 2018

@mkg20001 It seems to happen when circuit is trying to send the canHop message to the other peer, that happens to be the first thing that circuit will do when another peer just connected, but I don't see much going on besides that from the attached log.

One thing that comes to mind is, are you running on the same machine or are your nodes on different machines, if so, it might be a connection drop. Could you run this with debug='libp2p*' please?

@mkg20001
Copy link
Member Author

mkg20001 commented Feb 27, 2018

@dryajov This was already run with DEBUG=libp2p*. What I was saying is that the server is crashing when the connection gets dropped due to the other peer crashing. I'm reporting this issue because this could easily be abused to crash important nodes maliciously.

@mkg20001
Copy link
Member Author

Edit: Also note that in the log the error below "this is the actual crash" is server side and not on the client (the client was supposed to crash after receiving the servers response)

@dryajov
Copy link
Member

dryajov commented Feb 27, 2018

Got you, thanks for clarifying.

Does something similar happen when circuit is disabled? I think canHop just happens to be the next thing that happens in the sequence of events? We should definitely handle muxer errors more graciously.

@mkg20001
Copy link
Member Author

Yes. This time it took 2 runs to crash the server.

server-logs.txt

@dryajov
Copy link
Member

dryajov commented Feb 27, 2018

Ah, this is it -

/home/maciej/Projekte/libp2p/libp2p-nodetrust/server/node_modules/libp2p-mplex/src/internals/index.js:95
      throw new Error('Multiplexer is destroyed')

We're mixing callbacks and throws....

@dryajov
Copy link
Member

dryajov commented Feb 27, 2018

Here is the line - https://github.com/libp2p/js-libp2p-mplex/blob/bfc6b113e0026618cb72899b39d162eddeb42b3f/src/muxer.js#L54

We're never handling the throw anywhere.

@dryajov
Copy link
Member

dryajov commented Feb 27, 2018

I think this should help - https://github.com/libp2p/js-libp2p-mplex/pull/75/files

Note that there are other throws, but they don't seem to be used anywhere. Specifically in -https://github.com/libp2p/js-libp2p-mplex/blob/bfc6b113e0026618cb72899b39d162eddeb42b3f/src/internals/index.js#L106 which gets called by https://github.com/libp2p/js-libp2p-mplex/blob/bfc6b113e0026618cb72899b39d162eddeb42b3f/src/internals/index.js#L136, which doesn't get used anywhere.

@dryajov
Copy link
Member

dryajov commented Feb 28, 2018

Here is the required change in libp2p-switch libp2p/js-libp2p-switch#245

@daviddias daviddias added kind/bug A bug in existing code (including security flaws) P1 High: Likely tackled by core team if no one steps up labels Mar 2, 2018
@daviddias daviddias added status/ready Ready to be worked help wanted Seeking public contribution on this issue exp/wizard Extensive knowledge (implications, ramifications) required labels Mar 12, 2018
@dryajov dryajov self-assigned this Mar 23, 2018
@dryajov dryajov added status/in-progress In progress and removed status/ready Ready to be worked labels Mar 23, 2018
@daviddias
Copy link
Member

@jacobheun was this handled by your other PRs?

@jacobheun
Copy link
Contributor

@diasdavid this was ultimately resolved by libp2p/js-libp2p-mplex#79, which catches the error and returns it via the callback. This should be closable. This fix is available in libp2p-mplex 0.8.0.

@daviddias
Copy link
Member

RAD! :D Thanks for double checking :)

@ghost ghost removed the status/in-progress In progress label Jul 5, 2018
maschad pushed a commit to maschad/js-libp2p that referenced this issue Jun 21, 2023
)

Bumps [@libp2p/interface-peer-store](https://github.com/libp2p/js-libp2p-interfaces) from 1.2.9 to 2.0.0.
- [Release notes](https://github.com/libp2p/js-libp2p-interfaces/releases)
- [Commits](https://github.com/libp2p/js-libp2p-interfaces/compare/@libp2p/interface-peer-store-v1.2.9...@libp2p/interface-peer-store-v2.0.0)

---
updated-dependencies:
- dependency-name: "@libp2p/interface-peer-store"
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: achingbrain <alex@achingbrain.net>
maschad pushed a commit to maschad/js-libp2p that referenced this issue Jun 21, 2023
## [7.0.0](libp2p/js-libp2p-bootstrap@v6.0.3...v7.0.0) (2023-04-24)

### ⚠ BREAKING CHANGES

* bump @libp2p/interface-peer-store from 1.2.9 to 2.0.0 (libp2p#171)

### Dependencies

* bump @libp2p/interface-peer-store from 1.2.9 to 2.0.0 ([libp2p#171](libp2p/js-libp2p-bootstrap#171)) ([9f36bb2](libp2p/js-libp2p-bootstrap@9f36bb2))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exp/wizard Extensive knowledge (implications, ramifications) required help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws) P1 High: Likely tackled by core team if no one steps up
Projects
None yet
Development

No branches or pull requests

4 participants