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