# HG changeset patch # User alanb # Date 1570004178 -3600 # Node ID b25362cec8cef53f0682530855f9ce106a43d595 # Parent 1181f58f30e2b33d7055723eee9d890f1ae9fd11 8231603: (se) Selector implementations do not need to use cancelledKeys Reviewed-by: chegar, bpb diff -r 1181f58f30e2 -r b25362cec8ce src/java.base/share/classes/java/nio/channels/spi/AbstractSelectionKey.java --- a/src/java.base/share/classes/java/nio/channels/spi/AbstractSelectionKey.java Wed Oct 02 08:27:17 2019 +0200 +++ b/src/java.base/share/classes/java/nio/channels/spi/AbstractSelectionKey.java Wed Oct 02 09:16:18 2019 +0100 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000, 2013, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2000, 2019, 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 @@ -25,8 +25,13 @@ package java.nio.channels.spi; -import java.nio.channels.*; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.VarHandle; +import java.nio.channels.SelectionKey; +import java.nio.channels.Selector; +import sun.nio.ch.SelectionKeyImpl; +import sun.nio.ch.SelectorImpl; /** * Base implementation class for selection keys. @@ -41,20 +46,29 @@ public abstract class AbstractSelectionKey extends SelectionKey { + private static final VarHandle INVALID; + static { + try { + MethodHandles.Lookup l = MethodHandles.lookup(); + INVALID = l.findVarHandle(AbstractSelectionKey.class, "invalid", boolean.class); + } catch (Exception e) { + throw new InternalError(e); + } + } /** * Initializes a new instance of this class. */ protected AbstractSelectionKey() { } - private volatile boolean valid = true; + private volatile boolean invalid; public final boolean isValid() { - return valid; + return !invalid; } void invalidate() { // package-private - valid = false; + invalid = true; } /** @@ -64,13 +78,14 @@ * selector's cancelled-key set while synchronized on that set.

*/ public final void cancel() { - // Synchronizing "this" to prevent this key from getting canceled - // multiple times by different threads, which might cause race - // condition between selector's select() and channel's close(). - synchronized (this) { - if (valid) { - valid = false; - ((AbstractSelector)selector()).cancel(this); + boolean changed = (boolean) INVALID.compareAndSet(this, false, true); + if (changed) { + Selector sel = selector(); + if (sel instanceof SelectorImpl) { + // queue cancelled key directly + ((SelectorImpl) sel).cancel((SelectionKeyImpl) this); + } else { + ((AbstractSelector) sel).cancel(this); } } } diff -r 1181f58f30e2 -r b25362cec8ce src/java.base/share/classes/java/nio/channels/spi/AbstractSelector.java --- a/src/java.base/share/classes/java/nio/channels/spi/AbstractSelector.java Wed Oct 02 08:27:17 2019 +0200 +++ b/src/java.base/share/classes/java/nio/channels/spi/AbstractSelector.java Wed Oct 02 09:16:18 2019 +0100 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000, 2017, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2000, 2019, 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 @@ -26,12 +26,14 @@ package java.nio.channels.spi; import java.io.IOException; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.VarHandle; import java.nio.channels.SelectionKey; import java.nio.channels.Selector; import java.util.HashSet; import java.util.Set; import sun.nio.ch.Interruptible; -import java.util.concurrent.atomic.AtomicBoolean; +import sun.nio.ch.SelectorImpl; /** @@ -69,12 +71,23 @@ public abstract class AbstractSelector extends Selector { - - private final AtomicBoolean selectorOpen = new AtomicBoolean(true); + private static final VarHandle CLOSED; + static { + try { + MethodHandles.Lookup l = MethodHandles.lookup(); + CLOSED = l.findVarHandle(AbstractSelector.class, "closed", boolean.class); + } catch (Exception e) { + throw new InternalError(e); + } + } + private volatile boolean closed; // The provider that created this selector private final SelectorProvider provider; + // cancelled-key, not used by the JDK Selector implementations + private final Set cancelledKeys; + /** * Initializes a new instance of this class. * @@ -83,10 +96,14 @@ */ protected AbstractSelector(SelectorProvider provider) { this.provider = provider; + if (this instanceof SelectorImpl) { + // not used in JDK Selector implementations + this.cancelledKeys = Set.of(); + } else { + this.cancelledKeys = new HashSet<>(); + } } - private final Set cancelledKeys = new HashSet(); - void cancel(SelectionKey k) { // package-private synchronized (cancelledKeys) { cancelledKeys.add(k); @@ -105,10 +122,10 @@ * If an I/O error occurs */ public final void close() throws IOException { - boolean open = selectorOpen.getAndSet(false); - if (!open) - return; - implCloseSelector(); + boolean changed = (boolean) CLOSED.compareAndSet(this, false, true); + if (changed) { + implCloseSelector(); + } } /** @@ -130,7 +147,7 @@ protected abstract void implCloseSelector() throws IOException; public final boolean isOpen() { - return selectorOpen.get(); + return !closed; } /** diff -r 1181f58f30e2 -r b25362cec8ce src/java.base/share/classes/sun/nio/ch/SelectorImpl.java --- a/src/java.base/share/classes/sun/nio/ch/SelectorImpl.java Wed Oct 02 08:27:17 2019 +0200 +++ b/src/java.base/share/classes/sun/nio/ch/SelectorImpl.java Wed Oct 02 09:16:18 2019 +0100 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000, 2018, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2000, 2019, 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 @@ -33,7 +33,9 @@ import java.nio.channels.spi.AbstractSelectableChannel; import java.nio.channels.spi.AbstractSelector; import java.nio.channels.spi.SelectorProvider; +import java.util.ArrayDeque; import java.util.Collections; +import java.util.Deque; import java.util.HashSet; import java.util.Iterator; import java.util.Objects; @@ -46,7 +48,7 @@ * Base Selector implementation class. */ -abstract class SelectorImpl +public abstract class SelectorImpl extends AbstractSelector { // The set of keys registered with this Selector @@ -59,6 +61,9 @@ private final Set publicKeys; // Immutable private final Set publicSelectedKeys; // Removal allowed, but not addition + // pending cancelled keys for deregistration + private final Deque cancelledKeys = new ArrayDeque<>(); + // used to check for reentrancy private boolean inSelect; @@ -239,33 +244,36 @@ protected abstract void implDereg(SelectionKeyImpl ski) throws IOException; /** - * Invoked by selection operations to process the cancelled-key set + * Queue a cancelled key for the next selection operation + */ + public void cancel(SelectionKeyImpl ski) { + synchronized (cancelledKeys) { + cancelledKeys.addLast(ski); + } + } + + /** + * Invoked by selection operations to process the cancelled keys */ protected final void processDeregisterQueue() throws IOException { assert Thread.holdsLock(this); assert Thread.holdsLock(publicSelectedKeys); - Set cks = cancelledKeys(); - synchronized (cks) { - if (!cks.isEmpty()) { - Iterator i = cks.iterator(); - while (i.hasNext()) { - SelectionKeyImpl ski = (SelectionKeyImpl)i.next(); - i.remove(); - - // remove the key from the selector - implDereg(ski); + synchronized (cancelledKeys) { + SelectionKeyImpl ski; + while ((ski = cancelledKeys.pollFirst()) != null) { + // remove the key from the selector + implDereg(ski); - selectedKeys.remove(ski); - keys.remove(ski); + selectedKeys.remove(ski); + keys.remove(ski); - // remove from channel's key set - deregister(ski); + // remove from channel's key set + deregister(ski); - SelectableChannel ch = ski.channel(); - if (!ch.isOpen() && !ch.isRegistered()) - ((SelChImpl)ch).kill(); - } + SelectableChannel ch = ski.channel(); + if (!ch.isOpen() && !ch.isRegistered()) + ((SelChImpl)ch).kill(); } } }