Skip to content

Commit d500b62

Browse files
committed
fix(quic): cache connection addresses to avoid NPE on freed channel
ConnectionOverNetty resolved local/remote addresses from the underlying Netty channel on every call. For QUIC the native channel state is freed on close, after which remoteSocketAddress() returns null and the !! in QuicTransport.remoteAddress() throws an NPE. This surfaced when a disconnect handler read the remote address during idle-timeout teardown. Cache each address on first successful read while the channel is live so teardown-time callers get a stable value. Add regression tests covering both addresses surviving a subsequently-freed channel.
1 parent 78e9f53 commit d500b62

2 files changed

Lines changed: 67 additions & 2 deletions

File tree

libp2p/src/main/kotlin/io/libp2p/transport/implementation/ConnectionOverNetty.kt

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,16 @@ open class ConnectionOverNetty(
2020
private lateinit var muxerSession: StreamMuxer.Session
2121
private lateinit var secureSession: SecureChannel.Session
2222

23+
// Addresses are resolved from the underlying Netty channel, but some transports (e.g. QUIC)
24+
// free the native channel state on close, after which the socket addresses are no longer
25+
// available. We cache the addresses on first successful read while the channel is still live
26+
// so that teardown-time callers (e.g. disconnect handlers) get a stable value instead of an NPE.
27+
@Volatile
28+
private var cachedLocalAddress: Multiaddr? = null
29+
30+
@Volatile
31+
private var cachedRemoteAddress: Multiaddr? = null
32+
2333
init {
2434
ch.attr(CONNECTION).set(this)
2535
}
@@ -35,6 +45,9 @@ open class ConnectionOverNetty(
3545
override fun secureSession() = secureSession
3646
override fun transport() = nettyTransport
3747

38-
override fun localAddress(): Multiaddr = nettyTransport.localAddress(nettyChannel)
39-
override fun remoteAddress(): Multiaddr = nettyTransport.remoteAddress(nettyChannel)
48+
override fun localAddress(): Multiaddr =
49+
cachedLocalAddress ?: nettyTransport.localAddress(nettyChannel).also { cachedLocalAddress = it }
50+
51+
override fun remoteAddress(): Multiaddr =
52+
cachedRemoteAddress ?: nettyTransport.remoteAddress(nettyChannel).also { cachedRemoteAddress = it }
4053
}
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
package io.libp2p.transport.implementation
2+
3+
import io.libp2p.core.multiformats.Multiaddr
4+
import io.mockk.every
5+
import io.mockk.mockk
6+
import io.mockk.verify
7+
import io.netty.channel.embedded.EmbeddedChannel
8+
import org.assertj.core.api.Assertions.assertThat
9+
import org.junit.jupiter.api.Test
10+
11+
class ConnectionOverNettyTest {
12+
13+
private val localMultiaddr = Multiaddr("/ip4/127.0.0.1/udp/1234/quic-v1")
14+
private val remoteMultiaddr = Multiaddr("/ip4/127.0.0.1/udp/5678/quic-v1")
15+
16+
private fun newConnection(transport: NettyTransport): ConnectionOverNetty =
17+
ConnectionOverNetty(EmbeddedChannel(), transport, true)
18+
19+
@Test
20+
fun `remoteAddress is cached after first read so it survives a freed channel`() {
21+
val transport = mockk<NettyTransport>()
22+
// First read resolves from the live channel; a second resolution would NPE because the
23+
// underlying (QUIC) native channel has been freed and remoteSocketAddress() returns null.
24+
every { transport.remoteAddress(any()) } returns remoteMultiaddr andThenThrows
25+
NullPointerException("channel freed")
26+
27+
val connection = newConnection(transport)
28+
29+
// Read once while the channel is "live"
30+
assertThat(connection.remoteAddress()).isEqualTo(remoteMultiaddr)
31+
32+
// A teardown-time read (e.g. a disconnect handler) must return the cached value rather
33+
// than hitting the freed channel again.
34+
assertThat(connection.remoteAddress()).isEqualTo(remoteMultiaddr)
35+
36+
verify(exactly = 1) { transport.remoteAddress(any()) }
37+
}
38+
39+
@Test
40+
fun `localAddress is cached after first read so it survives a freed channel`() {
41+
val transport = mockk<NettyTransport>()
42+
every { transport.localAddress(any()) } returns localMultiaddr andThenThrows
43+
NullPointerException("channel freed")
44+
45+
val connection = newConnection(transport)
46+
47+
assertThat(connection.localAddress()).isEqualTo(localMultiaddr)
48+
assertThat(connection.localAddress()).isEqualTo(localMultiaddr)
49+
50+
verify(exactly = 1) { transport.localAddress(any()) }
51+
}
52+
}

0 commit comments

Comments
 (0)