I'm checking this in.
According to the spec, SelectionKey is supposed to be safe when used
by multiple threads. And, in PR 23682, we were seeing exceptions that
occurred because changes to the interestOps were not synchronized
against concurrent use by the selector impl.
This fix is the minimum necessary to achieve correctness.
Our select implementation is still pretty bad. We could do much less
synchronization, I think, if we designed our data structures a bit
differently. For instance we could keep a poll(2)-ready data
structure attached to the selector and update it in response to
requests. I think this would be much more efficient, at least for
those VMs that can use the native code here.
Tom
2006-07-31 Tom Tromey <[EMAIL PROTECTED]>
PR libgcj/23682:
* java/nio/channels/SelectionKey.java (attach): Now synchronized.
(attachment): Likewise.
* java/nio/channels/spi/AbstractSelectionKey.java (cancel): Now
synchronized.
(isValid): Likewise.
* gnu/java/nio/SelectionKeyImpl.java (impl): Now final
(ch): Likewise.
(interestOps): Synchronize.
(readyOps): Likewise.
* gnu/java/nio/SelectorImpl.java (register): Synchronize around
interestOps call.
Index: gnu/java/nio/SelectionKeyImpl.java
===================================================================
RCS file: /cvsroot/classpath/classpath/gnu/java/nio/SelectionKeyImpl.java,v
retrieving revision 1.9
diff -u -r1.9 SelectionKeyImpl.java
--- gnu/java/nio/SelectionKeyImpl.java 2 Jul 2005 20:32:13 -0000 1.9
+++ gnu/java/nio/SelectionKeyImpl.java 31 Jul 2006 22:03:11 -0000
@@ -1,5 +1,5 @@
/* SelectionKeyImpl.java --
- Copyright (C) 2002, 2003 Free Software Foundation, Inc.
+ Copyright (C) 2002, 2003, 2006 Free Software Foundation, Inc.
This file is part of GNU Classpath.
@@ -47,8 +47,8 @@
{
private int readyOps;
private int interestOps;
- private SelectorImpl impl;
- SelectableChannel ch;
+ private final SelectorImpl impl;
+ final SelectableChannel ch;
public SelectionKeyImpl (SelectableChannel ch, SelectorImpl impl)
{
@@ -61,7 +61,7 @@
return ch;
}
- public int readyOps ()
+ public synchronized int readyOps ()
{
if (!isValid())
throw new CancelledKeyException();
@@ -69,7 +69,7 @@
return readyOps;
}
- public SelectionKey readyOps (int ops)
+ public synchronized SelectionKey readyOps (int ops)
{
if (!isValid())
throw new CancelledKeyException();
@@ -83,15 +83,21 @@
if (!isValid())
throw new CancelledKeyException();
- return interestOps;
+ synchronized (impl.selectedKeys())
+ {
+ return interestOps;
+ }
}
public SelectionKey interestOps (int ops)
{
if (!isValid())
throw new CancelledKeyException();
-
- interestOps = ops;
+
+ synchronized (impl.selectedKeys())
+ {
+ interestOps = ops;
+ }
return this;
}
Index: gnu/java/nio/SelectorImpl.java
===================================================================
RCS file: /cvsroot/classpath/classpath/gnu/java/nio/SelectorImpl.java,v
retrieving revision 1.22
diff -u -r1.22 SelectorImpl.java
--- gnu/java/nio/SelectorImpl.java 14 May 2006 03:34:54 -0000 1.22
+++ gnu/java/nio/SelectorImpl.java 31 Jul 2006 22:03:11 -0000
@@ -1,5 +1,5 @@
/* SelectorImpl.java --
- Copyright (C) 2002, 2003, 2004 Free Software Foundation, Inc.
+ Copyright (C) 2002, 2003, 2004, 2006 Free Software Foundation, Inc.
This file is part of GNU Classpath.
@@ -387,10 +387,11 @@
synchronized (keys)
{
keys.add (result);
+
+ result.interestOps (ops);
+ result.attach (att);
}
- result.interestOps (ops);
- result.attach (att);
return result;
}
}
Index: java/nio/channels/SelectionKey.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/nio/channels/SelectionKey.java,v
retrieving revision 1.8
diff -u -r1.8 SelectionKey.java
--- java/nio/channels/SelectionKey.java 2 Jul 2005 20:32:40 -0000 1.8
+++ java/nio/channels/SelectionKey.java 31 Jul 2006 22:03:13 -0000
@@ -1,5 +1,5 @@
/* SelectionKey.java --
- Copyright (C) 2002 Free Software Foundation, Inc.
+ Copyright (C) 2002, 2006 Free Software Foundation, Inc.
This file is part of GNU Classpath.
@@ -60,7 +60,7 @@
/**
* Attaches obj to the key and returns the old attached object.
*/
- public final Object attach(Object obj)
+ public final synchronized Object attach(Object obj)
{
Object old = attached;
attached = obj;
@@ -70,7 +70,7 @@
/**
* Returns the object attached to the key.
*/
- public final Object attachment()
+ public final synchronized Object attachment()
{
return attached;
}
Index: java/nio/channels/spi/AbstractSelectionKey.java
===================================================================
RCS file:
/cvsroot/classpath/classpath/java/nio/channels/spi/AbstractSelectionKey.java,v
retrieving revision 1.9
diff -u -r1.9 AbstractSelectionKey.java
--- java/nio/channels/spi/AbstractSelectionKey.java 2 Jul 2005 20:32:40
-0000 1.9
+++ java/nio/channels/spi/AbstractSelectionKey.java 31 Jul 2006 22:03:13
-0000
@@ -1,5 +1,5 @@
/* AbstractSelectionKey.java --
- Copyright (C) 2002, 2003, 2004 Free Software Foundation, Inc.
+ Copyright (C) 2002, 2003, 2004, 2006 Free Software Foundation, Inc.
This file is part of GNU Classpath.
@@ -57,7 +57,7 @@
/**
* Cancels this key.
*/
- public final void cancel()
+ public final synchronized void cancel()
{
if (isValid())
{
@@ -71,7 +71,7 @@
*
* @return true if this key is valid, false otherwise
*/
- public final boolean isValid()
+ public final synchronized boolean isValid()
{
return ! cancelled;
}
