Skip to content

Commit

Permalink
fix(@libp2p/tcp): race condition in onSocket
Browse files Browse the repository at this point in the history
This fixes a race condition in the onSocket listener method.
onSocket gets a net.Socket parameter that needs to be closed later
before the tcp server can close.
The net.Socket is used to create a MultiaddrConnection but the
connection is not tracked with this.connections until after it has been
upgraded.
If the tcp listener.close is called while a the MultiaddrConnection is waiting
to be upgraded, then the MultiaddrConnection socket cannot be closed as
it does not exist in this.connections.

Instead of adding the MultiaddrConnection to this.connections on
creation I decided to create a separate property named
this.maConnections. I decided to track the non-upgraded connections
separately because I was unsure how this.connections must be used with other things
like metrics.
  • Loading branch information
tabcat committed Oct 11, 2024
1 parent 1cbfd6c commit c441230
Showing 1 changed file with 8 additions and 0 deletions.
8 changes: 8 additions & 0 deletions packages/transport-tcp/src/listener.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ export class TCPListener extends TypedEventEmitter<ListenerEvents> implements Li
private readonly server: net.Server
/** Keep track of open connections to destroy in case of timeout */
private readonly connections = new Set<MultiaddrConnection>()
private readonly maConnections = new Set<MultiaddrConnection>()
private status: Status = { code: TCPListenerStatusCode.INACTIVE }
private metrics?: TCPListenerMetrics
private addr: string
Expand Down Expand Up @@ -195,11 +196,13 @@ export class TCPListener extends TypedEventEmitter<ListenerEvents> implements Li
}

this.log('new inbound connection %s', maConn.remoteAddr)
this.maConnections.add(maConn)

this.context.upgrader.upgradeInbound(maConn)
.then((conn) => {
this.log('inbound connection upgraded %s', maConn.remoteAddr)
this.connections.add(maConn)
this.maConnections.delete(maConn)

socket.once('close', () => {
this.connections.delete(maConn)
Expand Down Expand Up @@ -307,6 +310,11 @@ export class TCPListener extends TypedEventEmitter<ListenerEvents> implements Li
conn.abort(err)
})

// cleanup connections that have not been upgraded
this.maConnections.forEach(conn => {
conn.abort(err)
})

// shut down the server socket, permanently
await this.pause(true)
}
Expand Down

0 comments on commit c441230

Please sign in to comment.