5025260: Register methods should throw ClosedChannelException instead of NPE
Summary: update the spec and implementation to throw ClosedSelectorException
Reviewed-by: alanb
--- a/jdk/src/share/classes/java/nio/channels/SelectableChannel.java Fri Oct 10 10:58:08 2008 +0200
+++ b/jdk/src/share/classes/java/nio/channels/SelectableChannel.java Mon Oct 13 14:45:27 2008 -0700
@@ -191,6 +191,9 @@
* @throws ClosedChannelException
* If this channel is closed
*
+ * @throws ClosedSelectorException
+ * If the selector is closed
+ *
* @throws IllegalBlockingModeException
* If this channel is in blocking mode
*
@@ -246,6 +249,9 @@
* @throws ClosedChannelException
* If this channel is closed
*
+ * @throws ClosedSelectorException
+ * If the selector is closed
+ *
* @throws IllegalBlockingModeException
* If this channel is in blocking mode
*
--- a/jdk/src/share/classes/java/nio/channels/spi/AbstractSelectableChannel.java Fri Oct 10 10:58:08 2008 +0200
+++ b/jdk/src/share/classes/java/nio/channels/spi/AbstractSelectableChannel.java Mon Oct 13 14:45:27 2008 -0700
@@ -175,6 +175,16 @@
* the selector is invoked while holding the appropriate locks. The
* resulting key is added to this channel's key set before being returned.
* </p>
+ *
+ * @throws ClosedSelectorException {@inheritDoc}
+ *
+ * @throws IllegalBlockingModeException {@inheritDoc}
+ *
+ * @throws IllegalSelectorException {@inheritDoc}
+ *
+ * @throws CancelledKeyException {@inheritDoc}
+ *
+ * @throws IllegalArgumentException {@inheritDoc}
*/
public final SelectionKey register(Selector sel, int ops,
Object att)
--- a/jdk/src/share/classes/sun/nio/ch/AbstractPollSelectorImpl.java Fri Oct 10 10:58:08 2008 +0200
+++ b/jdk/src/share/classes/sun/nio/ch/AbstractPollSelectorImpl.java Mon Oct 13 14:45:27 2008 -0700
@@ -58,6 +58,9 @@
// True if this Selector has been closed
private boolean closed = false;
+ // Lock for close and cleanup
+ private Object closeLock = new Object();
+
AbstractPollSelectorImpl(SelectorProvider sp, int channels, int offset) {
super(sp);
this.totalChannels = channels;
@@ -65,7 +68,11 @@
}
void putEventOps(SelectionKeyImpl sk, int ops) {
- pollWrapper.putEventOps(sk.getIndex(), ops);
+ synchronized (closeLock) {
+ if (closed)
+ throw new ClosedSelectorException();
+ pollWrapper.putEventOps(sk.getIndex(), ops);
+ }
}
public Selector wakeup() {
@@ -76,7 +83,9 @@
protected abstract int doSelect(long timeout) throws IOException;
protected void implClose() throws IOException {
- if (!closed) {
+ synchronized (closeLock) {
+ if (closed)
+ return;
closed = true;
// Deregister channels
for(int i=channelOffset; i<totalChannels; i++) {
@@ -129,23 +138,28 @@
}
protected void implRegister(SelectionKeyImpl ski) {
- // Check to see if the array is large enough
- if (channelArray.length == totalChannels) {
- // Make a larger array
- int newSize = pollWrapper.totalChannels * 2;
- SelectionKeyImpl temp[] = new SelectionKeyImpl[newSize];
- // Copy over
- for (int i=channelOffset; i<totalChannels; i++)
- temp[i] = channelArray[i];
- channelArray = temp;
- // Grow the NativeObject poll array
- pollWrapper.grow(newSize);
+ synchronized (closeLock) {
+ if (closed)
+ throw new ClosedSelectorException();
+
+ // Check to see if the array is large enough
+ if (channelArray.length == totalChannels) {
+ // Make a larger array
+ int newSize = pollWrapper.totalChannels * 2;
+ SelectionKeyImpl temp[] = new SelectionKeyImpl[newSize];
+ // Copy over
+ for (int i=channelOffset; i<totalChannels; i++)
+ temp[i] = channelArray[i];
+ channelArray = temp;
+ // Grow the NativeObject poll array
+ pollWrapper.grow(newSize);
+ }
+ channelArray[totalChannels] = ski;
+ ski.setIndex(totalChannels);
+ pollWrapper.addEntry(ski.channel);
+ totalChannels++;
+ keys.add(ski);
}
- channelArray[totalChannels] = ski;
- ski.setIndex(totalChannels);
- pollWrapper.addEntry(ski.channel);
- totalChannels++;
- keys.add(ski);
}
protected void implDereg(SelectionKeyImpl ski) throws IOException {
--- a/jdk/src/solaris/classes/sun/nio/ch/DevPollSelectorImpl.java Fri Oct 10 10:58:08 2008 +0200
+++ b/jdk/src/solaris/classes/sun/nio/ch/DevPollSelectorImpl.java Mon Oct 13 14:45:27 2008 -0700
@@ -46,15 +46,15 @@
// The poll object
DevPollArrayWrapper pollWrapper;
- // The number of valid channels in this Selector's poll array
- private int totalChannels;
-
// Maps from file descriptors to keys
private Map<Integer,SelectionKeyImpl> fdToKey;
// True if this Selector has been closed
private boolean closed = false;
+ // Lock for close/cleanup
+ private Object closeLock = new Object();
+
// Lock for interrupt triggering and clearing
private Object interruptLock = new Object();
private boolean interruptTriggered = false;
@@ -72,7 +72,6 @@
pollWrapper = new DevPollArrayWrapper();
pollWrapper.initInterrupt(fd0, fd1);
fdToKey = new HashMap<Integer,SelectionKeyImpl>();
- totalChannels = 1;
}
protected int doSelect(long timeout)
@@ -131,45 +130,39 @@
}
protected void implClose() throws IOException {
- if (!closed) {
- closed = true;
-
- // prevent further wakeup
- synchronized (interruptLock) {
- interruptTriggered = true;
- }
+ if (closed)
+ return;
+ closed = true;
- FileDispatcher.closeIntFD(fd0);
- FileDispatcher.closeIntFD(fd1);
- if (pollWrapper != null) {
+ // prevent further wakeup
+ synchronized (interruptLock) {
+ interruptTriggered = true;
+ }
- pollWrapper.release(fd0);
- pollWrapper.closeDevPollFD();
- pollWrapper = null;
- selectedKeys = null;
+ FileDispatcher.closeIntFD(fd0);
+ FileDispatcher.closeIntFD(fd1);
- // Deregister channels
- Iterator i = keys.iterator();
- while (i.hasNext()) {
- SelectionKeyImpl ski = (SelectionKeyImpl)i.next();
- deregister(ski);
- SelectableChannel selch = ski.channel();
- if (!selch.isOpen() && !selch.isRegistered())
- ((SelChImpl)selch).kill();
- i.remove();
- }
- totalChannels = 0;
+ pollWrapper.release(fd0);
+ pollWrapper.closeDevPollFD();
+ selectedKeys = null;
- }
- fd0 = -1;
- fd1 = -1;
+ // Deregister channels
+ Iterator i = keys.iterator();
+ while (i.hasNext()) {
+ SelectionKeyImpl ski = (SelectionKeyImpl)i.next();
+ deregister(ski);
+ SelectableChannel selch = ski.channel();
+ if (!selch.isOpen() && !selch.isRegistered())
+ ((SelChImpl)selch).kill();
+ i.remove();
}
+ fd0 = -1;
+ fd1 = -1;
}
protected void implRegister(SelectionKeyImpl ski) {
int fd = IOUtil.fdVal(ski.channel.getFD());
fdToKey.put(Integer.valueOf(fd), ski);
- totalChannels++;
keys.add(ski);
}
@@ -179,7 +172,6 @@
int fd = ski.channel.getFDVal();
fdToKey.remove(Integer.valueOf(fd));
pollWrapper.release(fd);
- totalChannels--;
ski.setIndex(-1);
keys.remove(ski);
selectedKeys.remove(ski);
@@ -190,6 +182,8 @@
}
void putEventOps(SelectionKeyImpl sk, int ops) {
+ if (closed)
+ throw new ClosedSelectorException();
int fd = IOUtil.fdVal(sk.channel.getFD());
pollWrapper.setInterest(fd, ops);
}
--- a/jdk/src/solaris/classes/sun/nio/ch/EPollSelectorImpl.java Fri Oct 10 10:58:08 2008 +0200
+++ b/jdk/src/solaris/classes/sun/nio/ch/EPollSelectorImpl.java Mon Oct 13 14:45:27 2008 -0700
@@ -1,5 +1,5 @@
/*
- * Copyright 2005-2008 Sun Microsystems, Inc. All Rights Reserved.
+ * Copyright 2005-2007 Sun Microsystems, Inc. All Rights Reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -31,7 +31,6 @@
import java.util.*;
import sun.misc.*;
-
/**
* An implementation of Selector for Linux 2.6+ kernels that uses
* the epoll event notification facility.
@@ -51,7 +50,7 @@
private Map<Integer,SelectionKeyImpl> fdToKey;
// True if this Selector has been closed
- private boolean closed = false;
+ private volatile boolean closed = false;
// Lock for interrupt triggering and clearing
private Object interruptLock = new Object();
@@ -128,40 +127,41 @@
}
protected void implClose() throws IOException {
- if (!closed) {
- closed = true;
+ if (closed)
+ return;
+ closed = true;
- // prevent further wakeup
- synchronized (interruptLock) {
- interruptTriggered = true;
- }
+ // prevent further wakeup
+ synchronized (interruptLock) {
+ interruptTriggered = true;
+ }
- FileDispatcher.closeIntFD(fd0);
- FileDispatcher.closeIntFD(fd1);
- if (pollWrapper != null) {
+ FileDispatcher.closeIntFD(fd0);
+ FileDispatcher.closeIntFD(fd1);
- pollWrapper.release(fd0);
- pollWrapper.closeEPollFD();
- pollWrapper = null;
- selectedKeys = null;
+ pollWrapper.release(fd0);
+ pollWrapper.closeEPollFD();
+ // it is possible
+ selectedKeys = null;
- // Deregister channels
- Iterator i = keys.iterator();
- while (i.hasNext()) {
- SelectionKeyImpl ski = (SelectionKeyImpl)i.next();
- deregister(ski);
- SelectableChannel selch = ski.channel();
- if (!selch.isOpen() && !selch.isRegistered())
- ((SelChImpl)selch).kill();
- i.remove();
- }
- }
- fd0 = -1;
- fd1 = -1;
+ // Deregister channels
+ Iterator i = keys.iterator();
+ while (i.hasNext()) {
+ SelectionKeyImpl ski = (SelectionKeyImpl)i.next();
+ deregister(ski);
+ SelectableChannel selch = ski.channel();
+ if (!selch.isOpen() && !selch.isRegistered())
+ ((SelChImpl)selch).kill();
+ i.remove();
}
+
+ fd0 = -1;
+ fd1 = -1;
}
protected void implRegister(SelectionKeyImpl ski) {
+ if (closed)
+ throw new ClosedSelectorException();
int fd = IOUtil.fdVal(ski.channel.getFD());
fdToKey.put(Integer.valueOf(fd), ski);
pollWrapper.add(fd);
@@ -183,6 +183,8 @@
}
void putEventOps(SelectionKeyImpl sk, int ops) {
+ if (closed)
+ throw new ClosedSelectorException();
int fd = IOUtil.fdVal(sk.channel.getFD());
pollWrapper.setInterest(fd, ops);
}
--- a/jdk/src/windows/classes/sun/nio/ch/WindowsSelectorImpl.java Fri Oct 10 10:58:08 2008 +0200
+++ b/jdk/src/windows/classes/sun/nio/ch/WindowsSelectorImpl.java Mon Oct 13 14:45:27 2008 -0700
@@ -1,5 +1,5 @@
/*
- * Copyright 2002-2008 Sun Microsystems, Inc. All Rights Reserved.
+ * Copyright 2002-2007 Sun Microsystems, Inc. All Rights Reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -80,6 +80,9 @@
// File descriptors corresponding to source and sink
private final int wakeupSourceFd, wakeupSinkFd;
+ // Lock for close cleanup
+ private Object closeLock = new Object();
+
// Maps file descriptors to their indices in pollArray
private final static class FdMap extends HashMap<Integer, MapEntry> {
static final long serialVersionUID = 0L;
@@ -473,42 +476,48 @@
}
protected void implClose() throws IOException {
- if (channelArray != null) {
- if (pollWrapper != null) {
- // prevent further wakeup
- synchronized (interruptLock) {
- interruptTriggered = true;
- }
- wakeupPipe.sink().close();
- wakeupPipe.source().close();
- for(int i = 1; i < totalChannels; i++) { // Deregister channels
- if (i % MAX_SELECTABLE_FDS != 0) { // skip wakeupEvent
- deregister(channelArray[i]);
- SelectableChannel selch = channelArray[i].channel();
- if (!selch.isOpen() && !selch.isRegistered())
- ((SelChImpl)selch).kill();
+ synchronized (closeLock) {
+ if (channelArray != null) {
+ if (pollWrapper != null) {
+ // prevent further wakeup
+ synchronized (interruptLock) {
+ interruptTriggered = true;
}
- }
- pollWrapper.free();
- pollWrapper = null;
- selectedKeys = null;
- channelArray = null;
- threads.clear();
- // Call startThreads. All remaining helper threads now exit,
- // since threads.size() = 0;
- startLock.startThreads();
+ wakeupPipe.sink().close();
+ wakeupPipe.source().close();
+ for(int i = 1; i < totalChannels; i++) { // Deregister channels
+ if (i % MAX_SELECTABLE_FDS != 0) { // skip wakeupEvent
+ deregister(channelArray[i]);
+ SelectableChannel selch = channelArray[i].channel();
+ if (!selch.isOpen() && !selch.isRegistered())
+ ((SelChImpl)selch).kill();
+ }
+ }
+ pollWrapper.free();
+ pollWrapper = null;
+ selectedKeys = null;
+ channelArray = null;
+ threads.clear();
+ // Call startThreads. All remaining helper threads now exit,
+ // since threads.size() = 0;
+ startLock.startThreads();
+ }
}
}
}
protected void implRegister(SelectionKeyImpl ski) {
- growIfNeeded();
- channelArray[totalChannels] = ski;
- ski.setIndex(totalChannels);
- fdMap.put(ski);
- keys.add(ski);
- pollWrapper.addEntry(totalChannels, ski);
- totalChannels++;
+ synchronized (closeLock) {
+ if (pollWrapper == null)
+ throw new ClosedSelectorException();
+ growIfNeeded();
+ channelArray[totalChannels] = ski;
+ ski.setIndex(totalChannels);
+ fdMap.put(ski);
+ keys.add(ski);
+ pollWrapper.addEntry(totalChannels, ski);
+ totalChannels++;
+ }
}
private void growIfNeeded() {
@@ -554,7 +563,11 @@
}
void putEventOps(SelectionKeyImpl sk, int ops) {
- pollWrapper.putEventOps(sk.getIndex(), ops);
+ synchronized (closeLock) {
+ if (pollWrapper == null)
+ throw new ClosedSelectorException();
+ pollWrapper.putEventOps(sk.getIndex(), ops);
+ }
}
public Selector wakeup() {
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/java/nio/channels/Selector/CloseThenRegister.java Mon Oct 13 14:45:27 2008 -0700
@@ -0,0 +1,48 @@
+/*
+ * Copyright 2008 Sun Microsystems, Inc. All Rights Reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Sun Microsystems, Inc., 4150 Network Circle, Santa Clara,
+ * CA 95054 USA or visit www.sun.com if you need additional information or
+ * have any questions.
+ */
+
+/* @test
+ * @bug 5025260
+ * @summary ClosedSelectorException is expected when register after close
+ */
+
+import java.net.*;
+import java.nio.channels.*;
+
+public class CloseThenRegister {
+
+ public static void main (String [] args) throws Exception {
+ try {
+ Selector s = Selector.open();
+ s.close();
+ ServerSocketChannel c = ServerSocketChannel.open();
+ c.socket().bind(new InetSocketAddress(40000));
+ c.configureBlocking(false);
+ c.register(s, SelectionKey.OP_ACCEPT);
+ } catch (ClosedSelectorException cse) {
+ return;
+ }
+ throw new RuntimeException("register after close does not cause CSE!");
+ }
+
+}