Alpha Patch: Selector.select() Busted in 3.4

[Mohan Embar <gnustuff at thisiscool dot com>]

Hi Michael (and whoever else is interested),

Although we got Selector.select() working at one point in time,
it got busted before the 3.4 freeze because of this patch:

http://gcc.gnu.org/ml/java-patches/2004-q1/msg00052.html

I don't think the above patch itself is at fault, but the
problem is Sun's confusing specs.

Here is an example of what's wrong. If you look at ServerSocket.accept(),
Sun's JavaDoc says that an IllegalBlockingModeException should be thrown
if the socket has an associated channel and it is non-blocking.

But here is an example of how one sets up a Selector.select() call
(from my NetTest.java):

      ServerSocketChannel ssc = ServerSocketChannel.open();
      arssc[nCurIndex++] = ssc;
      ssc.configureBlocking(false);
      ssc.socket().bind(new InetSocketAddress(nPort));
      ssc.register(m_sel, SelectionKey.OP_ACCEPT);
      // register interest in ACCEPT

This is consistent with this:

http://www.onjava.com/pub/a/onjava/2002/10/02/javanio.html?page=last

(see that section on Multiplexed I/O)

However, ServerSocket.implAccept() then bombs because its associated
ServerSocketChannel has blocking = false.

I think the real specification to be followed is that of ServerSocketChannel.accept(),
which overrides the specification of ServerSocket.accept() by saying that if blocking
is false, the method should return immediately. But, as far as I can see, that means
that in our implementation, ServerSocket.implAccept() needs to have some way of detecting
that it is being called via ServerSocketChannel.accept() and ignoring the blocking flag.

The same applies for connect(), etc.

Here is an alpha patch I threw together to attempt to fix this. It's not
breathtaking, but it makes NetTest pass again. While I was in here, I also fixed
a couple of other minor things.

I did this against the 3.4 branch. I haven't done a ChangeLog because I wanted
to see if you or anyone freaked out before I spent anymore time on this.
Like I said, it's not amazing, but I think the only elegant way of solving the
problem would be messing around with inner classes and stuff like that, and I
was too lazy to be overly-elegant about this.

-- Mohan
http://www.thisiscool.com/
http://www.animalsong.org/

Index: gnu/java/nio/ServerSocketChannelImpl.java
===================================================================
RCS file: /cvs/gcc/gcc/libjava/gnu/java/nio/ServerSocketChannelImpl.java,v
retrieving revision 1.11
diff -u -2 -r1.11 ServerSocketChannelImpl.java
--- gnu/java/nio/ServerSocketChannelImpl.java	7 Jan 2004 16:51:49 -0000	1.11
+++ gnu/java/nio/ServerSocketChannelImpl.java	29 Jan 2004 05:15:50 -0000
@@ -57,4 +57,5 @@
   private NIOServerSocket serverSocket;
   private boolean connected;
+  private boolean inIOOperation;
 
   protected ServerSocketChannelImpl (SelectorProvider provider)
@@ -70,4 +71,16 @@
     return serverSocket.getPlainSocketImpl().getNativeFD();
   }
+  
+  /**
+   * Indicates whether we're in the middle of an I/O operation.
+   * This indicator is set and reset in tandem with our
+   * <code>begin()</code> and <code>end()</code> calls, but
+   * we can't set this flag in those methods because we can't
+   * add a new protected field to <code>AbstractInterruptibleChannel</code>.
+   */
+  public final boolean isInIOOperation()
+  {
+    return inIOOperation;
+  }
  
   public void finalizer()
@@ -108,4 +121,6 @@
     try
       {
+        begin();
+        inIOOperation = true;
         NIOSocket socket = (NIOSocket) serverSocket.accept();
         completed = true;
@@ -118,4 +133,5 @@
     finally
       {
+        inIOOperation = false;
         end (completed);
       }
Index: gnu/java/nio/SocketChannelImpl.java
===================================================================
RCS file: /cvs/gcc/gcc/libjava/gnu/java/nio/SocketChannelImpl.java,v
retrieving revision 1.15
diff -u -2 -r1.15 SocketChannelImpl.java
--- gnu/java/nio/SocketChannelImpl.java	7 Jan 2004 16:51:49 -0000	1.15
+++ gnu/java/nio/SocketChannelImpl.java	29 Jan 2004 05:15:50 -0000
@@ -67,4 +67,5 @@
   private NIOSocket socket;
   private boolean connectionPending;
+  private boolean inIOOperation;
 
   SocketChannelImpl (SelectorProvider provider)
@@ -110,4 +111,16 @@
   }
 
+  /**
+   * Indicates whether we're in the middle of an I/O operation.
+   * This indicator is set and reset in tandem with our
+   * <code>begin()</code> and <code>end()</code> calls, but
+   * we can't set this flag in those methods because we can't
+   * add a new protected field to <code>AbstractInterruptibleChannel</code>.
+   */
+  public final boolean isInIOOperation()
+  {
+    return inIOOperation;
+  }
+ 
   protected void implCloseSelectableChannel () throws IOException
   {
@@ -137,21 +150,30 @@
       throw new UnresolvedAddressException();
     
-    if (isBlocking())
-      {
-        // Do blocking connect.
-        socket.connect (remote);
-        return true;
-      }
-
-    // Do non-blocking connect.
     try
       {
-        socket.connect (remote, NIOConstants.DEFAULT_TIMEOUT);
-        return true;
+        inIOOperation = true;
+        
+        if (isBlocking())
+          {
+            // Do blocking connect.
+            socket.connect (remote);
+            return true;
+          }
+
+        // Do non-blocking connect.
+        try
+          {
+            socket.connect (remote, NIOConstants.DEFAULT_TIMEOUT);
+            return true;
+          }
+        catch (SocketTimeoutException e)
+          {
+            connectionPending = true;
+            return false;
+          }
       }
-    catch (SocketTimeoutException e)
+    finally
       {
-        connectionPending = true;
-        return false;
+        inIOOperation = false;
       }
   }
@@ -239,4 +261,5 @@
       {
         begin();
+        inIOOperation = true;
         readBytes = input.read (data, offset, len);
         completed = true;
@@ -245,4 +268,5 @@
       {
         end (completed);
+        inIOOperation = false;
       }
 
@@ -302,5 +326,18 @@
 
     OutputStream output = socket.getOutputStream();
-    output.write (data, offset, len);
+    boolean completed = false;
+
+    try
+      {
+        begin();
+        inIOOperation = true;
+        output.write (data, offset, len);
+        completed = true;
+      }
+    finally
+      {
+        end (completed);
+        inIOOperation = false;
+      }
 
     if (src.hasArray())
Index: java/net/ServerSocket.java
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/net/ServerSocket.java,v
retrieving revision 1.34
diff -u -2 -r1.34 ServerSocket.java
--- java/net/ServerSocket.java	7 Jan 2004 16:37:45 -0000	1.34
+++ java/net/ServerSocket.java	29 Jan 2004 05:15:53 -0000
@@ -40,4 +40,6 @@
 
 import gnu.java.net.PlainSocketImpl;
+import gnu.java.nio.ServerSocketChannelImpl;
+
 import java.io.IOException;
 import java.nio.channels.IllegalBlockingModeException;
@@ -347,5 +349,6 @@
     
     if (getChannel() != null
-        && !getChannel().isBlocking())
+        && !getChannel().isBlocking ()
+        && !((ServerSocketChannelImpl) getChannel()).isInIOOperation ())
       throw new IllegalBlockingModeException();
 	    
Index: java/net/Socket.java
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/net/Socket.java,v
retrieving revision 1.33
diff -u -2 -r1.33 Socket.java
--- java/net/Socket.java	9 Dec 2003 15:39:23 -0000	1.33
+++ java/net/Socket.java	29 Jan 2004 05:15:54 -0000
@@ -40,4 +40,6 @@
 
 import gnu.java.net.PlainSocketImpl;
+import gnu.java.nio.SocketChannelImpl;
+
 import java.io.InputStream;
 import java.io.IOException;
@@ -422,5 +424,6 @@
 
     if (getChannel() != null
-        && !getChannel().isBlocking ())
+        && !getChannel().isBlocking ()
+        && !((SocketChannelImpl) getChannel()).isInIOOperation ())
       throw new IllegalBlockingModeException ();
   
Index: java/nio/channels/spi/AbstractSelectableChannel.java
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/nio/channels/spi/AbstractSelectableChannel.java,v
retrieving revision 1.7
diff -u -2 -r1.7 AbstractSelectableChannel.java
--- java/nio/channels/spi/AbstractSelectableChannel.java	7 Jan 2004 16:51:49 -0000	1.7
+++ java/nio/channels/spi/AbstractSelectableChannel.java	29 Jan 2004 05:15:55 -0000
@@ -81,6 +81,9 @@
     synchronized (blockingLock())
       {
-        implConfigureBlocking(blocking);
-        this.blocking = blocking;
+        if (this.blocking != blocking)
+          {
+            implConfigureBlocking(blocking);
+            this.blocking = blocking;
+          }
       }