diff --git a/src/java.base/linux/classes/sun/nio/ch/EPollSelectorImpl.java b/src/java.base/linux/classes/sun/nio/ch/EPollSelectorImpl.java index cb55932fd7e..a07a927e635 100644 --- a/src/java.base/linux/classes/sun/nio/ch/EPollSelectorImpl.java +++ b/src/java.base/linux/classes/sun/nio/ch/EPollSelectorImpl.java @@ -26,7 +26,6 @@ package sun.nio.ch; import java.io.IOException; -import java.nio.channels.ClosedSelectorException; import java.nio.channels.SelectionKey; import java.nio.channels.Selector; import java.nio.channels.spi.SelectorProvider; @@ -92,11 +91,6 @@ class EPollSelectorImpl extends SelectorImpl { EPoll.ctl(epfd, EPOLL_CTL_ADD, eventfd.efd(), EPOLLIN); } - private void ensureOpen() { - if (!isOpen()) - throw new ClosedSelectorException(); - } - @Override protected int doSelect(Consumer action, long timeout) throws IOException @@ -243,7 +237,6 @@ class EPollSelectorImpl extends SelectorImpl { @Override public void setEventOps(SelectionKeyImpl ski) { - ensureOpen(); synchronized (updateLock) { updateKeys.addLast(ski); } diff --git a/src/java.base/macosx/classes/sun/nio/ch/KQueueSelectorImpl.java b/src/java.base/macosx/classes/sun/nio/ch/KQueueSelectorImpl.java index 6c84984f515..6abc9e31879 100644 --- a/src/java.base/macosx/classes/sun/nio/ch/KQueueSelectorImpl.java +++ b/src/java.base/macosx/classes/sun/nio/ch/KQueueSelectorImpl.java @@ -26,7 +26,6 @@ package sun.nio.ch; import java.io.IOException; -import java.nio.channels.ClosedSelectorException; import java.nio.channels.SelectionKey; import java.nio.channels.Selector; import java.nio.channels.spi.SelectorProvider; @@ -97,11 +96,6 @@ class KQueueSelectorImpl extends SelectorImpl { KQueue.register(kqfd, fd0, EVFILT_READ, EV_ADD); } - private void ensureOpen() { - if (!isOpen()) - throw new ClosedSelectorException(); - } - @Override protected int doSelect(Consumer action, long timeout) throws IOException @@ -285,7 +279,6 @@ class KQueueSelectorImpl extends SelectorImpl { @Override public void setEventOps(SelectionKeyImpl ski) { - ensureOpen(); synchronized (updateLock) { updateKeys.addLast(ski); } diff --git a/src/java.base/share/classes/java/nio/channels/spi/AbstractSelectableChannel.java b/src/java.base/share/classes/java/nio/channels/spi/AbstractSelectableChannel.java index 558158cc852..4b342603caf 100644 --- a/src/java.base/share/classes/java/nio/channels/spi/AbstractSelectableChannel.java +++ b/src/java.base/share/classes/java/nio/channels/spi/AbstractSelectableChannel.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2000, 2024, Oracle and/or its affiliates. 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 @@ -137,6 +137,8 @@ public abstract class AbstractSelectableChannel void removeKey(SelectionKey k) { // package-private synchronized (keyLock) { + if (keys == null) + return; for (int i = 0; i < keys.length; i++) if (keys[i] == k) { keys[i] = null; @@ -233,7 +235,7 @@ public abstract class AbstractSelectableChannel k.interestOps(ops); } else { // New registration - k = ((AbstractSelector)sel).register(this, ops, att); + k = ((AbstractSelector) sel).register(this, ops, att); addKey(k); } return k; diff --git a/src/java.base/share/classes/sun/nio/ch/SelectorImpl.java b/src/java.base/share/classes/sun/nio/ch/SelectorImpl.java index bb0d603790f..de7a1f98daf 100644 --- a/src/java.base/share/classes/sun/nio/ch/SelectorImpl.java +++ b/src/java.base/share/classes/sun/nio/ch/SelectorImpl.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2000, 2024, Oracle and/or its affiliates. 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 @@ -188,11 +188,11 @@ public abstract class SelectorImpl // Deregister channels Iterator i = keys.iterator(); while (i.hasNext()) { - SelectionKeyImpl ski = (SelectionKeyImpl)i.next(); + SelectionKeyImpl ski = (SelectionKeyImpl) i.next(); deregister(ski); SelectableChannel selch = ski.channel(); if (!selch.isOpen() && !selch.isRegistered()) - ((SelChImpl)selch).kill(); + ((SelChImpl) selch).kill(); selectedKeys.remove(ski); i.remove(); } @@ -221,13 +221,14 @@ public abstract class SelectorImpl keys.add(k); try { k.interestOps(ops); - } catch (ClosedSelectorException e) { + } catch (CancelledKeyException e) { + // key observed and cancelled. Okay to return a cancelled key. + } + if (!isOpen()) { assert ch.keyFor(this) == null; keys.remove(k); k.cancel(); - throw e; - } catch (CancelledKeyException e) { - // key observed and cancelled. Okay to return a cancelled key. + throw new ClosedSelectorException(); } return k; } @@ -277,7 +278,7 @@ public abstract class SelectorImpl SelectableChannel ch = ski.channel(); if (!ch.isOpen() && !ch.isRegistered()) - ((SelChImpl)ch).kill(); + ((SelChImpl) ch).kill(); } } } diff --git a/src/java.base/unix/classes/sun/nio/ch/PollSelectorImpl.java b/src/java.base/unix/classes/sun/nio/ch/PollSelectorImpl.java index a3bff8d1e65..d29eacb81c1 100644 --- a/src/java.base/unix/classes/sun/nio/ch/PollSelectorImpl.java +++ b/src/java.base/unix/classes/sun/nio/ch/PollSelectorImpl.java @@ -231,7 +231,6 @@ class PollSelectorImpl extends SelectorImpl { @Override public void setEventOps(SelectionKeyImpl ski) { - ensureOpen(); synchronized (updateLock) { updateKeys.addLast(ski); } diff --git a/src/java.base/windows/classes/sun/nio/ch/WEPollSelectorImpl.java b/src/java.base/windows/classes/sun/nio/ch/WEPollSelectorImpl.java index bbcfab9bbf1..f2bff88675f 100644 --- a/src/java.base/windows/classes/sun/nio/ch/WEPollSelectorImpl.java +++ b/src/java.base/windows/classes/sun/nio/ch/WEPollSelectorImpl.java @@ -28,7 +28,6 @@ package sun.nio.ch; import java.io.FileDescriptor; import java.io.IOException; import java.nio.ByteBuffer; -import java.nio.channels.ClosedSelectorException; import java.nio.channels.Pipe; import java.nio.channels.SelectionKey; import java.nio.channels.Selector; @@ -89,11 +88,6 @@ class WEPollSelectorImpl extends SelectorImpl { WEPoll.ctl(eph, EPOLL_CTL_ADD, fd0Val, WEPoll.EPOLLIN); } - private void ensureOpen() { - if (!isOpen()) - throw new ClosedSelectorException(); - } - @Override protected int doSelect(Consumer action, long timeout) throws IOException @@ -228,7 +222,6 @@ class WEPollSelectorImpl extends SelectorImpl { @Override public void setEventOps(SelectionKeyImpl ski) { - ensureOpen(); synchronized (updateLock) { updateKeys.addLast(ski); } diff --git a/src/java.base/windows/classes/sun/nio/ch/WindowsSelectorImpl.java b/src/java.base/windows/classes/sun/nio/ch/WindowsSelectorImpl.java index 977a1653fdd..eb681bc5ab9 100644 --- a/src/java.base/windows/classes/sun/nio/ch/WindowsSelectorImpl.java +++ b/src/java.base/windows/classes/sun/nio/ch/WindowsSelectorImpl.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2002, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2002, 2024, Oracle and/or its affiliates. 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 @@ -606,7 +606,6 @@ class WindowsSelectorImpl extends SelectorImpl { @Override public void setEventOps(SelectionKeyImpl ski) { - ensureOpen(); synchronized (updateLock) { updateKeys.addLast(ski); } diff --git a/test/jdk/java/nio/channels/Selector/RaceUpdatesAndClose.java b/test/jdk/java/nio/channels/Selector/RaceUpdatesAndClose.java new file mode 100644 index 00000000000..8173d3d761f --- /dev/null +++ b/test/jdk/java/nio/channels/Selector/RaceUpdatesAndClose.java @@ -0,0 +1,122 @@ +/* + * Copyright (c) 2024, Oracle and/or its affiliates. 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 Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @bug 8336339 + * @summary Race registration and selection key updates with Selector.close + * @run junit RaceUpdatesAndClose + */ + +import java.nio.channels.CancelledKeyException; +import java.nio.channels.ClosedSelectorException; +import java.nio.channels.DatagramChannel; +import java.nio.channels.SelectionKey; +import java.nio.channels.Selector; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Phaser; + +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.RepeatedTest; + +class RaceUpdatesAndClose { + private static ExecutorService executor; + + @BeforeAll + static void setup() throws Exception { + executor = Executors.newFixedThreadPool(2); + } + + @AfterAll + static void finish() { + executor.shutdown(); + } + + /** + * Race SelectableChannel.register and Selector.close. + */ + @RepeatedTest(100) + void raceRegisterAndClose() throws Exception { + try (Selector sel = Selector.open(); + DatagramChannel dc = DatagramChannel.open()) { + + dc.configureBlocking(false); + + Phaser phaser = new Phaser(2); + + // register + var task1 = executor.submit(() -> { + phaser.arriveAndAwaitAdvance(); + try { + dc.register(sel, SelectionKey.OP_READ); + } catch (ClosedSelectorException e) { } + return null; + }); + + // close selector + var task2 = executor.submit(() -> { + phaser.arriveAndAwaitAdvance(); + sel.close(); + return null; + }); + + task1.get(); + task2.get(); + } + } + + /** + * Race SelectionKey.interestOps and Selector.close. + */ + @RepeatedTest(100) + void raceInterestOpsAndClose() throws Exception { + try (Selector sel = Selector.open(); + DatagramChannel dc = DatagramChannel.open()) { + + dc.configureBlocking(false); + SelectionKey key = dc.register(sel, SelectionKey.OP_READ); + + Phaser phaser = new Phaser(2); + + // interestOps + var task1 = executor.submit(() -> { + phaser.arriveAndAwaitAdvance(); + try { + key.interestOps(0); + } catch (CancelledKeyException e) { } + }); + + // close selector + var task2 = executor.submit(() -> { + phaser.arriveAndAwaitAdvance(); + sel.close(); + return null; + }); + + task1.get(); + task2.get(); + } + } +}