Skip to content

Commit

Permalink
fix: ensure streams are closed when protocol negotiation fails (#1236)
Browse files Browse the repository at this point in the history
If an error is thrown during the initial stages of setting up a multiplexed stream, ensure we close the stream to free up any resources associated with it.
  • Loading branch information
achingbrain authored Jun 8, 2022
1 parent 3babbbd commit eee256d
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 0 deletions.
8 changes: 8 additions & 0 deletions src/upgrader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,10 @@ export class DefaultUpgrader extends EventEmitter<UpgraderEvents> implements Upg
})
.catch(err => {
log.error(err)

if (muxedStream.timeline.close == null) {
muxedStream.close()
}
})
},
// Run anytime a stream closes
Expand Down Expand Up @@ -330,6 +334,10 @@ export class DefaultUpgrader extends EventEmitter<UpgraderEvents> implements Upg
} catch (err: any) {
log.error('could not create new stream', err)

if (muxedStream.timeline.close == null) {
muxedStream.close()
}

if (err.code != null) {
throw err
}
Expand Down
23 changes: 23 additions & 0 deletions test/upgrading/upgrader.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,29 @@ describe('Upgrader', () => {
}))
.to.eventually.be.rejected.with.property('code', 'ABORT_ERR')
})

it('should close streams when protocol negotiation fails', async () => {
await remoteComponents.getRegistrar().unhandle('/echo/1.0.0')

const { inbound, outbound } = mockMultiaddrConnPair({ addrs, remotePeer })

const connections = await Promise.all([
localUpgrader.upgradeOutbound(outbound),
remoteUpgrader.upgradeInbound(inbound)
])

expect(connections[0].streams).to.have.lengthOf(0)
expect(connections[1].streams).to.have.lengthOf(0)

await expect(connections[0].newStream('/echo/1.0.0'))
.to.eventually.be.rejected.with.property('code', 'ERR_UNSUPPORTED_PROTOCOL')

// wait for remote to close
await delay(100)

expect(connections[0].streams).to.have.lengthOf(0)
expect(connections[1].streams).to.have.lengthOf(0)
})
})

describe('libp2p.upgrader', () => {
Expand Down

0 comments on commit eee256d

Please sign in to comment.