Copilot commented on code in PR #2645:
URL: https://github.com/apache/plc4x/pull/2645#discussion_r3585086341
##########
plc4j/transports/tcp/src/test/java/org/apache/plc4x/java/transport/tcp/TcpTransportInstanceTest.java:
##########
@@ -290,6 +290,50 @@ void testConcurrentReadWrite() throws Exception {
assertTrue(doneLatch.await(10, TimeUnit.SECONDS));
}*/
+ @Test
+ void testConstructor_failedBind_doesNotLeakFileDescriptors() throws
Exception {
+ // getOpenFileDescriptorCount() is only exposed on Unix-flavoured JVMs
+ com.sun.management.UnixOperatingSystemMXBean osBean;
+ try {
+ osBean = (com.sun.management.UnixOperatingSystemMXBean)
+
java.lang.management.ManagementFactory.getOperatingSystemMXBean();
+ } catch (ClassCastException e) {
+ org.junit.jupiter.api.Assumptions.assumeTrue(false,
+ "Open file descriptor count not available on this platform");
+ return;
+ }
+
+ // Occupy a specific local port so binding our own channel to it
deterministically fails
+ // with BindException
+ try (ServerSocketChannel occupier = ServerSocketChannel.open()) {
+ occupier.bind(new InetSocketAddress("127.0.0.1", 0));
+ int occupiedPort = ((InetSocketAddress)
occupier.getLocalAddress()).getPort();
+
+ TcpTransportConfiguration config = new TcpTransportConfiguration();
+ config.receiveBufferSize = 81920;
+ config.localAddress = "127.0.0.1";
+ config.localPort = occupiedPort;
+ // Never actually reached - bind() fails first - but must be a
well-formed address.
+ InetSocketAddress remoteAddress = new
InetSocketAddress("127.0.0.1", occupiedPort);
+
+ int attempts = 20;
+ long before = osBean.getOpenFileDescriptorCount();
+ for (int i = 0; i < attempts; i++) {
+ assertThrows(TransportException.class, () ->
+ new TcpTransportInstance(remoteAddress, config,
AuditLog.builder().build())
+ );
+ }
+ long grown = osBean.getOpenFileDescriptorCount() - before;
+
+ // Without the fix each failed constructor call leaks the
SocketChannel it opened
+ // before bind() failed, so fd count grows by ~1 per attempt. With
the fix it stays flat.
+ assertTrue(grown < attempts,
+ "Open file descriptor count grew by " + grown + " across " +
attempts
+ + " failed bind attempts - each failed
TcpTransportInstance constructor call "
+ + "is leaking its SocketChannel instead of closing it");
Review Comment:
The assertion is too weak for the stated intent (“fd count stays flat”):
`grown < attempts` would still pass even if the constructor leaked file
descriptors intermittently or on only some iterations (e.g., `grown == attempts
- 1`). Tighten this to assert no per-attempt growth (optionally with a small
tolerance for background noise).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
