[kaffe] gnu.java.nio.VMSelector implementation

[Rei Odaira]

Dalibor Topic wrote:
> Rei Odaira wrote:
> > Hi,
> > 
> > I'm trying to run Tomcat 5 with clustering support on Kaffe/jthread.
> > 
> > The current implementation of gnu.java.nio.VMSelector#select()
> > seems to directly invoke a system call "select()", instead of KSELECT().
> > Since the system call is not aware of jthread, Kaffe can stop
> > indefinitely when java.nio.channels.Selector#select() is called
> > with timeout == 0 (blocking mode).
> > 
> > Below is a simple testcase.
> > 
> > I think the previous implementation, which uses KSELECT(), would be better,
> > although it has several serious bugs.
> > Any thoughts?
> 
> Konnichiwa Rei,
> 
> Thanks you very much for spotting this bug, and for the great bug 
> report! And thanks to Guilhem for checking in a fix so rapidly and 
> fixing my mistake! :)

Thanks to Guilhem for fixing the issue!

But unfortunately, this is not the end of the problem.
The attached patch will fix the following five bugs
in the selector of Kaffe/jthread.

o The parameter "e" of jthreadedSelect() can be NULL,
  which means an infinite timeout. In that case,
  time_milli should be NOTIMEOUT.

o We must not pass parameters "b", "c", and "d" of
  jthreadedSelect() directly to the system call select(),
  because they can be changed by select().

o We must set maxFd, readsPending, and writesPending properly
  in jthreadedSelect().

o helper_select() in gnu_java_nio_VMSelector.c retuns EINTR
  when the current thread is interrupted.
  However, EINTR == 4 in Linux, so we cannot distinguish
  the interrupted case from the one where the number of bits
  set in the bit masks is 4.

o java.nio.channels.spi.AbstractSelectableChannel#register()
  should throw IllegalBlockingModeException if the channel is
  in blocking mode.

Rei
-------------- next part --------------
Index: kaffe/kaffevm/systems/unix-jthreads/jthread.c
===================================================================
RCS file: /cvs/kaffe/kaffe/kaffe/kaffevm/systems/unix-jthreads/jthread.c,v
retrieving revision 1.130
diff -a -u -r1.130 jthread.c
--- kaffe/kaffevm/systems/unix-jthreads/jthread.c	20 Mar 2005 20:30:43 -0000	1.130
+++ kaffe/kaffevm/systems/unix-jthreads/jthread.c	3 Apr 2005 07:24:27 -0000
@@ -3325,29 +3325,64 @@
 	tval.tv_sec = 0;
 	tval.tv_usec = 0;
 
-	time_milli = e->tv_usec / 1000 + e->tv_sec * 1000;
+	if (e != NULL)
+		time_milli = e->tv_usec / 1000 + e->tv_sec * 1000;
+	else
+		time_milli = NOTIMEOUT;
 
 	intsDisable();
 
 	for (;;) {
-		if ((*out = select(a, b, c, d, &tval)) == -1) {
+		fd_set tmp_b, tmp_c, tmp_d;
+
+		FD_COPY(b, &tmp_b);
+		FD_COPY(c, &tmp_c);
+		FD_COPY(d, &tmp_d);
+
+		if ((*out = select(a, &tmp_b, &tmp_c, &tmp_d, &tval)) == -1) {
 			rc = errno;
 			break;
 		}
-		if ((*out == 0 && second_time) || *out != 0)
+		if ((*out == 0 && second_time) || *out != 0) {
+			FD_COPY(&tmp_b, b);
+			FD_COPY(&tmp_c, c);
+			FD_COPY(&tmp_d, d);
 			break;
+		}
 
 		if (time_milli != 0) {
+			int interrupted;
+
+			BLOCKED_ON_EXTERNAL(currentJThread);
+
+			if (a - 1 > maxFd)
+				maxFd = a - 1;
+
 			for (i=0;i<a;i++) {
-				if (b && FD_ISSET(i, b))
+				if (b && FD_ISSET(i, b)) {
+					FD_SET(i, &readsPending);
 					addWaitQThread(currentJThread, &readQ[i]);
-				if (c && FD_ISSET(i, c))
+				}
+				if (c && FD_ISSET(i, c)) {
+					FD_SET(i, &writesPending);
 					addWaitQThread(currentJThread, &writeQ[i]);
+				}
 			}
 
-			if (suspendOnQThread(currentJThread, NULL, time_milli)) {
+			interrupted = suspendOnQThread(currentJThread, NULL, time_milli);
+
+			for (i=0;i<a;i++) {
+				if (b && FD_ISSET(i, b))
+					FD_CLR(i, &readsPending);
+				if (c && FD_ISSET(i, c))
+					FD_CLR(i, &writesPending);
+			}
+			if (interrupted) {
 				rc = EINTR;
 				*out = 0;
+				FD_ZERO(b);
+				FD_ZERO(c);
+				FD_ZERO(d);
 				break;
 			}
 		}
Index: libraries/clib/nio/gnu_java_nio_VMSelector.c
===================================================================
RCS file: /cvs/kaffe/kaffe/libraries/clib/nio/gnu_java_nio_VMSelector.c,v
retrieving revision 1.5
diff -a -u -r1.5 gnu_java_nio_VMSelector.c
--- libraries/clib/nio/gnu_java_nio_VMSelector.c	1 Apr 2005 01:02:56 -0000	1.5
+++ libraries/clib/nio/gnu_java_nio_VMSelector.c	3 Apr 2005 07:24:27 -0000
@@ -166,7 +166,7 @@
 		/* Here we know we got EINTR. */
 		if ( (*env)->CallStaticBooleanMethod(env, thread_class, thread_interrupted) )
 		{
-			return EINTR;
+			return -2;
 		}
 
 		if (timeout)
@@ -249,7 +249,7 @@
 	result = helper_select (env, thread_class, thread_interrupted, max_fd + 1, &read_fds, &write_fds,
 								&except_fds, time_data);
 
-	if( result == EINTR ) {
+	if( result == -2 ) {
 		/* The behavior of JRE 1.4.1 is that no exception is thrown
 		 * when the thread is interrupted, but the thread's interrupt
 		 * status is set. Clear all of our select sets and return 0,
Index: libraries/javalib/java/nio/channels/spi/AbstractSelectableChannel.java
===================================================================
RCS file: /cvs/kaffe/kaffe/libraries/javalib/java/nio/channels/spi/AbstractSelectableChannel.java,v
retrieving revision 1.11
diff -a -u -r1.11 AbstractSelectableChannel.java
--- libraries/javalib/java/nio/channels/spi/AbstractSelectableChannel.java	3 Dec 2004 00:44:29 -0000	1.11
+++ libraries/javalib/java/nio/channels/spi/AbstractSelectableChannel.java	3 Apr 2005 07:24:28 -0000
@@ -43,6 +43,7 @@
 import java.nio.channels.SelectableChannel;
 import java.nio.channels.SelectionKey;
 import java.nio.channels.Selector;
+import java.nio.channels.IllegalBlockingModeException;
 import java.util.LinkedList;
 import java.util.ListIterator;
 
@@ -224,6 +225,9 @@
 
     synchronized (blockingLock())
       {
+	if (blocking)
+	  throw new IllegalBlockingModeException();
+
 	key = locate(selector);
 
 	if (key != null && key.isValid())