Skip to content

Commit

Permalink
[netty#3457] Proper fix for IllegalStateException caused by closed fi…
Browse files Browse the repository at this point in the history
…le descriptor / channel

Motivation:

During 6b941e9 I introduced a regression that could cause an IllegalStateException.
A non-proper fix was commited as part of netty#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.
  • Loading branch information
normanmaurer committed Mar 1, 2015
1 parent 3ac08a0 commit 0985e07
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
abstract class AbstractEpollChannel extends AbstractChannel implements UnixChannel {
private static final ChannelMetadata DATA = new ChannelMetadata(false);
private final int readFlag;
private volatile FileDescriptor fileDescriptor;
private final FileDescriptor fileDescriptor;
protected int flags = Native.EPOLLET;

protected volatile boolean active;
Expand Down Expand Up @@ -103,8 +103,7 @@ protected void doClose() throws Exception {
doDeregister();

FileDescriptor fd = fileDescriptor;
fileDescriptor = FileDescriptor.INVALID;
Native.close(fd.intValue());
fd.close();
}

@Override
Expand All @@ -119,7 +118,7 @@ protected boolean isCompatible(EventLoop loop) {

@Override
public boolean isOpen() {
return fileDescriptor != FileDescriptor.INVALID;
return fileDescriptor.isOpen();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public ChannelMetadata metadata() {
@Override
@SuppressWarnings("deprecation")
public boolean isActive() {
return fd() != FileDescriptor.INVALID &&
return fd().isOpen() &&
(config.getOption(ChannelOption.DATAGRAM_CHANNEL_ACTIVE_ON_REGISTRATION) && isRegistered()
|| active);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
public class FileDescriptor {

private final int fd;
private volatile boolean open = true;

public FileDescriptor(int fd) {
if (fd < 0) {
Expand All @@ -32,21 +33,6 @@ public FileDescriptor(int 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.
*/
Expand All @@ -58,9 +44,17 @@ public int intValue() {
* Close the file descriptor.
*/
public void close() throws IOException {
open = false;
close(fd);
}

/**
* Returns {@code true} if the file descriptor is open.
*/
public boolean isOpen() {
return open;
}

@Override
public String toString() {
return "FileDescriptor{" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,10 @@ public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws E
sc.close().sync();

if (received instanceof FileDescriptor) {
Assert.assertNotSame(FileDescriptor.INVALID, received);
((FileDescriptor) received).close();
FileDescriptor fd = (FileDescriptor) received;
Assert.assertTrue(fd.isOpen());
fd.close();
Assert.assertFalse(fd.isOpen());
Assert.assertNull(queue.poll());
} else {
throw (Throwable) received;
Expand Down

0 comments on commit 0985e07

Please sign in to comment.