Skip to content
This repository has been archived by the owner on Aug 29, 2023. It is now read-only.

fix: remove abortable-iterator and close socket directly on abort #220

Merged
merged 6 commits into from
Dec 13, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,6 @@
"@libp2p/utils": "^3.0.2",
"@multiformats/mafmt": "^11.0.3",
"@multiformats/multiaddr": "^11.0.0",
"abortable-iterator": "^4.0.2",
"err-code": "^3.0.1",
"stream-to-it": "^0.2.2"
},
Expand Down
9 changes: 8 additions & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ class TCP implements Transport {
async dial (ma: Multiaddr, options: TCPDialOptions): Promise<Connection> {
options.keepAlive = options.keepAlive ?? true

// options.signal destroys the socket before 'connect' event
const socket = await this._connect(ma, options)

// Avoid uncaught errors caused by unstable connections
Expand All @@ -82,13 +83,19 @@ class TCP implements Transport {

const maConn = toMultiaddrConnection(socket, {
remoteAddr: ma,
signal: options.signal,
socketInactivityTimeout: this.opts.outboundSocketInactivityTimeout,
socketCloseTimeout: this.opts.socketCloseTimeout
})

const onAbort = () => socket.end()
dapplion marked this conversation as resolved.
Show resolved Hide resolved
options.signal?.addEventListener('abort', onAbort, { once: true })

log('new outbound connection %s', maConn.remoteAddr)
const conn = await options.upgrader.upgradeOutbound(maConn)
log('outbound connection %s upgraded', maConn.remoteAddr)

options.signal?.removeEventListener('abort', onAbort)

return conn
}

Expand Down
8 changes: 1 addition & 7 deletions src/socket-to-conn.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { abortableSource } from 'abortable-iterator'
import { logger } from '@libp2p/logger'
// @ts-expect-error no types
import toIterable from 'stream-to-it'
Expand All @@ -16,7 +15,6 @@ interface ToConnectionOptions {
listeningAddr?: Multiaddr
remoteAddr?: Multiaddr
localAddr?: Multiaddr
signal?: AbortSignal
socketInactivityTimeout?: number
socketCloseTimeout?: number
}
Expand Down Expand Up @@ -92,10 +90,6 @@ export const toMultiaddrConnection = (socket: Socket, options?: ToConnectionOpti

const maConn: MultiaddrConnection = {
async sink (source) {
if ((options?.signal) != null) {
source = abortableSource(source, options.signal)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any usecase to having to abort the remote socket read source during dial


try {
await sink(source)
} catch (err: any) {
Expand All @@ -112,7 +106,7 @@ export const toMultiaddrConnection = (socket: Socket, options?: ToConnectionOpti
socket.end()
},

source: (options.signal != null) ? abortableSource(source, options.signal) : source,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Socket source will abort when destroying it

source,

// If the remote address was passed, use it - it may have the peer ID encapsulated
remoteAddr,
Expand Down