PATCH: remove all traces of java.io.FileDescriptor from java.net

Jeff Sturm jsturm@one-point.com
Tue Mar 26 10:58:00 GMT 2002


On 25 Mar 2002, Tom Tromey wrote:
> Jeff> So my inclination is to resubmit the patch minus the SocketImpl
> Jeff> changes.
> 
> Is the idea that the SocketImpl.getFileDescriptor would just return
> null?

getFileDescriptor() returns `fd'.  If we don't initialize `fd' in
PlainSocketImpl then yes, it will return null.

> That seems reasonable, weirdly enough.  Or maybe we could have
> it either return an invalid fd or throw some exception.

It will never be called by PlainSocketImpl so it doesn't really matter
what we do.  Someone else may implement a different SocketImpl outside 
of libgcj that does use getFileDescriptor(), then it's up to them to get
it right wrt close, finalization etc.

Ditto for DatagramSocketImpl.

> There's actually two uses for getFileDescriptor; you can call
> FileDescriptor.sync or FileDescriptor.valid.  I don't find either of
> these really compelling.

Especially not for sockets.

Here's my updated patch.  Hans convinced me that close/finalize ought to
synchronize access to fnum, I've included that below.  We don't really
have a good testsuite for java.net, but our application does exercise
java.net.Socket considerably and it seems to work fine:

2002-03-26  Jeff Sturm  <jsturm@one-point.com>

	* java/net/PlainDatagramSocketImpl.java
	(close): Use native implementation.
	(finalize): New method.

	* java/net/PlainSocketImpl.java (finalize): New method.

	* java/net/natPlainDatagramSocketImpl.cc
	(java/io/FileDescriptor.h): Don't include.
	(close): Implement method here.
	(create): Don't assign fd.

	* java/net/natPlainSocketImpl.cc
	(java/io/FileDescriptor.h): Don't include.
	(create): Don't assign fd.
	(accept): Likewise.
	(close): Synchronize.

diff -up java/net.orig/PlainDatagramSocketImpl.java java/net/PlainDatagramSocketImpl.java
--- java/net.orig/PlainDatagramSocketImpl.java	Fri Dec  8 05:28:32 2000
+++ java/net/PlainDatagramSocketImpl.java	Tue Mar 26 11:07:16 2002
@@ -67,27 +67,7 @@ class PlainDatagramSocketImpl extends Da
   public native Object getOption(int optID) throws SocketException;
   private native void mcastGrp(InetAddress inetaddr, boolean join)
 	throws IOException;
-
-  protected void close()
-  {
-    // FIXME: The close method in each of the DatagramSocket* classes does
-    // not throw an IOException.  The issue is that FileDescriptor.close()
-    // in natFileDescriptorPosix.cc can throw one, so we have to catch
-    // it here.  It seems that FileDescriptor.close is properly throwing
-    // the IOException on errors since many of the java.io classes depend
-    // on that.  This probably requires a bit more research but for now,
-    // we'll catch the IOException here.
-    try
-      {
-        if (fd.valid())
-	  fd.close();
-      }
-    catch (IOException e)
-      {
-	System.err.println("PlainDatagramSocketImpl.close: Error closing - " +
-	  e.getMessage());
-      }
-  }
+  protected native void close();
 
   // Deprecated in JDK 1.2.
   protected byte getTTL() throws IOException
@@ -109,5 +89,15 @@ class PlainDatagramSocketImpl extends Da
   protected void leave(InetAddress inetaddr) throws IOException
   {
     mcastGrp(inetaddr, false);
+  }
+
+  protected void finalize() throws Throwable
+  {
+    synchronized (this)
+      {
+	if (fnum != -1)
+	  close();
+      }
+    super.finalize();
   }
 }
diff -up java/net.orig/PlainSocketImpl.java java/net/PlainSocketImpl.java
--- java/net.orig/PlainSocketImpl.java	Tue Jan  8 16:14:58 2002
+++ java/net/PlainSocketImpl.java	Tue Mar 26 11:08:24 2002
@@ -39,11 +39,6 @@ class PlainSocketImpl extends SocketImpl
    * This is used for reads and writes to/from the socket and
    * to close it.
    *
-   * {@link SocketImpl#fd} is created from this like so:
-   * <pre>
-   *   fd = new FileDescriptor (fnum);
-   * </pre>
-   *
    * When the socket is closed this is reset to -1.
    */
   int fnum = -1;
@@ -108,6 +103,22 @@ class PlainSocketImpl extends SocketImpl
   private native void write(byte[] buffer, int offset, int count)
     throws IOException;
 
+  protected void finalize() throws Throwable
+  {
+    synchronized (this)
+      {
+	if (fnum != -1)
+	  try
+	    {
+	      close();
+	    }
+	  catch (IOException ex)
+	    {
+	      // ignore
+	    }
+      }
+    super.finalize();
+  }
 
   /** @return the input stream attached to the socket.
    */
diff -up java/net.orig/natPlainDatagramSocketImpl.cc java/net/natPlainDatagramSocketImpl.cc
--- java/net.orig/natPlainDatagramSocketImpl.cc	Sun Mar 10 13:00:06 2002
+++ java/net/natPlainDatagramSocketImpl.cc	Tue Mar 26 11:10:28 2002
@@ -51,7 +51,6 @@ _Jv_bind (int fd, struct sockaddr *addr,
 
 #include <gcj/cni.h>
 #include <java/io/IOException.h>
-#include <java/io/FileDescriptor.h>
 #include <java/io/InterruptedIOException.h>
 #include <java/net/BindException.h>
 #include <java/net/SocketException.h>
@@ -91,6 +90,13 @@ java::net::PlainDatagramSocketImpl::peek
 }
 
 void
+java::net::PlainDatagramSocketImpl::close ()
+{
+  throw new java::io::IOException (
+    JvNewStringLatin1 ("DatagramSocketImpl.close: unimplemented"));
+}
+
+void
 java::net::PlainDatagramSocketImpl::send (java::net::DatagramPacket *)
 {
   throw new java::io::IOException (
@@ -189,7 +195,6 @@ java::net::PlainDatagramSocketImpl::crea
   _Jv_platform_close_on_exec (sock);
 
   fnum = sock;
-  fd = new java::io::FileDescriptor (sock);
 }
 
 void
@@ -282,6 +287,19 @@ java::net::PlainDatagramSocketImpl::peek
  error:
   char* strerr = strerror (errno);
   throw new java::io::IOException (JvNewStringUTF (strerr));
+}
+
+// Close(shutdown) the socket.
+void
+java::net::PlainDatagramSocketImpl::close ()
+{
+  // Avoid races from asynchronous finalization.
+  JvSynchronize sync (this);
+
+  // The method isn't declared to throw anything, so we disregard
+  // the return value.
+  ::close (fnum);
+  fnum = -1;
 }
 
 void
diff -up java/net.orig/natPlainSocketImpl.cc java/net/natPlainSocketImpl.cc
--- java/net.orig/natPlainSocketImpl.cc	Sat Mar 23 09:44:45 2002
+++ java/net/natPlainSocketImpl.cc	Tue Mar 26 11:11:06 2002
@@ -102,7 +102,6 @@ _Jv_accept (int fd, struct sockaddr *add
 #include <gcj/cni.h>
 #include <gcj/javaprims.h>
 #include <java/io/IOException.h>
-#include <java/io/FileDescriptor.h>
 #include <java/io/InterruptedIOException.h>
 #include <java/net/BindException.h>
 #include <java/net/ConnectException.h>
@@ -235,7 +234,6 @@ java::net::PlainSocketImpl::create (jboo
   _Jv_platform_close_on_exec (sock);
 
   fnum = sock;
-  fd = new java::io::FileDescriptor (sock);
 }
 
 void
@@ -402,7 +400,6 @@ java::net::PlainSocketImpl::accept (java
   s->localport = localport;
   s->address = new InetAddress (raddr, NULL);
   s->port = rport;
-  s->fd = new java::io::FileDescriptor (new_socket);
   return;
  error:
   char* strerr = strerror (errno);
@@ -413,6 +410,9 @@ java::net::PlainSocketImpl::accept (java
 void
 java::net::PlainSocketImpl::close()
 {
+  // Avoid races from asynchronous finalization.
+  JvSynchronize sync (this);
+
   // should we use shutdown here? how would that effect so_linger?
   int res = ::close (fnum);
 



More information about the Java-patches mailing list