You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by br...@apache.org on 2013/10/19 19:16:31 UTC

svn commit: r1533790 - in /subversion/trunk/subversion/bindings/javahl: native/ src/org/apache/subversion/javahl/util/

Author: brane
Date: Sat Oct 19 17:16:30 2013
New Revision: 1533790

URL: http://svn.apache.org/r1533790
Log:
Make the channel implementation thread-safe(r) and add additional checks
for file descriptor validity.

[in subversion/bindings/javahl/src/org/apache/subversion/javahl]

* util/TunnelChannel.java
  (TunnelChannel.nativeChannel): Use an atomic long to store the native handle.
  (TunnelChannel.TunnelChannel, TunnelChannel.isOpen, TunnelChannel.close):
   Update to use atomic operations.
* util/RequestChannel.java (RequestChannel.read): Properly detect a closed
   channel, and deactivate it if the underlying implementation threw
   an exception.
* util/ResponseChannel.java (ResponseChannel.write): As above.

[in subversion/bindings/javahl/native]

* org_apache_subversion_javahl_util_TunnelChannel.cpp
  (throw_IOException): Allow the APR status to be zero.
  (Java_org_apache_subversion_javahl_util_RequestChannel_nativeRead,
   Java_org_apache_subversion_javahl_util_ResponseChannel_nativeWrite):
   Throw an exception if the read failed.

Modified:
    subversion/trunk/subversion/bindings/javahl/native/org_apache_subversion_javahl_util_TunnelChannel.cpp
    subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/util/RequestChannel.java
    subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/util/ResponseChannel.java
    subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/util/TunnelChannel.java

Modified: subversion/trunk/subversion/bindings/javahl/native/org_apache_subversion_javahl_util_TunnelChannel.cpp
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/org_apache_subversion_javahl_util_TunnelChannel.cpp?rev=1533790&r1=1533789&r2=1533790&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/native/org_apache_subversion_javahl_util_TunnelChannel.cpp (original)
+++ subversion/trunk/subversion/bindings/javahl/native/org_apache_subversion_javahl_util_TunnelChannel.cpp Sat Oct 19 17:16:30 2013
@@ -55,12 +55,14 @@ apr_file_t* get_file_descriptor(jlong jf
 
 void throw_IOException(const char* message, apr_status_t status)
 {
-  char buf[1024];
-  apr_strerror(status, buf, sizeof(buf) - 1);
-
   std::string msg(message);
-  msg += ": ";
-  msg += buf;
+  if (status)
+    {
+      char buf[1024];
+      apr_strerror(status, buf, sizeof(buf) - 1);
+      msg += ": ";
+      msg += buf;
+    }
   JNIUtil::raiseThrowable("java/io/IOException", msg.c_str());
 }
 
@@ -208,6 +210,7 @@ Java_org_apache_subversion_javahl_util_R
   apr_file_t* fd = get_file_descriptor(nativeChannel);
   if (fd)
     return ByteBufferProxy(dst, env).read(fd, env);
+  throw_IOException(_("Invalid native file hanlde"), 0);
   return -1;
 }
 
@@ -219,5 +222,6 @@ Java_org_apache_subversion_javahl_util_R
   apr_file_t* fd = get_file_descriptor(nativeChannel);
   if (fd)
     return ByteBufferProxy(src, env).write(fd, env);
+  throw_IOException(_("Invalid native file hanlde"), 0);
   return -1;
 }

Modified: subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/util/RequestChannel.java
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/util/RequestChannel.java?rev=1533790&r1=1533789&r2=1533790&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/util/RequestChannel.java (original)
+++ subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/util/RequestChannel.java Sat Oct 19 17:16:30 2013
@@ -26,6 +26,7 @@ package org.apache.subversion.javahl.uti
 import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.nio.channels.ReadableByteChannel;
+import java.nio.channels.ClosedChannelException;
 
 /* The following channel subclasses are used by the native
    implementation of the tunnel management code. */
@@ -41,7 +42,15 @@ class RequestChannel
 
     public int read(ByteBuffer dst) throws IOException
     {
-        return nativeRead(nativeChannel, dst);
+        long channel = nativeChannel.get();
+        if (channel != 0)
+            try {
+                return nativeRead(channel, dst);
+            } catch (IOException ex) {
+                nativeChannel.set(0); // Close the channel
+                throw ex;
+            }
+        throw new ClosedChannelException();
     }
 
     private static native int nativeRead(long nativeChannel, ByteBuffer dst)

Modified: subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/util/ResponseChannel.java
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/util/ResponseChannel.java?rev=1533790&r1=1533789&r2=1533790&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/util/ResponseChannel.java (original)
+++ subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/util/ResponseChannel.java Sat Oct 19 17:16:30 2013
@@ -26,6 +26,7 @@ package org.apache.subversion.javahl.uti
 import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.nio.channels.WritableByteChannel;
+import java.nio.channels.ClosedChannelException;
 
 /* The following channel subclasses are used by the native
    implementation of the tunnel management code. */
@@ -41,7 +42,15 @@ class ResponseChannel
 
     public int write(ByteBuffer src) throws IOException
     {
-        return nativeWrite(nativeChannel, src);
+        long channel = this.nativeChannel.get();
+        if (channel != 0)
+            try {
+                return nativeWrite(channel, src);
+            } catch (IOException ex) {
+                nativeChannel.set(0); // Close the channel
+                throw ex;
+            }
+        throw new ClosedChannelException();
     }
 
     private static native int nativeWrite(long nativeChannel, ByteBuffer src)

Modified: subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/util/TunnelChannel.java
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/util/TunnelChannel.java?rev=1533790&r1=1533789&r2=1533790&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/util/TunnelChannel.java (original)
+++ subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/util/TunnelChannel.java Sat Oct 19 17:16:30 2013
@@ -26,6 +26,7 @@ package org.apache.subversion.javahl.uti
 import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.nio.channels.Channel;
+import java.util.concurrent.atomic.AtomicLong;
 
 /* The following channel subclasses are used by the native
    implementation of the tunnel management code. */
@@ -34,24 +35,23 @@ abstract class TunnelChannel implements 
 {
     protected TunnelChannel(long nativeChannel)
     {
-        this.nativeChannel = nativeChannel;
+        this.nativeChannel = new AtomicLong(nativeChannel);
     }
 
     public boolean isOpen()
     {
-        return (nativeChannel != 0);
+        return (nativeChannel.get() != 0);
     }
 
     public void close() throws IOException
     {
-        /* Avoid closing twice on error: explicit and via gc */
-        long channel = this.nativeChannel;
-        this.nativeChannel = 0;
-        nativeClose(channel);
+        long channel = nativeChannel.getAndSet(0);
+        if (channel != 0)
+            nativeClose(channel);
     }
 
     private native static void nativeClose(long nativeChannel)
         throws IOException;
 
-    protected long nativeChannel;
+    protected AtomicLong nativeChannel;
 }