synchronously closing one side of the connection breaks libp2p

[mkg20001]

• Version: libp2p v18

• Platform: server: Linux - 4.4.0-97-generic #120-Ubuntu SMP Tue Sep 19 17:28:18 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux, client: Linux - 4.13.0-36-generic #40-Ubuntu SMP Fri Feb 16 20:07:48 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux, node (same on both): v8.9.4

• Subsystem: transport, muxer

Type: bug

Severity: high

Description: synchronously closing one side of the connection breaks libp2p

Steps to reproduce the error: https://github.com/mkg20001/libp2p-nodetrust

Install and setup the server and client from the fix/update branch at commit 2b34ab0eeacc410840a9daa76a9afa80a8a6ffa8

server-log.txt
client-log.txt

The fix is to close one side of the connection asynchronously. This patch fixes it (but this is just a workaround):

diff --git a/server/src/proto/index.js b/server/src/proto/index.js
index 2a34925..739ecc6 100644
--- a/server/src/proto/index.js
+++ b/server/src/proto/index.js
@@ -28,8 +28,6 @@ class RPC {
     this.sink = this.sink.bind(this)
   }
   sink (read) {
-    read(true, () => {}) // we will never read from the client
-
     const cb = (err, res) => {
       if (err) {
         log(err)
@@ -39,6 +37,8 @@ class RPC {
       }
 
       this.source.end()
+
+      read(true, () => {}) // we will never read from the client
     }
     const ips = this.addr.map(a => a.toString()).filter(a => a.startsWith('/ip')).map(a => a.split('/')[2]) // TODO: filter unique
     const domains = ips.map(ip => encodeAddr(ip)).filter(Boolean).map(sub => sub + '.' + this.opt.zone)

I don't know if this is a bug from my module but I think that closing one side of the connection in sync shouldn't close the other. Or the whole muxer connection.

[mkg20001]

Note: both mplex and spdy show this behaviour

[daviddias]

@mkg20001 can you add a test that reproduces this? I'm not sure if I understand the issue. Sounds like you are just not catching the errors and they go uncaught.

[mkg20001]

@diasdavid https://github.com/mkg20001/libp2p-reprd-test-case

[daviddias]

@mkg20001 can you convert that into a PR with a test for this module?

[mkg20001]

@diasdavid I tried to implement the test case but the error does not seem to occur. Either this is fixed already or it's because both peers are running in the same process.

[dryajov]

This seems to be related to the multiplexer throwing that is being worked on in - https://github.com/libp2p/js-libp2p-mplex/pull/75/files.

[vasco-santos]

This should not be a problem anymore