Skip to content

Commit b8d965d

Browse files
committed
Make socket traffic shutdown idempotent
Treat ENOTCONN and WSAENOTCONN as successful traffic shutdown because the peer may finish a graceful close before the owner cancels the I/O thread. Preserve all other failures and descriptor ownership. Add real loopback coverage for peer-first close and a synthetic failure case that verifies non-benign shutdown errors remain visible.
1 parent a531164 commit b8d965d

3 files changed

Lines changed: 85 additions & 1 deletion

File tree

core/src/main/c/share/net.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,8 @@ JNIEXPORT jint JNICALL Java_io_questdb_client_network_Net_send
130130

131131
JNIEXPORT jint JNICALL Java_io_questdb_client_network_Net_shutdown
132132
(JNIEnv *e, jclass cl, jint fd) {
133-
return shutdown((int) fd, SHUT_RDWR);
133+
const int result = shutdown((int) fd, SHUT_RDWR);
134+
return result == -1 && errno == ENOTCONN ? 0 : result;
134135
}
135136

136137
JNIEXPORT jint JNICALL Java_io_questdb_client_network_Net_recv

core/src/main/c/windows/net.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,11 @@ JNIEXPORT jint JNICALL Java_io_questdb_client_network_Net_shutdown
234234
(JNIEnv *e, jclass cl, jint fd) {
235235
const int result = shutdown((SOCKET) fd, SD_BOTH);
236236
if (result == SOCKET_ERROR) {
237+
const int error = WSAGetLastError();
238+
if (error == WSAENOTCONN) {
239+
return 0;
240+
}
241+
WSASetLastError(error);
237242
SaveLastError();
238243
}
239244
return result;

core/src/test/java/io/questdb/client/test/network/SocketTrafficShutdownTest.java

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,84 @@ public void testCompatibilityDefaultsDoNotBypassCustomTransportOwnership() {
259259
0, socketCloseCount.get());
260260
}
261261

262+
@Test(timeout = 30_000L)
263+
public void testPlainSocketShutdownAfterPeerDisconnectRetainsFd() throws Exception {
264+
Socket socket = new PlainSocket(NF, LoggerFactory.getLogger(SocketTrafficShutdownTest.class));
265+
266+
long buffer = 0;
267+
int fd = -1;
268+
try (ServerSocket listener = new ServerSocket()) {
269+
listener.bind(new InetSocketAddress("127.0.0.1", 0));
270+
long addrInfo = NF.getAddrInfo("127.0.0.1", listener.getLocalPort());
271+
Assert.assertNotEquals(-1L, addrInfo);
272+
try {
273+
fd = NF.socketTcp(true);
274+
Assert.assertTrue("could not allocate client socket", fd >= 0);
275+
Assert.assertEquals(0, NF.connectAddrInfo(fd, addrInfo));
276+
} finally {
277+
NF.freeAddrInfo(addrInfo);
278+
}
279+
280+
try (java.net.Socket peer = listener.accept()) {
281+
socket.of(fd);
282+
int retainedFd = fd;
283+
fd = -1;
284+
buffer = Unsafe.malloc(1, MemoryTag.NATIVE_DEFAULT);
285+
286+
peer.close();
287+
Assert.assertTrue("client must observe the peer disconnect", socket.recv(buffer, 1) < 0);
288+
289+
socket.closeTraffic();
290+
Assert.assertEquals("traffic cancellation must retain fd ownership", retainedFd, socket.getFd());
291+
Assert.assertFalse("traffic cancellation must not perform full close", socket.isClosed());
292+
Assert.assertTrue("shutdown must leave the descriptor allocated", NF.getSndBuf(retainedFd) > 0);
293+
294+
socket.close();
295+
Assert.assertTrue("full close must release the retained fd", socket.isClosed());
296+
Assert.assertEquals("released descriptor must reject socket operations", -1, NF.getSndBuf(retainedFd));
297+
}
298+
} finally {
299+
socket.close();
300+
if (buffer != 0) {
301+
Unsafe.free(buffer, 1, MemoryTag.NATIVE_DEFAULT);
302+
}
303+
if (fd != -1) {
304+
NF.close(fd);
305+
}
306+
}
307+
}
308+
309+
@Test
310+
public void testPlainSocketShutdownFailureStillThrows() {
311+
AtomicInteger closeCount = new AtomicInteger();
312+
NetworkFacade failingFacade = new CompatibilityNetworkFacade(closeCount) {
313+
@Override
314+
public int errno() {
315+
return 1234;
316+
}
317+
318+
@Override
319+
public int shutdown(int fd) {
320+
Assert.assertEquals(42, fd);
321+
return -1;
322+
}
323+
};
324+
PlainSocket socket = new PlainSocket(failingFacade, LoggerFactory.getLogger(SocketTrafficShutdownTest.class));
325+
socket.of(42);
326+
327+
try {
328+
socket.closeTraffic();
329+
Assert.fail("expected genuine traffic shutdown failure");
330+
} catch (IllegalStateException expected) {
331+
Assert.assertEquals("could not shut down socket traffic [fd=42, errno=1234]", expected.getMessage());
332+
} finally {
333+
socket.close();
334+
}
335+
336+
Assert.assertTrue(socket.isClosed());
337+
Assert.assertEquals("full close must release facade ownership exactly once", 1, closeCount.get());
338+
}
339+
262340
@Test(timeout = 30_000L)
263341
public void testPlainSocketShutdownWakesMacOsKqueueAndRetainsFd() throws Exception {
264342
assertShutdownWakesMacOsKqueue(new PlainSocket(NF, LoggerFactory.getLogger(SocketTrafficShutdownTest.class)));

0 commit comments

Comments
 (0)