[#3457] Proper fix for IllegalStateException caused by closed file descriptor / channel

Motivation:

During 6b941e9bdb I introduced a regression that could cause an IllegalStateException.
A non-proper fix was commited as part of #3443. This commit add a proper fix.

Modifications:

Remove FileDescriptor.INVALID and add FileDescriptor.isOpen() as replacement. Once FileDescriptor.close() is called isOpen() will return false.

Result:

No more IllegalStateException caused by a close channel.
This commit is contained in:
Norman Maurer 2015-03-01 19:36:31 +01:00
parent 91db4b71b2
commit d6016b7be8
4 changed files with 17 additions and 22 deletions

View File

@ -35,7 +35,7 @@ import java.nio.channels.UnresolvedAddressException;
abstract class AbstractEpollChannel extends AbstractChannel implements UnixChannel { abstract class AbstractEpollChannel extends AbstractChannel implements UnixChannel {
private static final ChannelMetadata DATA = new ChannelMetadata(false); private static final ChannelMetadata DATA = new ChannelMetadata(false);
private final int readFlag; private final int readFlag;
private volatile FileDescriptor fileDescriptor; private final FileDescriptor fileDescriptor;
protected int flags = Native.EPOLLET; protected int flags = Native.EPOLLET;
protected volatile boolean active; protected volatile boolean active;
@ -103,8 +103,7 @@ abstract class AbstractEpollChannel extends AbstractChannel implements UnixChann
doDeregister(); doDeregister();
FileDescriptor fd = fileDescriptor; FileDescriptor fd = fileDescriptor;
fileDescriptor = FileDescriptor.INVALID; fd.close();
Native.close(fd.intValue());
} }
@Override @Override
@ -119,7 +118,7 @@ abstract class AbstractEpollChannel extends AbstractChannel implements UnixChann
@Override @Override
public boolean isOpen() { public boolean isOpen() {
return fileDescriptor != FileDescriptor.INVALID; return fileDescriptor.isOpen();
} }
@Override @Override

View File

@ -97,7 +97,7 @@ public final class EpollDatagramChannel extends AbstractEpollChannel implements
@Override @Override
@SuppressWarnings("deprecation") @SuppressWarnings("deprecation")
public boolean isActive() { public boolean isActive() {
return fd() != FileDescriptor.INVALID && return fd().isOpen() &&
(config.getOption(ChannelOption.DATAGRAM_CHANNEL_ACTIVE_ON_REGISTRATION) && isRegistered() (config.getOption(ChannelOption.DATAGRAM_CHANNEL_ACTIVE_ON_REGISTRATION) && isRegistered()
|| active); || active);
} }

View File

@ -24,6 +24,7 @@ import java.io.IOException;
public class FileDescriptor { public class FileDescriptor {
private final int fd; private final int fd;
private volatile boolean open = true;
public FileDescriptor(int fd) { public FileDescriptor(int fd) {
if (fd < 0) { if (fd < 0) {
@ -32,21 +33,6 @@ public class FileDescriptor {
this.fd = fd; this.fd = fd;
} }
/**
* An invalid file descriptor which was closed before.
*/
public static final FileDescriptor INVALID = new FileDescriptor(0) {
@Override
public int intValue() {
throw new IllegalStateException("invalid file descriptor");
}
@Override
public void close() {
// NOOP
}
};
/** /**
* Return the int value of the filedescriptor. * Return the int value of the filedescriptor.
*/ */
@ -58,9 +44,17 @@ public class FileDescriptor {
* Close the file descriptor. * Close the file descriptor.
*/ */
public void close() throws IOException { public void close() throws IOException {
open = false;
close(fd); close(fd);
} }
/**
* Returns {@code true} if the file descriptor is open.
*/
public boolean isOpen() {
return open;
}
@Override @Override
public String toString() { public String toString() {
return "FileDescriptor{" + return "FileDescriptor{" +

View File

@ -93,8 +93,10 @@ public class EpollDomainSocketFdTest extends AbstractSocketTest {
sc.close().sync(); sc.close().sync();
if (received instanceof FileDescriptor) { if (received instanceof FileDescriptor) {
Assert.assertNotSame(FileDescriptor.INVALID, received); FileDescriptor fd = (FileDescriptor) received;
((FileDescriptor) received).close(); Assert.assertTrue(fd.isOpen());
fd.close();
Assert.assertFalse(fd.isOpen());
Assert.assertNull(queue.poll()); Assert.assertNull(queue.poll());
} else { } else {
throw (Throwable) received; throw (Throwable) received;