fix(transport): use pull.empty for closing #200
Conversation
| @@ -67,7 +68,7 @@ module.exports = function (swarm) { | |||
| log('dial canceled: %s', multiaddr.toString()) | |||
| // clean up already done dials | |||
| if (conn) { | |||
| conn.close() | |||
| pull(pull.empty(), conn) | |||
There was a problem hiding this comment.
This is just a quick patch for tests, it won't close connections as the otherside will be waiting for the multistream handshake to finish.
There was a problem hiding this comment.
What I'm thinking now is:
Connections should have a conn.close that sends a pull.empty() by default or call their parent instance version of conn.close. Transports should overload that method to do a destroy/close, whatever the transport prefers.
When a stream muxer is applied, we need to make sure that conn.close gets overloaded back to the conn.close that defaults to pull.empty, because closing a muxed stream should not destroy the socket not the muxer.
This is similar to what we do with getPeerInfo
|
sounds like a lot of work but the right approach 👍 |
|
I'm down to get this started with #202 and finish it in parallel, right now swarm is in this weird state where it can't be released from master because of conn.close, but it also solves the bug of several types of multiaddr. This is work that users need and also our work in relay. So we have have a 'quick fix' of the pull.empty for now and work on what I proposed above: #200 (comment) |
Ref #195 (comment)