Reuse relay connections across pairings by marcus-pousette-hp · Pull Request #252 · holepunchto/hyperdht

marcus-pousette-hp · 2026-04-28T10:57:13Z

Previously, each relayed app connection opened its own HyperDHT connection to the same relay. Multiple relayed connections through one relay therefore created duplicate relay transport sockets and extra keepalive/session overhead.

This adds an internal relay pool keyed by relay public key. Relay pairings now reuse one relay transport connection when possible, while each app connection still gets its own blind-relay pairing/raw stream.

Adds coverage for two simultaneous relayed app connections through the same relay. Previously this opened four relay transport sockets: client-to-relay and server-to-relay for each app connection. With reuse, the client and server each keep one relay transport socket, and the two app connections are represented as separate blind-relay pairings over those shared sockets.

Context

There are already two related reuse mechanisms, but neither owns blind-relay pairings:

ConnectionPool reuses app-level HyperDHT connections keyed by remote public key. It returns an existing encrypted app stream and handles duplicate app connections.
_socketPool / reusableSocket reuses lower-level UDX socket routes.

Relay reuse needs different lifecycle semantics. The shared object is the HyperDHT transport connection to the relay, but each app connection still needs its own blind-relay pairing/token/raw stream. When one app connection upgrades to direct or closes, we must release only that pairing, not destroy the shared relay transport if another pairing is still active.

This PR adds a small internal RelayPool for that relay-specific ownership model.

Notes

Unpairing delay

When a relayed app connection upgrades to direct, this PR releases only that relay pairing and keeps the shared relay transport open for any other active pairings.

The unpair is delayed because remote-changed is only a local signal. It means this side moved its raw stream to the direct path, but it does not prove the peer has also finished switching direct. Sending unpair immediately can tear down the relay leg while the peer still needs it, which showed up as a timeout in the direct-upgrade regression test.

The delay is a pragmatic, non-protocol-changing guard. A longer delay is safer for the upgrade race but keeps relay resources around longer after direct upgrade. A future protocol-level improvement could replace this with an explicit “both sides are direct” release signal before unpairing.

Race Coverage

This is the tricky part of this PR since we re-using relay connection naturally creates a large flake surface around lifecycle mangement.

This PR includes coverage for the relay-pool cases most likely to regress:

avoids reusing a relay transport after it has started closing
keeps active relay pairings alive when another pairing upgrades to direct
clears/unrefs delayed direct-upgrade unpair timers on pool destroy
keeps the shared relay transport open when a new pairing starts during another pairing’s delayed unpair window

Disclaimer: PR was co-written with AI.

noahlevenson · 2026-05-31T01:45:16Z

+    this.socket.setKeepAlive(keepAlive)
+  }
+
+  get reusable() {


Reviewing at the request of @marcus-pousette-hp...

The death signal is actually 4 async signals spread across different layers of abstraction. Scares me a little. The different destruction paths and layers of destruction state (RelayPool vs. socket vs. client) make it hard to reason about whether there might be a subtle race between the predicates. Feels like a violation of the isolation of complexity.

Makes sense, yeap this could be fragile even though it is tested. I did a cleanup commit

13af091

that unifies this somewhat, and makes this a bit easier to overview

lejeunerenard · 2026-05-26T23:55:37Z

+  await Promise.all([
+    relaySocketsOpened,
+    relayStreamsPaired,
+    serverSocketsOpened,
+    ...clientSockets.map((socket) => once(socket, 'open'))
+  ])


Suggested change

await Promise.all([

relaySocketsOpened,

relayStreamsPaired,

serverSocketsOpened,

...clientSockets.map((socket) => once(socket, 'open'))

])

await Promise.all(clientSockets.map((socket) => once(socket, 'open'))

These aren't necessary as since the client connection can't open until the connection is fully established (handshake etc). Timing only matters if we use the server sockets array immediately since it might not have run its callback yet. But we dont until after replies etc.

This line can also will go away if we opt to await the connections individually.

Fixed in 3f91591

The relaySocketsOpened & relayStreamsPaired are still not required here:
https://github.com/holepunchto/hyperdht/pull/252/changes#diff-77e8658bbd4f0803c631d07808c79139fb0bb8f533f28dd5268a9bc196613b13R622

We know they are true because the client socket's being open when its forced to relay means the sockets were open and the streams have paired.

Yeah you are right here too. Fixed 5b4b5cf

lejeunerenard · 2026-06-03T17:41:46Z

  }
 }

+function getOnlyRelayPoolConnection(node) {


All helpers should be at the end of the file unless within a given test.

Renaming fix in 0f7cbda

Looks like while there are less helpers, there are still helpers added in the middle of the tests.

Missed that. Now fixed in 0ce3a5a.

lejeunerenard · 2026-06-03T20:36:24Z

+  secondPairing.release()
+  secondStream.destroy()
+
+  await waitFor(() => relay._pairing.size === 0)
+  await waitFor(() => clientNode._relayPool._entries.size === 0)


Why is this necessary? There are no asserts afterwards and this is done via destroying the nodes.

In the earlier version those waits were implicit cleanup assertions after releasing the second pairing. That was outside this test’s main scope, so I removed that part and kept the test focused on the delayed-unpair race.

So these aren't required still correct? I only see the relay._pairing.size === 0 removed at the moment.

Yeah, hmm for some reason I only removed one part by mistake. Fixed in bafe3fd

lejeunerenard · 2026-06-03T21:09:46Z

+  const [upgradeServerSocket, relayOnlyServerSocket] = await Promise.all([
+    upgradeServerSocketOpened,
+    relayOnlyServerSocketOpened,
+    once(upgradeClientSocket, 'open'),
+    once(relayOnlyClientSocket, 'open')
+  ])
+  await Promise.all([relaySocketsOpened, relayStreamsPaired])


Suggested change

const [upgradeServerSocket, relayOnlyServerSocket] = await Promise.all([

upgradeServerSocketOpened,

relayOnlyServerSocketOpened,

once(upgradeClientSocket, 'open'),

once(relayOnlyClientSocket, 'open')

])

await Promise.all([relaySocketsOpened, relayStreamsPaired])

await Promise.all(

once(upgradeClientSocket, 'open'),

once(relayOnlyClientSocket, 'open')

])

Since upgradeServerSocket & relayOnlyServerSocket are the only sockets on their respective servers and aren't needed immediately, these can be set in their respective callbacks. The unused promises etc can be cleaned up then.

Yup good point, fixed
88b5487

Thanks the sockets are now defined in their callbacks, but another aspect of this suggested change was removing the relaySocketsOpened and the relayStreamsPaired as those are not required because the client socket's being open implies they were done per this comment:
#252 (comment)

Fixed in 4e00f13.

Makes things way simpler, thank you

lejeunerenard · 2026-06-08T21:20:38Z

+      this._connections.set(keyString, connection)
+    } else {
+      connection.setKeepAlive(keepAlive)
+    }
+
+    return connection
+  }
+
+  _delete(connection) {
+    if (this._connections.get(connection.keyString) === connection) {
+      this._connections.delete(connection.keyString)
+    }


Just wanted to note: Technically a connection could be orphaned here if its not reusable when another connection for the same public key is made.

You are right, good catch! Fixed e4c2908

mafintosh · 2026-07-06T06:36:17Z

+    if (!this._closeState()) return
+
+    this._releasePairings(false)
+    const closed = this.socket.destroyed ? null : once(this.socket, 'close')


Events.once does weird stuff if error event is emitted. Just listen once for close only manually

Good point! that was subtle and fragile.. Fixed 18376ef

marcus-pousette-hp force-pushed the fix/close-relay-after-direct-upgrade branch from a6abc84 to bfd5f96 Compare April 28, 2026 18:53

marcus-pousette-hp force-pushed the feat/relay-connection-reuse branch from 73a3495 to 1da0e29 Compare April 28, 2026 18:56

marcus-pousette-hp force-pushed the fix/close-relay-after-direct-upgrade branch from bfd5f96 to 8ed4285 Compare April 28, 2026 19:18

marcus-pousette-hp force-pushed the feat/relay-connection-reuse branch from 1da0e29 to 05e36bb Compare April 28, 2026 19:19

marcus-pousette-hp force-pushed the fix/close-relay-after-direct-upgrade branch from 8ed4285 to c31bc3e Compare April 28, 2026 19:23

marcus-pousette-hp force-pushed the feat/relay-connection-reuse branch 2 times, most recently from a67cc69 to 28f3eea Compare May 4, 2026 11:49

marcus-pousette-hp force-pushed the fix/close-relay-after-direct-upgrade branch from c31bc3e to 3a0979e Compare May 4, 2026 11:49

marcus-pousette-hp force-pushed the feat/relay-connection-reuse branch from 28f3eea to 10b2532 Compare May 12, 2026 08:37

marcus-pousette-hp marked this pull request as ready for review May 12, 2026 08:55

marcus-pousette-hp force-pushed the feat/relay-connection-reuse branch 3 times, most recently from 452c794 to a43de21 Compare May 12, 2026 10:44

marcus-pousette-hp mentioned this pull request May 12, 2026

Feat/relay service mode #255

Draft

Base automatically changed from fix/close-relay-after-direct-upgrade to main May 12, 2026 13:58

lejeunerenard requested a review from a team May 12, 2026 13:58

marcus-pousette-hp force-pushed the feat/relay-connection-reuse branch from a43de21 to 7f2025a Compare May 12, 2026 16:22

marcus-pousette-hp marked this pull request as draft May 12, 2026 21:01

marcus-pousette-hp force-pushed the feat/relay-connection-reuse branch from 7f2025a to c7170b8 Compare May 12, 2026 21:11

marcus-pousette-hp marked this pull request as ready for review May 13, 2026 21:27

lejeunerenard requested changes May 21, 2026

View reviewed changes

Comment thread lib/connect.js Outdated

Comment thread lib/relay-pool.js

Comment thread lib/connect.js

Comment thread lib/relay-connection.js Outdated

Comment thread lib/relay-connection.js Outdated

Comment thread lib/server.js Outdated

marcus-pousette-hp requested a review from lejeunerenard May 22, 2026 13:41

noahlevenson self-requested a review May 31, 2026 00:25

noahlevenson reviewed May 31, 2026

View reviewed changes

lejeunerenard requested changes Jun 3, 2026

View reviewed changes

marcus-pousette-hp requested a review from lejeunerenard June 4, 2026 13:51

lejeunerenard reviewed Jun 19, 2026

View reviewed changes

mafintosh reviewed Jul 6, 2026

View reviewed changes

marcus-pousette-hp added 2 commits July 6, 2026 13:27

Reuse relay connections across pairings

00e3066

Avoid reusing closing relay pool transports

61a495e

marcus-pousette-hp added 26 commits July 6, 2026 13:27

Simplify relay pairing cleanup

0d809c4

Respect relay keepalive when reusing transports

453c157

Remove stale relay socket references

a8f4746

Rename RelayPoolEntry -> RelayPoolConnection

d923585

Centralize relay pool connection cleanup

f63144f

Clean up relay reuse test socket tracking

379ed9c

Reuse relay transport across keepalive values

6c68ef1

Rename relay pool entries to connections

2a36b16

Avoid monkey patching relay unpair timers

a9fc37c

Use shorter delayed unpair in relay test

b87b3aa

Clarify delayed relay unpair test

38a5670

Format relay reuse test

1cf1137

Use normal relay pool cleanup in test

6269768

Simplify relay upgrade server socket tracking

85e41fe

Remove redundant relay test wait

58baf13

Simplify relay reconnect test waits

a3badb5

Simplify relay pool open waits

11dd797

Tighten relay abort assertions

f54fd1c

Remove redundant relay keepalive cleanup

9632059

Simplify relay reuse open waits

e7c1106

Simplify pooled relay upgrade waits

1893d5d

Move relay test helpers to end

96904e6

Remove redundant delayed unpair cleanup

9ad10fb

Remove redundant closing relay cleanup

6037a2d

Clean up stale relay pool connections before reuse

6b7701e

Wait for relay transport close on destroy

18376ef

marcus-pousette-hp force-pushed the feat/relay-connection-reuse branch from e4c2908 to 18376ef Compare July 6, 2026 12:17

marcus-pousette-hp added 2 commits July 6, 2026 14:24

Allow slow relay upgrade test

6957f7b

Handle duplicate pending relay tokens

7e37d94

marcus-pousette-hp force-pushed the feat/relay-connection-reuse branch from 0798b81 to 7e37d94 Compare July 13, 2026 15:55

Uh oh!

Conversation

marcus-pousette-hp commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Context

Notes

Unpairing delay

Race Coverage

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marcus-pousette-hp Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marcus-pousette-hp Jun 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

marcus-pousette-hp commented Apr 28, 2026 •

edited

Loading

marcus-pousette-hp Jun 4, 2026 •

edited

Loading

marcus-pousette-hp Jun 5, 2026 •

edited

Loading