8231603: (se) Selector implementations do not need to use cancelledKeys
Reviewed-by: chegar, bpb
--- 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. </p>
*/
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);
}
}
}
--- 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<SelectionKey> 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<SelectionKey> cancelledKeys = new HashSet<SelectionKey>();
-
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;
}
/**
--- 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<SelectionKey> publicKeys; // Immutable
private final Set<SelectionKey> publicSelectedKeys; // Removal allowed, but not addition
+ // pending cancelled keys for deregistration
+ private final Deque<SelectionKeyImpl> 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<SelectionKey> cks = cancelledKeys();
- synchronized (cks) {
- if (!cks.isEmpty()) {
- Iterator<SelectionKey> 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();
}
}
}