From 80f45d6ae5919c4eb4135914571ebc73cf88f60d Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Tue, 22 Dec 2015 21:16:29 +0100 Subject: [PATCH] Ensure closing a Socket / FileDescriptor multiple times will not throw exception Motivation: If an user will close a Socket / FileDescriptor multiple times we should handle the extra close operations as NOOP. Modifications: Only do the actual closing one time Result: No exception if close is called multiple times. --- .../io/netty/channel/unix/FileDescriptor.java | 26 ++++-- .../io/netty/channel/unix/SocketTest.java | 84 +++++++------------ 2 files changed, 50 insertions(+), 60 deletions(-) diff --git a/transport-native-epoll/src/main/java/io/netty/channel/unix/FileDescriptor.java b/transport-native-epoll/src/main/java/io/netty/channel/unix/FileDescriptor.java index 285505c26ff..48fc555c3a4 100644 --- a/transport-native-epoll/src/main/java/io/netty/channel/unix/FileDescriptor.java +++ b/transport-native-epoll/src/main/java/io/netty/channel/unix/FileDescriptor.java @@ -15,9 +15,12 @@ */ package io.netty.channel.unix; +import io.netty.util.internal.PlatformDependent; + import java.io.File; import java.io.IOException; import java.nio.ByteBuffer; +import java.util.concurrent.atomic.AtomicIntegerFieldUpdater; import static io.netty.channel.unix.Errors.CONNECTION_RESET_EXCEPTION_READ; import static io.netty.channel.unix.Errors.CONNECTION_RESET_EXCEPTION_WRITE; @@ -31,8 +34,18 @@ * {@link FileDescriptor} for it. */ public class FileDescriptor { + private static final AtomicIntegerFieldUpdater openUpdater; + static { + AtomicIntegerFieldUpdater updater + = PlatformDependent.newAtomicIntegerFieldUpdater(FileDescriptor.class, "open"); + if (updater == null) { + updater = AtomicIntegerFieldUpdater.newUpdater(FileDescriptor.class, "open"); + } + openUpdater = updater; + } + private final int fd; - private volatile boolean open = true; + private volatile int open = 1; public FileDescriptor(int fd) { if (fd < 0) { @@ -52,10 +65,11 @@ public int intValue() { * Close the file descriptor. */ public void close() throws IOException { - open = false; - int res = close(fd); - if (res < 0) { - throw newIOException("close", res); + if (openUpdater.compareAndSet(this, 1, 0)) { + int res = close(fd); + if (res < 0) { + throw newIOException("close", res); + } } } @@ -63,7 +77,7 @@ public void close() throws IOException { * Returns {@code true} if the file descriptor is open. */ public boolean isOpen() { - return open; + return open == 1; } public final int write(ByteBuffer buf, int pos, int limit) throws IOException { diff --git a/transport-native-epoll/src/test/java/io/netty/channel/unix/SocketTest.java b/transport-native-epoll/src/test/java/io/netty/channel/unix/SocketTest.java index 0fe9df0f385..8671bdedfa6 100644 --- a/transport-native-epoll/src/test/java/io/netty/channel/unix/SocketTest.java +++ b/transport-native-epoll/src/test/java/io/netty/channel/unix/SocketTest.java @@ -44,80 +44,56 @@ public void tearDown() throws IOException { @Test public void testKeepAlive() throws Exception { - Socket socket = Socket.newSocketStream(); - try { - assertFalse(socket.isKeepAlive()); - socket.setKeepAlive(true); - assertTrue(socket.isKeepAlive()); - } finally { - socket.close(); - } + assertFalse(socket.isKeepAlive()); + socket.setKeepAlive(true); + assertTrue(socket.isKeepAlive()); } @Test public void testTcpCork() throws Exception { - Socket socket = Socket.newSocketStream(); - try { - assertFalse(socket.isTcpCork()); - socket.setTcpCork(true); - assertTrue(socket.isTcpCork()); - } finally { - socket.close(); - } + assertFalse(socket.isTcpCork()); + socket.setTcpCork(true); + assertTrue(socket.isTcpCork()); } @Test public void testTcpNoDelay() throws Exception { - Socket socket = Socket.newSocketStream(); - try { - assertFalse(socket.isTcpNoDelay()); - socket.setTcpNoDelay(true); - assertTrue(socket.isTcpNoDelay()); - } finally { - socket.close(); - } + assertFalse(socket.isTcpNoDelay()); + socket.setTcpNoDelay(true); + assertTrue(socket.isTcpNoDelay()); } @Test public void testReceivedBufferSize() throws Exception { - Socket socket = Socket.newSocketStream(); - try { - int size = socket.getReceiveBufferSize(); - int newSize = 65535; - assertTrue(size > 0); - socket.setReceiveBufferSize(newSize); - // Linux usually set it to double what is specified - assertTrue(newSize <= socket.getReceiveBufferSize()); - } finally { - socket.close(); - } + int size = socket.getReceiveBufferSize(); + int newSize = 65535; + assertTrue(size > 0); + socket.setReceiveBufferSize(newSize); + // Linux usually set it to double what is specified + assertTrue(newSize <= socket.getReceiveBufferSize()); } @Test public void testSendBufferSize() throws Exception { - Socket socket = Socket.newSocketStream(); - try { - int size = socket.getSendBufferSize(); - int newSize = 65535; - assertTrue(size > 0); - socket.setSendBufferSize(newSize); - // Linux usually set it to double what is specified - assertTrue(newSize <= socket.getSendBufferSize()); - } finally { - socket.close(); - } + int size = socket.getSendBufferSize(); + int newSize = 65535; + assertTrue(size > 0); + socket.setSendBufferSize(newSize); + // Linux usually set it to double what is specified + assertTrue(newSize <= socket.getSendBufferSize()); } @Test public void testSoLinger() throws Exception { + assertEquals(-1, socket.getSoLinger()); + socket.setSoLinger(10); + assertEquals(10, socket.getSoLinger()); + } + + @Test + public void testDoubleCloseDoesNotThrow() throws IOException { Socket socket = Socket.newSocketStream(); - try { - assertEquals(-1, socket.getSoLinger()); - socket.setSoLinger(10); - assertEquals(10, socket.getSoLinger()); - } finally { - socket.close(); - } + socket.close(); + socket.close(); } } -