Skip to content

Commit b9e32cc

Browse files
achingbrainachingbrain
authored
fix: close handshake datachannel after use (#3076)
Some times the handshake datachannel can remain open which causes a memory leak and stops the process from exiting so explicitly close the channel after the noise handshake has completed. Also removes a redundant `raceSignal` - it's not necessary as we pass the signal in to the `connect` method.
1 parent e4f603f commit b9e32cc

File tree

9 files changed

+265
-210
lines changed

9 files changed

+265
-210
lines changed

‎.github/workflows/main.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,8 @@ jobs:
233233
- uses: libp2p/test-plans/.github/actions/run-transport-interop-test@master
234234
with:
235235
test-filter: js-libp2p-head
236+
# remove after https://github.com/libp2p/rust-libp2p/issues/5986 is fixed
237+
test-ignore: rust-v0.53
236238
extra-versions: ${{ github.workspace }}/interop/node-version.json ${{ github.workspace }}/interop/chromium-version.json ${{ github.workspace }}/interop/firefox-version.json ${{ github.workspace }}/interop/webkit-version.json
237239
s3-cache-bucket: ${{ vars.S3_LIBP2P_BUILD_CACHE_BUCKET_NAME }}
238240
s3-access-key-id: ${{ vars.S3_LIBP2P_BUILD_CACHE_AWS_ACCESS_KEY_ID }}

‎packages/integration-tests/.aegir.js

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,11 @@ export default {
99
test: {
1010
before: async () => {
1111
// use dynamic import because we only want to reference these files during the test run, e.g. after building
12-
const { webSockets } = await import('@libp2p/websockets')
13-
const { mplex } = await import('@libp2p/mplex')
1412
const { noise } = await import('@chainsafe/libp2p-noise')
1513
const { yamux } = await import('@chainsafe/libp2p-yamux')
16-
const { WebSockets } = await import('@multiformats/multiaddr-matcher')
14+
const { WebSockets, WebRTCDirect } = await import('@multiformats/multiaddr-matcher')
15+
const { webSockets } = await import('@libp2p/websockets')
16+
const { mplex } = await import('@libp2p/mplex')
1717
const { createLibp2p } = await import('libp2p')
1818
const { plaintext } = await import('@libp2p/plaintext')
1919
const { circuitRelayServer, circuitRelayTransport } = await import('@libp2p/circuit-relay-v2')
@@ -22,6 +22,7 @@ export default {
2222
const { mockMuxer } = await import('@libp2p/interface-compliance-tests/mocks')
2323
const { ping } = await import('@libp2p/ping')
2424
const { prefixLogger } = await import('@libp2p/logger')
25+
const { webRTCDirect } = await import('@libp2p/webrtc')
2526

2627
const libp2p = await createLibp2p({
2728
logger: prefixLogger('relay'),
@@ -31,12 +32,15 @@ export default {
3132
addresses: {
3233
listen: [
3334
'/ip4/127.0.0.1/tcp/0/ws',
34-
'/ip4/127.0.0.1/tcp/0/ws'
35+
'/ip4/127.0.0.1/tcp/0/ws',
36+
'/ip4/0.0.0.0/udp/0/webrtc-direct',
37+
'/ip4/0.0.0.0/udp/0/webrtc-direct'
3538
]
3639
},
3740
transports: [
3841
circuitRelayTransport(),
39-
webSockets()
42+
webSockets(),
43+
webRTCDirect()
4044
],
4145
streamMuxers: [
4246
yamux(),
@@ -67,6 +71,23 @@ export default {
6771

6872
const goLibp2pRelay = await createGoLibp2pRelay()
6973
const wsAddresses = libp2p.getMultiaddrs().filter(ma => WebSockets.exactMatch(ma))
74+
const webRTCDirectPorts = new Set()
75+
const webRTCDirectAddresses = libp2p.getMultiaddrs()
76+
.filter(ma => {
77+
const options = ma.toOptions()
78+
// firefox can't seem to dial loopback :shrug:
79+
if (options.host !== '127.0.0.1') {
80+
return false
81+
}
82+
83+
// only return one addr per port
84+
if (webRTCDirectPorts.has(options.port)) {
85+
return false
86+
}
87+
webRTCDirectPorts.add(options.port)
88+
89+
return WebRTCDirect.exactMatch(ma)
90+
})
7091

7192
return {
7293
libp2p,
@@ -75,6 +96,8 @@ export default {
7596
RELAY_MULTIADDR: wsAddresses[0],
7697
RELAY_WS_MULTIADDR_0: wsAddresses[0],
7798
RELAY_WS_MULTIADDR_1: wsAddresses[1],
99+
RELAY_WEBRTC_DIRECT_MULTIADDR_0: webRTCDirectAddresses[0],
100+
RELAY_WEBRTC_DIRECT_MULTIADDR_1: webRTCDirectAddresses[1],
78101
GO_RELAY_PEER: goLibp2pRelay.peerId,
79102
GO_RELAY_MULTIADDRS: goLibp2pRelay.multiaddrs,
80103
GO_RELAY_APIADDR: goLibp2pRelay.apiAddr

‎packages/integration-tests/test/compliance/transport/webrtc-direct.spec.ts

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,24 @@
11
import tests from '@libp2p/interface-compliance-tests/transport'
22
import { webRTCDirect } from '@libp2p/webrtc'
3+
import { multiaddr } from '@multiformats/multiaddr'
34
import { WebRTCDirect } from '@multiformats/multiaddr-matcher'
4-
import { isNode, isElectron } from 'wherearewe'
5+
import { isNode, isElectronMain, isWebWorker } from 'wherearewe'
6+
import { isFirefox } from '../../fixtures/utils.js'
57

68
describe('WebRTC-Direct interface-transport compliance', () => {
7-
if (!isNode && !isElectron) {
9+
if (isWebWorker) {
10+
return
11+
}
12+
13+
if (isFirefox) {
14+
// FireFox cannot dial loopback addresses and CI doesn't always have others
815
return
916
}
1017

1118
tests({
1219
async setup () {
20+
const canListen = isNode || isElectronMain
21+
1322
const dialer = {
1423
transports: [
1524
webRTCDirect()
@@ -21,15 +30,23 @@ describe('WebRTC-Direct interface-transport compliance', () => {
2130

2231
return {
2332
dialer,
24-
listener: {
25-
addresses: {
26-
listen: [
27-
'/ip4/127.0.0.1/udp/0/webrtc-direct',
28-
'/ip4/127.0.0.1/udp/0/webrtc-direct'
29-
]
30-
},
31-
...dialer
32-
},
33+
listener: canListen
34+
? {
35+
addresses: {
36+
listen: [
37+
'/ip4/127.0.0.1/udp/0/webrtc-direct',
38+
'/ip4/127.0.0.1/udp/0/webrtc-direct'
39+
]
40+
},
41+
...dialer
42+
}
43+
: undefined,
44+
dialAddrs: canListen
45+
? undefined
46+
: [
47+
multiaddr(process.env.RELAY_WEBRTC_DIRECT_MULTIADDR_0 ?? ''),
48+
multiaddr(process.env.RELAY_WEBRTC_DIRECT_MULTIADDR_1 ?? '')
49+
],
3350
dialMultiaddrMatcher: WebRTCDirect,
3451
listenMultiaddrMatcher: WebRTCDirect
3552
}

‎packages/integration-tests/test/fixtures/utils.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
import { RELAY_V2_HOP_CODEC } from '@libp2p/circuit-relay-v2'
22
import { peerIdFromString } from '@libp2p/peer-id'
3+
import { multiaddr, type Multiaddr } from '@multiformats/multiaddr'
34
import { detect } from 'detect-browser'
45
import pWaitFor from 'p-wait-for'
56
import { toString as uint8ArrayToString } from 'uint8arrays/to-string'
67
import type { Libp2p, AbortOptions, ContentRouting, PeerId, PeerInfo } from '@libp2p/interface'
78
import type { AddressManager } from '@libp2p/interface-internal'
8-
import type { Multiaddr } from '@multiformats/multiaddr'
99
import type { CID, Version } from 'multiformats'
1010
import type { Options as PWaitForOptions } from 'p-wait-for'
1111

@@ -66,10 +66,18 @@ export async function hasRelay (node: Libp2p, opts?: PWaitForOptions<PeerId>): P
6666
const relayHosts = new Set<string>()
6767

6868
const relayAddrs = node.getMultiaddrs().filter(addr => {
69-
const options = addr.toOptions()
70-
relayHosts.add(options.host)
69+
if (!addr.protoNames().includes('p2p-circuit')) {
70+
return false
71+
}
72+
73+
const ma = multiaddr(addr.toString().split('/p2p-circuit')[0])
74+
const relayPeer = ma.getPeerId()
75+
76+
if (relayPeer != null) {
77+
relayHosts.add(relayPeer)
78+
}
7179

72-
return addr.protoNames().includes('p2p-circuit')
80+
return true
7381
})
7482

7583
if (relayAddrs.length === 0) {

‎packages/integration-tests/test/webrtc-private-to-private.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ describe('webrtc private-to-private', () => {
5050
}
5151
})
5252

53-
// two because the relay listens on two TCP addresses so it binds to both
54-
expect(local.getMultiaddrs().filter(WebRTC.exactMatch)).to.have.lengthOf(2)
53+
expect(local.getMultiaddrs().filter(WebRTC.exactMatch))
54+
.to.have.property('length').that.is.greaterThan(1)
5555
})
5656
})

‎packages/transport-webrtc/src/private-to-public/listener.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ export class WebRTCDirectListener extends TypedEventEmitter<ListenerEvents> impl
145145
// ourselves
146146
this.log.trace('searching for free port')
147147
port = await getPort()
148+
this.log.trace('listening on free port %d', port)
148149
}
149150

150151
return stunListener(host, port, this.log, (ufrag, remoteHost, remotePort) => {

‎packages/transport-webrtc/src/private-to-public/transport.ts

Lines changed: 33 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import { BasicConstraintsExtension, X509Certificate, X509CertificateGenerator }
66
import { Key } from 'interface-datastore'
77
import { base64url } from 'multiformats/bases/base64'
88
import { sha256 } from 'multiformats/hashes/sha2'
9-
import { raceSignal } from 'race-signal'
109
import { equals as uint8ArrayEquals } from 'uint8arrays/equals'
1110
import { fromString as uint8ArrayFromString } from 'uint8arrays/from-string'
1211
import { DEFAULT_CERTIFICATE_DATASTORE_KEY, DEFAULT_CERTIFICATE_LIFESPAN, DEFAULT_CERTIFICATE_PRIVATE_KEY_NAME, DEFAULT_CERTIFICATE_RENEWAL_THRESHOLD } from '../constants.js'
@@ -158,45 +157,7 @@ export class WebRTCDirectTransport implements Transport, Startable {
158157
* Dial a given multiaddr
159158
*/
160159
async dial (ma: Multiaddr, options: DialTransportOptions<WebRTCDialEvents>): Promise<Connection> {
161-
const rawConn = await this._connect(ma, options)
162-
this.log('dialing address: %a', ma)
163-
return rawConn
164-
}
165-
166-
/**
167-
* Create transport listeners no supported by browsers
168-
*/
169-
createListener (options: CreateListenerOptions): Listener {
170-
if (this.certificate == null) {
171-
throw new NotStartedError()
172-
}
173-
174-
return new WebRTCDirectListener(this.components, {
175-
...this.init,
176-
...options,
177-
certificate: this.certificate,
178-
emitter: this.emitter
179-
})
180-
}
181-
182-
/**
183-
* Filter check for all Multiaddrs that this transport can listen on
184-
*/
185-
listenFilter (multiaddrs: Multiaddr[]): Multiaddr[] {
186-
return multiaddrs.filter(WebRTCDirect.exactMatch)
187-
}
188-
189-
/**
190-
* Filter check for all Multiaddrs that this transport can dial
191-
*/
192-
dialFilter (multiaddrs: Multiaddr[]): Multiaddr[] {
193-
return this.listenFilter(multiaddrs)
194-
}
195-
196-
/**
197-
* Connect to a peer using a multiaddr
198-
*/
199-
async _connect (ma: Multiaddr, options: DialTransportOptions<WebRTCDialEvents>): Promise<Connection> {
160+
this.log('dial %a', ma)
200161
// do not create RTCPeerConnection if the signal has already been aborted
201162
options.signal.throwIfAborted()
202163

@@ -212,7 +173,7 @@ export class WebRTCDirectTransport implements Transport, Startable {
212173
const peerConnection = await createDialerRTCPeerConnection('client', ufrag, typeof this.init.rtcConfiguration === 'function' ? await this.init.rtcConfiguration() : this.init.rtcConfiguration ?? {})
213174

214175
try {
215-
return await raceSignal(connect(peerConnection, ufrag, {
176+
return await connect(peerConnection, ufrag, {
216177
role: 'client',
217178
log: this.log,
218179
logger: this.components.logger,
@@ -225,13 +186,43 @@ export class WebRTCDirectTransport implements Transport, Startable {
225186
peerId: this.components.peerId,
226187
remotePeerId: theirPeerId,
227188
privateKey: this.components.privateKey
228-
}), options.signal)
189+
})
229190
} catch (err) {
230191
peerConnection.close()
231192
throw err
232193
}
233194
}
234195

196+
/**
197+
* Create a transport listener - this will throw in browsers
198+
*/
199+
createListener (options: CreateListenerOptions): Listener {
200+
if (this.certificate == null) {
201+
throw new NotStartedError()
202+
}
203+
204+
return new WebRTCDirectListener(this.components, {
205+
...this.init,
206+
...options,
207+
certificate: this.certificate,
208+
emitter: this.emitter
209+
})
210+
}
211+
212+
/**
213+
* Filter check for all Multiaddrs that this transport can listen on
214+
*/
215+
listenFilter (multiaddrs: Multiaddr[]): Multiaddr[] {
216+
return multiaddrs.filter(WebRTCDirect.exactMatch)
217+
}
218+
219+
/**
220+
* Filter check for all Multiaddrs that this transport can dial
221+
*/
222+
dialFilter (multiaddrs: Multiaddr[]): Multiaddr[] {
223+
return this.listenFilter(multiaddrs)
224+
}
225+
235226
private async getCertificate (forceRenew?: boolean): Promise<TransportCertificate> {
236227
if (isTransportCertificate(this.init.certificate)) {
237228
this.log('using provided TLS certificate')

0 commit comments

Comments
 (0)