This is the mail archive of the java-patches@gcc.gnu.org mailing list for the Java project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Patch: RFA PR libgcj/15430 (read keeps blocking after socket closed).


New version of the patch attached.

Tested against the testcase in the PR and with make -k check in libjava + mauve with no regressions on I686-pc-linux-gnu (FC3). This patch also seems to fix the mauve gnu.testlet.java.net.ServerSocket.ReturnOnClose FAIL (now a PASS).

OK to commit?

David Daney

2005-11-09 David Daney <ddaney@avtrex.com>

	PR libgcj/15430
	* gnu/java/net/natPlainSocketImplPosix.cc (throw_on_sock_closed): New
	function.
	(accept): Call it.
	(close): Call shutdown before closing.
	(read()): Call read_helper with proper parameters.
	(read(buffer, int, int)): Likewise.
	(read_helper):  Pass pointer to the PlainSocketImpl, remove native_fd
	and timeout parameters.  Make prototype to match. Use
	pointer to PlainSocketImpl to access members. Call throw_on_sock_closed
	in two places.

Index: gnu/java/net/natPlainSocketImplPosix.cc
===================================================================
--- gnu/java/net/natPlainSocketImplPosix.cc	(revision 106716)
+++ gnu/java/net/natPlainSocketImplPosix.cc	(working copy)
@@ -226,6 +226,19 @@
     }
 }
 
+static void 
+throw_on_sock_closed (gnu::java::net::PlainSocketImpl *soc_impl)
+{
+    // Avoid races from asynchronous close().
+    JvSynchronize sync (soc_impl);
+    if (soc_impl->native_fd == -1) {
+      // Socket was closed while blocked in recv.
+      ::java::net::SocketException *se =
+          new ::java::net::SocketException (JvNewStringUTF ("Socket Closed"));
+      throw se;
+    }
+}
+
 void
 gnu::java::net::PlainSocketImpl::accept (gnu::java::net::PlainSocketImpl *s)
 {
@@ -284,6 +297,7 @@
 
  error:
   char* strerr = strerror (errno);
+  throw_on_sock_closed (this);
   throw new ::java::io::IOException (JvNewStringUTF (strerr));
 }
 
@@ -294,7 +308,11 @@
   // Avoid races from asynchronous finalization.
   JvSynchronize sync (this);
 
-  // should we use shutdown here? how would that effect so_linger?
+  // Should we use shutdown here? Yes.
+  // How would that effect so_linger? Uncertain.
+  ::shutdown (native_fd, 2);
+  // Ignore errors in shutdown as we are closing and all the same
+  // errors are handled in the close.
   int res = _Jv_close (native_fd);
 
   if (res == -1)
@@ -371,7 +389,8 @@
 }
 
 static jint
-read_helper (jint native_fd, jint timeout, jbyte *bytes, jint count);
+read_helper (gnu::java::net::PlainSocketImpl *soc_impl,
+             jbyte *bytes, jint count);
 
 // Read a single byte from the socket.
 jint
@@ -379,7 +398,7 @@
 {
   jbyte data;
 
-  if (read_helper (this$0->native_fd, this$0->timeout, &data, 1) == 1)
+  if (read_helper (this$0, &data, 1) == 1)
     return data & 0xFF;
 
   return -1;
@@ -387,8 +406,9 @@
 
 // Read count bytes into the buffer, starting at offset.
 jint
-gnu::java::net::PlainSocketImpl$SocketInputStream::read(jbyteArray buffer, jint offset, 
-  jint count)
+gnu::java::net::PlainSocketImpl$SocketInputStream::read(jbyteArray buffer,
+                                                        jint offset, 
+                                                        jint count)
 {
  if (! buffer)
     throw new ::java::lang::NullPointerException;
@@ -398,12 +418,13 @@
   if (offset < 0 || count < 0 || offset + count > bsize)
     throw new ::java::lang::ArrayIndexOutOfBoundsException;
 
-  return read_helper (this$0->native_fd, this$0->timeout,
+  return read_helper (this$0,
 		      elements (buffer) + offset * sizeof (jbyte), count);
 }
 
 static jint
-read_helper (jint native_fd, jint timeout, jbyte *bytes, jint count)
+read_helper (gnu::java::net::PlainSocketImpl *soc_impl,
+             jbyte *bytes, jint count)
 {
   // If zero bytes were requested, short circuit so that recv
   // doesn't signal EOF.
@@ -411,19 +432,22 @@
     return 0;
     
   // Do timeouts via select.
-  if (timeout > 0 && native_fd >= 0 && native_fd < FD_SETSIZE)
+  if (soc_impl->timeout > 0
+      && soc_impl->native_fd >= 0
+      && soc_impl->native_fd < FD_SETSIZE)
     {
       // Create the file descriptor set.
       fd_set read_fds;
       FD_ZERO (&read_fds);
-      FD_SET (native_fd, &read_fds);
+      FD_SET (soc_impl->native_fd, &read_fds);
       // Create the timeout struct based on our internal timeout value.
       struct timeval timeout_value;
-      timeout_value.tv_sec = timeout / 1000;
-      timeout_value.tv_usec =(timeout % 1000) * 1000;
+      timeout_value.tv_sec = soc_impl->timeout / 1000;
+      timeout_value.tv_usec =(soc_impl->timeout % 1000) * 1000;
       // Select on the fds.
       int sel_retval =
-        _Jv_select (native_fd + 1, &read_fds, NULL, NULL, &timeout_value);
+        _Jv_select (soc_impl->native_fd + 1,
+                    &read_fds, NULL, NULL, &timeout_value);
       // We're only interested in the 0 return.
       // error returns still require us to try to read 
       // the socket to see what happened.
@@ -437,10 +461,12 @@
     }
 
   // Read the socket.
-  int r = ::recv (native_fd, (char *) bytes, count, 0);
+  int r = ::recv (soc_impl->native_fd, (char *) bytes, count, 0);
 
-  if (r == 0)
+  if (r == 0) {
+    throw_on_sock_closed (soc_impl);
     return -1;
+  }
 
   if (::java::lang::Thread::interrupted())
     {
@@ -452,6 +478,7 @@
     }
   else if (r == -1)
     {
+      throw_on_sock_closed (soc_impl);
       // Some errors cause us to return end of stream...
       if (errno == ENOTCONN)
         return -1;

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]