You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by sv...@apache.org on 2021/01/30 04:01:04 UTC
svn commit: r1886045 - in /subversion/branches/1.14.x: ./
subversion/bindings/javahl/native/
subversion/bindings/javahl/src/org/apache/subversion/javahl/util/
subversion/bindings/javahl/tests/org/apache/subversion/javahl/
Author: svn-role
Date: Sat Jan 30 04:01:04 2021
New Revision: 1886045
URL: http://svn.apache.org/viewvc?rev=1886045&view=rev
Log:
Merge r1886029 from trunk:
* r1886029
Fix several crashes and JNI warnings in javahl TunnelAgent.
Justification:
JavaHL shouldn't crash.
Votes:
+1: jcorvel, amiloslavskiy
Modified:
subversion/branches/1.14.x/ (props changed)
subversion/branches/1.14.x/STATUS
subversion/branches/1.14.x/subversion/bindings/javahl/native/JNIUtil.cpp
subversion/branches/1.14.x/subversion/bindings/javahl/native/JNIUtil.h
subversion/branches/1.14.x/subversion/bindings/javahl/native/OperationContext.cpp
subversion/branches/1.14.x/subversion/bindings/javahl/src/org/apache/subversion/javahl/util/RequestChannel.java
subversion/branches/1.14.x/subversion/bindings/javahl/src/org/apache/subversion/javahl/util/ResponseChannel.java
subversion/branches/1.14.x/subversion/bindings/javahl/src/org/apache/subversion/javahl/util/TunnelChannel.java
subversion/branches/1.14.x/subversion/bindings/javahl/tests/org/apache/subversion/javahl/BasicTests.java
Propchange: subversion/branches/1.14.x/
------------------------------------------------------------------------------
Merged /subversion/branches/javahl-1.14-fixes:r1882126-1886028
Merged /subversion/trunk:r1886029
Modified: subversion/branches/1.14.x/STATUS
URL: http://svn.apache.org/viewvc/subversion/branches/1.14.x/STATUS?rev=1886045&r1=1886044&r2=1886045&view=diff
==============================================================================
--- subversion/branches/1.14.x/STATUS (original)
+++ subversion/branches/1.14.x/STATUS Sat Jan 30 04:01:04 2021
@@ -66,10 +66,3 @@ Veto-blocked changes:
Approved changes:
=================
-
- * r1886029
- Fix several crashes and JNI warnings in javahl TunnelAgent.
- Justification:
- JavaHL shouldn't crash.
- Votes:
- +1: jcorvel, amiloslavskiy
Modified: subversion/branches/1.14.x/subversion/bindings/javahl/native/JNIUtil.cpp
URL: http://svn.apache.org/viewvc/subversion/branches/1.14.x/subversion/bindings/javahl/native/JNIUtil.cpp?rev=1886045&r1=1886044&r2=1886045&view=diff
==============================================================================
--- subversion/branches/1.14.x/subversion/bindings/javahl/native/JNIUtil.cpp (original)
+++ subversion/branches/1.14.x/subversion/bindings/javahl/native/JNIUtil.cpp Sat Jan 30 04:01:04 2021
@@ -551,6 +551,11 @@ std::string JNIUtil::makeSVNErrorMessage
jstring *jerror_message,
jobject *jmessage_stack)
{
+ // This function may be called with a pending Java exception.
+ // It is incorrect to call Java methods (see code below) with a pending
+ // exception. Stash it away until this function exits.
+ StashException stash(getEnv());
+
if (jerror_message)
*jerror_message = NULL;
if (jmessage_stack)
@@ -761,16 +766,27 @@ namespace {
const char* known_exception_to_cstring(apr_pool_t* pool)
{
JNIEnv *env = JNIUtil::getEnv();
+
+ // This function may be called with a pending Java exception.
+ // It is incorrect to call Java methods (see code below) with a pending
+ // exception. Stash it away until this function exits.
jthrowable t = env->ExceptionOccurred();
+ StashException stashed(env);
+
jclass cls = env->GetObjectClass(t);
jstring jclass_name;
{
jmethodID mid = env->GetMethodID(cls, "getClass", "()Ljava/lang/Class;");
jobject clsobj = env->CallObjectMethod(t, mid);
+ if (JNIUtil::isJavaExceptionThrown())
+ return NULL;
+
jclass basecls = env->GetObjectClass(clsobj);
mid = env->GetMethodID(basecls, "getName", "()Ljava/lang/String;");
jclass_name = (jstring) env->CallObjectMethod(clsobj, mid);
+ if (JNIUtil::isJavaExceptionThrown())
+ return NULL;
}
jstring jmessage;
@@ -778,6 +794,8 @@ const char* known_exception_to_cstring(a
jmethodID mid = env->GetMethodID(cls, "getMessage",
"()Ljava/lang/String;");
jmessage = (jstring) env->CallObjectMethod(t, mid);
+ if (JNIUtil::isJavaExceptionThrown())
+ return NULL;
}
JNIStringHolder class_name(jclass_name);
@@ -1169,3 +1187,28 @@ jthrowable JNIUtil::unwrapJavaException(
return
WrappedException::get_exception(err->pool);
}
+
+StashException::StashException(JNIEnv* env)
+{
+ m_env = env;
+ m_stashed = NULL;
+ stashException();
+}
+
+StashException::~StashException()
+{
+ if (m_stashed)
+ m_env->Throw(m_stashed);
+}
+
+void StashException::stashException()
+{
+ jthrowable jexc = m_env->ExceptionOccurred();
+ if (!jexc)
+ return;
+
+ if (!m_stashed)
+ m_stashed = jexc;
+
+ m_env->ExceptionClear();
+}
Modified: subversion/branches/1.14.x/subversion/bindings/javahl/native/JNIUtil.h
URL: http://svn.apache.org/viewvc/subversion/branches/1.14.x/subversion/bindings/javahl/native/JNIUtil.h?rev=1886045&r1=1886044&r2=1886045&view=diff
==============================================================================
--- subversion/branches/1.14.x/subversion/bindings/javahl/native/JNIUtil.h (original)
+++ subversion/branches/1.14.x/subversion/bindings/javahl/native/JNIUtil.h Sat Jan 30 04:01:04 2021
@@ -330,4 +330,37 @@ class JNIUtil
} \
} while(0)
+/**
+ * If there's an exception pending, temporarily stash it away, then
+ * re-throw again in destructor. The goal is to allow some Java calls
+ * to be made despite a pending exception. For example, doing some
+ * necessary cleanup.
+ */
+class StashException
+{
+ public:
+ /*
+ * Works like stashException().
+ */
+ StashException(JNIEnv* env);
+
+ /**
+ * If there's an exception stashed, re-throws it.
+ */
+ ~StashException();
+
+ /**
+ * Check for a pending exception.
+ * If present, stash it away until this class's destructor.
+ * If another exception is already stashed, forget the _new_ one. The
+ * reason behind it is that usually the first exception is the most
+ * informative.
+ */
+ void stashException();
+
+ private:
+ JNIEnv* m_env;
+ jthrowable m_stashed;
+};
+
#endif // JNIUTIL_H
Modified: subversion/branches/1.14.x/subversion/bindings/javahl/native/OperationContext.cpp
URL: http://svn.apache.org/viewvc/subversion/branches/1.14.x/subversion/bindings/javahl/native/OperationContext.cpp?rev=1886045&r1=1886044&r2=1886045&view=diff
==============================================================================
--- subversion/branches/1.14.x/subversion/bindings/javahl/native/OperationContext.cpp (original)
+++ subversion/branches/1.14.x/subversion/bindings/javahl/native/OperationContext.cpp Sat Jan 30 04:01:04 2021
@@ -492,6 +492,8 @@ public:
request_out(NULL),
response_in(NULL),
response_out(NULL),
+ jrequest(NULL),
+ jresponse(NULL),
jclosecb(NULL)
{
status = apr_file_pipe_create_ex(&request_in, &request_out,
@@ -512,6 +514,8 @@ public:
apr_file_t *response_in;
apr_file_t *response_out;
apr_status_t status;
+ jobject jrequest;
+ jobject jresponse;
jobject jclosecb;
};
@@ -523,7 +527,10 @@ jobject create_Channel(const char *class
jmethodID ctor = env->GetMethodID(cls, "<init>", "(J)V");
if (JNIUtil::isJavaExceptionThrown())
return NULL;
- return env->NewObject(cls, ctor, reinterpret_cast<jlong>(fd));
+ jobject channel = env->NewObject(cls, ctor, reinterpret_cast<jlong>(fd));
+ if (JNIUtil::isJavaExceptionThrown())
+ return NULL;
+ return env->NewGlobalRef(channel);
}
jobject create_RequestChannel(JNIEnv *env, apr_file_t *fd)
@@ -534,6 +541,24 @@ jobject create_ResponseChannel(JNIEnv *e
{
return create_Channel(JAVAHL_CLASS("/util/ResponseChannel"), env, fd);
}
+void close_TunnelChannel(JNIEnv* env, jobject channel)
+{
+ // Usually after this function, the memory will be freed behind
+ // 'TunnelChannel.nativeChannel'. Ask Java side to forget it. This is the
+ // only way to avoid a JVM crash when 'TunnelAgent' tries to read/write,
+ // not knowing that 'TunnelChannel' is already closed in native side.
+ static jmethodID mid = 0;
+ if (0 == mid)
+ {
+ jclass cls;
+ SVN_JNI_CATCH_VOID(
+ cls = env->FindClass(JAVAHL_CLASS("/util/TunnelChannel")));
+ SVN_JNI_CATCH_VOID(mid = env->GetMethodID(cls, "syncClose", "()V"));
+ }
+
+ SVN_JNI_CATCH_VOID(env->CallVoidMethod(channel, mid));
+ env->DeleteGlobalRef(channel);
+}
} // anonymous namespace
svn_boolean_t
@@ -590,10 +615,10 @@ OperationContext::openTunnel(svn_stream_
JNIEnv *env = JNIUtil::getEnv();
- jobject jrequest = create_RequestChannel(env, tc->request_in);
+ tc->jrequest = create_RequestChannel(env, tc->request_in);
SVN_JNI_CATCH(, SVN_ERR_BASE);
- jobject jresponse = create_ResponseChannel(env, tc->response_out);
+ tc->jresponse = create_ResponseChannel(env, tc->response_out);
SVN_JNI_CATCH(, SVN_ERR_BASE);
jstring jtunnel_name = JNIUtil::makeJString(tunnel_name);
@@ -623,29 +648,32 @@ OperationContext::openTunnel(svn_stream_
}
jobject jtunnelcb = jobject(tunnel_baton);
- SVN_JNI_CATCH(
- tc->jclosecb = env->CallObjectMethod(
- jtunnelcb, mid, jrequest, jresponse,
- jtunnel_name, juser, jhostname, jint(port)),
- SVN_ERR_BASE);
+ tc->jclosecb = env->CallObjectMethod(
+ jtunnelcb, mid, tc->jrequest, tc->jresponse,
+ jtunnel_name, juser, jhostname, jint(port));
+ svn_error_t* openTunnelError = JNIUtil::checkJavaException(SVN_ERR_BASE);
+ if (SVN_NO_ERROR != openTunnelError)
+ {
+ // OperationContext::closeTunnel() will never be called, clean up here.
+ // This also prevents a JVM native crash, see comment in
+ // close_TunnelChannel().
+ *close_baton = 0;
+ tc->jclosecb = 0;
+ OperationContext::closeTunnel(tc, 0);
+ SVN_ERR(openTunnelError);
+ }
+
+ if (tc->jclosecb)
+ {
+ tc->jclosecb = env->NewGlobalRef(tc->jclosecb);
+ SVN_JNI_CATCH(, SVN_ERR_BASE);
+ }
return SVN_NO_ERROR;
}
-void
-OperationContext::closeTunnel(void *tunnel_context, void *)
+void callCloseTunnelCallback(JNIEnv* env, jobject jclosecb)
{
- TunnelContext* tc = static_cast<TunnelContext*>(tunnel_context);
- jobject jclosecb = tc->jclosecb;
- delete tc;
-
- if (!jclosecb)
- return;
-
- JNIEnv *env = JNIUtil::getEnv();
- if (JNIUtil::isJavaExceptionThrown())
- return;
-
static jmethodID mid = 0;
if (0 == mid)
{
@@ -656,4 +684,41 @@ OperationContext::closeTunnel(void *tunn
SVN_JNI_CATCH_VOID(mid = env->GetMethodID(cls, "closeTunnel", "()V"));
}
SVN_JNI_CATCH_VOID(env->CallVoidMethod(jclosecb, mid));
+ env->DeleteGlobalRef(jclosecb);
+}
+
+void
+OperationContext::closeTunnel(void *tunnel_context, void *)
+{
+ TunnelContext* tc = static_cast<TunnelContext*>(tunnel_context);
+ jobject jrequest = tc->jrequest;
+ jobject jresponse = tc->jresponse;
+ jobject jclosecb = tc->jclosecb;
+
+ // Note that this closes other end of the pipe, which cancels and
+ // prevents further read/writes in 'TunnelAgent'
+ delete tc;
+
+ JNIEnv *env = JNIUtil::getEnv();
+
+ // Cleanup is important, otherwise TunnelAgent may crash when
+ // accessing freed native objects. For this reason, cleanup is done
+ // despite a pending exception. If more exceptions occur, they are
+ // stashed as well in order to complete all cleanup steps.
+ StashException ex(env);
+
+ if (jclosecb)
+ callCloseTunnelCallback(env, jclosecb);
+
+ if (jrequest)
+ {
+ ex.stashException();
+ close_TunnelChannel(env, jrequest);
+ }
+
+ if (jresponse)
+ {
+ ex.stashException();
+ close_TunnelChannel(env, jresponse);
+ }
}
Modified: subversion/branches/1.14.x/subversion/bindings/javahl/src/org/apache/subversion/javahl/util/RequestChannel.java
URL: http://svn.apache.org/viewvc/subversion/branches/1.14.x/subversion/bindings/javahl/src/org/apache/subversion/javahl/util/RequestChannel.java?rev=1886045&r1=1886044&r2=1886045&view=diff
==============================================================================
--- subversion/branches/1.14.x/subversion/bindings/javahl/src/org/apache/subversion/javahl/util/RequestChannel.java (original)
+++ subversion/branches/1.14.x/subversion/bindings/javahl/src/org/apache/subversion/javahl/util/RequestChannel.java Sat Jan 30 04:01:04 2021
@@ -42,15 +42,18 @@ class RequestChannel
public int read(ByteBuffer dst) throws IOException
{
- 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();
+ synchronized (nativeChannel)
+ {
+ 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/branches/1.14.x/subversion/bindings/javahl/src/org/apache/subversion/javahl/util/ResponseChannel.java
URL: http://svn.apache.org/viewvc/subversion/branches/1.14.x/subversion/bindings/javahl/src/org/apache/subversion/javahl/util/ResponseChannel.java?rev=1886045&r1=1886044&r2=1886045&view=diff
==============================================================================
--- subversion/branches/1.14.x/subversion/bindings/javahl/src/org/apache/subversion/javahl/util/ResponseChannel.java (original)
+++ subversion/branches/1.14.x/subversion/bindings/javahl/src/org/apache/subversion/javahl/util/ResponseChannel.java Sat Jan 30 04:01:04 2021
@@ -42,15 +42,18 @@ class ResponseChannel
public int write(ByteBuffer src) throws IOException
{
- 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();
+ synchronized (nativeChannel)
+ {
+ 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/branches/1.14.x/subversion/bindings/javahl/src/org/apache/subversion/javahl/util/TunnelChannel.java
URL: http://svn.apache.org/viewvc/subversion/branches/1.14.x/subversion/bindings/javahl/src/org/apache/subversion/javahl/util/TunnelChannel.java?rev=1886045&r1=1886044&r2=1886045&view=diff
==============================================================================
--- subversion/branches/1.14.x/subversion/bindings/javahl/src/org/apache/subversion/javahl/util/TunnelChannel.java (original)
+++ subversion/branches/1.14.x/subversion/bindings/javahl/src/org/apache/subversion/javahl/util/TunnelChannel.java Sat Jan 30 04:01:04 2021
@@ -50,6 +50,39 @@ abstract class TunnelChannel implements
nativeClose(channel);
}
+ /**
+ * Wait for current read/write to complete, then close() channel.
+ * Compared to close(), it has the following differences:
+ * <ul>
+ * <li>
+ * Prevents race condition where read/write could use incorrect file :
+ * <ol>
+ * <li>Writer thread extracts OS file descriptor from nativeChannel.</li>
+ * <li>Writer thread calls OS API to write to file, passing file descriptor.</li>
+ * <li>Writer thread is interrupted.</li>
+ * <li>Closer thread closes OS file descriptor. The file descriptor number is now free.</li>
+ * <li>Unrelated thread opens a new file. OS reuses the old file descriptor (currently free).</li>
+ * <li>Writer thread resumes inside OS API to write to file.</li>
+ * <li>Writer thread writes to unrelated file, corrupting it with unexpected data.</li>
+ * </ol>
+ * </li>
+ * <li>
+ * It can no longer cancel a read/write operation already in progress.
+ * The native implementation closes the other end of the pipe, breaking the pipe,
+ * which prevents the risk of never-completing read/write.
+ * </li>
+ * <ul/>
+ *
+ * @throws IOException
+ */
+ public void syncClose() throws IOException
+ {
+ synchronized (nativeChannel)
+ {
+ close();
+ }
+ }
+
private native static void nativeClose(long nativeChannel)
throws IOException;
Modified: subversion/branches/1.14.x/subversion/bindings/javahl/tests/org/apache/subversion/javahl/BasicTests.java
URL: http://svn.apache.org/viewvc/subversion/branches/1.14.x/subversion/bindings/javahl/tests/org/apache/subversion/javahl/BasicTests.java?rev=1886045&r1=1886044&r2=1886045&view=diff
==============================================================================
--- subversion/branches/1.14.x/subversion/bindings/javahl/tests/org/apache/subversion/javahl/BasicTests.java (original)
+++ subversion/branches/1.14.x/subversion/bindings/javahl/tests/org/apache/subversion/javahl/BasicTests.java Sat Jan 30 04:01:04 2021
@@ -25,6 +25,7 @@ package org.apache.subversion.javahl;
import static org.junit.Assert.*;
import org.apache.subversion.javahl.callback.*;
+import org.apache.subversion.javahl.remote.*;
import org.apache.subversion.javahl.types.*;
import java.io.File;
@@ -36,6 +37,7 @@ import java.io.PrintWriter;
import java.io.ByteArrayOutputStream;
import java.io.UnsupportedEncodingException;
import java.nio.ByteBuffer;
+import java.nio.channels.ClosedChannelException;
import java.nio.channels.ReadableByteChannel;
import java.nio.channels.WritableByteChannel;
import java.text.ParseException;
@@ -4417,6 +4419,320 @@ public class BasicTests extends SVNTests
assertEquals("fake", new String(revprop));
}
+ public static int FLAG_ECHO = 0x00000001;
+ public static int FLAG_THROW_IN_OPEN = 0x00000002;
+
+ public enum Actions
+ {
+ READ_CLIENT, // Read a request from SVN client
+ EMUL_SERVER, // Emulate server response
+ WAIT_TUNNEL, // Wait for tunnel to be closed
+ };
+
+ public static class ScriptItem
+ {
+ Actions action;
+ String value;
+
+ ScriptItem(Actions action, String value)
+ {
+ this.action = action;
+ this.value = value;
+ }
+ }
+
+ private static class TestTunnelAgent extends Thread
+ implements TunnelAgent
+ {
+ ScriptItem[] script;
+ int flags;
+ String error = null;
+ ReadableByteChannel request;
+ WritableByteChannel response;
+
+ final CloseTunnelCallback closeTunnelCallback = () ->
+ {
+ if ((flags & FLAG_ECHO) != 0)
+ System.out.println("TunnelAgent.CloseTunnelCallback");
+ };
+
+ TestTunnelAgent(int flags, ScriptItem[] script)
+ {
+ this.flags = flags;
+ this.script = script;
+ }
+
+ public void joinAndTest()
+ {
+ try
+ {
+ join();
+ }
+ catch (InterruptedException e)
+ {
+ fail("InterruptedException was caught");
+ }
+
+ if (error != null)
+ fail(error);
+ }
+
+ @Override
+ public boolean checkTunnel(String name)
+ {
+ return true;
+ }
+
+ private String readClient(ByteBuffer readBuffer)
+ throws IOException
+ {
+ readBuffer.reset();
+ request.read(readBuffer);
+
+ final int offset = readBuffer.arrayOffset();
+ return new String(readBuffer.array(),
+ offset,
+ readBuffer.position() - offset);
+ }
+
+ private void emulateServer(String serverMessage)
+ throws IOException
+ {
+ final byte[] responseBytes = serverMessage.getBytes();
+ response.write(ByteBuffer.wrap(responseBytes));
+ }
+
+ private void doScriptItem(ScriptItem scriptItem, ByteBuffer readBuffer)
+ throws Exception
+ {
+ switch (scriptItem.action)
+ {
+ case READ_CLIENT:
+ final String actualLine = readClient(readBuffer);
+
+ if ((flags & FLAG_ECHO) != 0)
+ {
+ System.out.println("SERVER: " + scriptItem.value);
+ System.out.flush();
+ }
+
+ if (!actualLine.contains(scriptItem.value))
+ {
+ System.err.println("Expected: " + scriptItem.value);
+ System.err.println("Actual: " + actualLine);
+ System.err.flush();
+
+ // Unblock the SVN thread by emulating a server error
+ final String serverError = "( success ( ( ) 0: ) ) ( failure ( ( 160000 39:Test script received unexpected request 0: 0 ) ) ) ";
+ emulateServer(serverError);
+
+ fail("Unexpected client request");
+ }
+ break;
+ case EMUL_SERVER:
+ if ((flags & FLAG_ECHO) != 0)
+ {
+ System.out.println("CLIENT: " + scriptItem.value);
+ System.out.flush();
+ }
+
+ emulateServer(scriptItem.value);
+ break;
+ case WAIT_TUNNEL:
+ // The loop will end with an exception when tunnel is closed
+ for (;;)
+ {
+ readClient(readBuffer);
+ }
+ }
+ }
+
+ public void run()
+ {
+ final ByteBuffer readBuffer = ByteBuffer.allocate(1024 * 1024);
+ readBuffer.mark();
+
+ for (ScriptItem scriptItem : script)
+ {
+ try
+ {
+ doScriptItem(scriptItem, readBuffer);
+ }
+ catch (ClosedChannelException ex)
+ {
+ // Expected when closed properly
+ }
+ catch (IOException e)
+ {
+ // IOException occurs when already-freed apr_file_t was lucky
+ // to have reasonable fields to avoid the crash. It still
+ // indicates a problem.
+ error = "IOException was caught in run()";
+ return;
+ }
+ catch (Throwable t)
+ {
+ // No other exceptions are expected here.
+ error = "Exception was caught in run()";
+ t.printStackTrace();
+ return;
+ }
+ }
+ }
+
+ @Override
+ public CloseTunnelCallback openTunnel(ReadableByteChannel request,
+ WritableByteChannel response,
+ String name,
+ String user,
+ String hostname,
+ int port)
+ throws Throwable
+ {
+ this.request = request;
+ this.response = response;
+
+ start();
+
+ if ((flags & FLAG_THROW_IN_OPEN) != 0)
+ throw ClientException.fromException(new RuntimeException("Test exception"));
+
+ return closeTunnelCallback;
+ }
+ };
+
+ /**
+ * Test scenario which previously caused a JVM crash.
+ * In this scenario, GC is invoked before closing tunnel.
+ */
+ public void testCrash_RemoteSession_nativeDispose()
+ {
+ final ScriptItem[] script = new ScriptItem[]
+ {
+ new ScriptItem(Actions.EMUL_SERVER, "( success ( 2 2 ( ) ( edit-pipeline svndiff1 absent-entries commit-revprops depth log-revprops atomic-revprops partial-replay inherited-props ephemeral-txnprops file-revs-reverse ) ) ) "),
+ new ScriptItem(Actions.READ_CLIENT, "edit-pipeline"),
+ new ScriptItem(Actions.EMUL_SERVER, "( success ( ( ANONYMOUS ) 36:0113e071-0208-4a7b-9f20-3038f9caf0f0 ) ) "),
+ new ScriptItem(Actions.READ_CLIENT, "ANONYMOUS"),
+ new ScriptItem(Actions.EMUL_SERVER, "( success ( ) ) ( success ( 36:00000000-0000-0000-0000-000000000000 25:svn+test://localhost/test ( mergeinfo ) ) ) "),
+ };
+
+ final TestTunnelAgent tunnelAgent = new TestTunnelAgent(0, script);
+ final RemoteFactory remoteFactory = new RemoteFactory();
+ remoteFactory.setTunnelAgent(tunnelAgent);
+
+ ISVNRemote remote = null;
+ try
+ {
+ remote = remoteFactory.openRemoteSession("svn+test://localhost/test", 1);
+ }
+ catch (SubversionException e)
+ {
+ fail("SubversionException was caught");
+ }
+
+ // Previously, 'OperationContext::openTunnel()' didn't 'NewGlobalRef()'
+ // callback returned by 'TunnelAgent.openTunnel()'. This caused JVM to
+ // dispose it on next GC. JavaHL calls callback in 'remote.dispose()'.
+ // If the callback was disposed, this caused a JVM crash.
+ System.gc();
+ remote.dispose();
+
+ tunnelAgent.joinAndTest();
+ }
+
+ /**
+ * Test scenario which previously caused a JVM crash.
+ * In this scenario, tunnel was not properly closed after exception in
+ * 'TunnelAgent.openTunnel()'.
+ */
+ public void testCrash_RequestChannel_nativeRead_AfterException()
+ {
+ // Previously, exception caused TunnelChannel's native side to be
+ // destroyed with the following abbreviated stack:
+ // TunnelChannel.nativeClose()
+ // svn_pool_destroy(sesspool)
+ // svn_ra_open5()
+ // TunnelAgent was unaware and called 'RequestChannel.nativeRead()'
+ // or 'ResponseChannel.nativeWrite()', causing either a crash or
+ // an attempt to use a random file.
+ final int flags = FLAG_THROW_IN_OPEN;
+
+ final ScriptItem[] script = new ScriptItem[]
+ {
+ new ScriptItem(Actions.EMUL_SERVER, "( success ( 2 2 ( ) ( edit-pipeline svndiff1 absent-entries commit-revprops depth log-revprops atomic-revprops partial-replay inherited-props ephemeral-txnprops file-revs-reverse ) ) ) "),
+ new ScriptItem(Actions.WAIT_TUNNEL, ""),
+ };
+
+ final TestTunnelAgent tunnelAgent = new TestTunnelAgent(flags, script);
+ final SVNClient svnClient = new SVNClient();
+ svnClient.setTunnelAgent(tunnelAgent);
+
+ try
+ {
+ svnClient.openRemoteSession("svn+test://localhost/test");
+ }
+ catch (SubversionException e)
+ {
+ // RuntimeException("Test exception") is expected here
+ }
+
+ tunnelAgent.joinAndTest();
+ }
+
+ /**
+ * Test scenario which previously caused a JVM crash.
+ * In this scenario, tunnel was not properly closed after an SVN error.
+ */
+ public void testCrash_RequestChannel_nativeRead_AfterSvnError()
+ {
+ final String wcRoot = new File("tempSvnRepo").getAbsolutePath();
+
+ final ScriptItem[] script = new ScriptItem[]
+ {
+ // openRemoteSession
+ new ScriptItem(Actions.EMUL_SERVER, "( success ( 2 2 ( ) ( edit-pipeline svndiff1 absent-entries commit-revprops depth log-revprops atomic-revprops partial-replay inherited-props ephemeral-txnprops file-revs-reverse ) ) ) "),
+ new ScriptItem(Actions.READ_CLIENT, "edit-pipeline"),
+ new ScriptItem(Actions.EMUL_SERVER, "( success ( ( ANONYMOUS ) 36:0113e071-0208-4a7b-9f20-3038f9caf0f0 ) ) "),
+ new ScriptItem(Actions.READ_CLIENT, "ANONYMOUS"),
+ new ScriptItem(Actions.EMUL_SERVER, "( success ( ) ) ( success ( 36:00000000-0000-0000-0000-000000000000 25:svn+test://localhost/test ( mergeinfo ) ) ) "),
+ // checkout
+ new ScriptItem(Actions.READ_CLIENT, "( get-latest-rev ( ) ) "),
+ // Previously, error caused a SubversionException to be created,
+ // which then skipped closing the Tunnel properly due to
+ // 'ExceptionOccurred()' in 'OperationContext::closeTunnel()'.
+ // If TunnelAgent was unaware and called 'RequestChannel.nativeRead()',
+ // it either crashed or tried to use a random file.
+ new ScriptItem(Actions.EMUL_SERVER, "( success ( ( ) 0: ) ) ( failure ( ( 160006 20:This is a test error 0: 0 ) ) ) "),
+ // Pretend that TunnelAgent tries to read more
+ new ScriptItem(Actions.WAIT_TUNNEL, ""),
+ };
+
+ final TestTunnelAgent tunnelAgent = new TestTunnelAgent(0, script);
+ final SVNClient svnClient = new SVNClient();
+ svnClient.setTunnelAgent(tunnelAgent);
+
+ try
+ {
+ svnClient.checkout("svn+test://localhost/test",
+ wcRoot,
+ Revision.getInstance(1),
+ null,
+ Depth.infinity,
+ true,
+ false);
+
+ svnClient.dispose();
+ }
+ catch (ClientException ex)
+ {
+ final int SVN_ERR_FS_NO_SUCH_REVISION = 160006;
+ if (SVN_ERR_FS_NO_SUCH_REVISION != ex.getAllMessages().get(0).getCode())
+ ex.printStackTrace();
+ }
+
+ tunnelAgent.joinAndTest();
+ }
+
/**
* @return <code>file</code> converted into a -- possibly
* <code>canonical</code>-ized -- Subversion-internal path