svn commit: r1886035 - /subversion/branches/1.14.x/STATUS
Author: amiloslavskiy Date: Fri Jan 29 20:27:55 2021 New Revision: 1886035 URL: http://svn.apache.org/viewvc?rev=1886035=rev Log: * 1.14.x/STATUS: Vote for r1886029, approving. Modified: subversion/branches/1.14.x/STATUS Modified: subversion/branches/1.14.x/STATUS URL: http://svn.apache.org/viewvc/subversion/branches/1.14.x/STATUS?rev=1886035=1886034=1886035=diff == --- subversion/branches/1.14.x/STATUS (original) +++ subversion/branches/1.14.x/STATUS Fri Jan 29 20:27:55 2021 @@ -50,13 +50,6 @@ Candidate changes: Votes: +1: hartmannathan, stsp - * r1886029 - Fix several crashes and JNI warnings in javahl TunnelAgent. - Justification: - JavaHL shouldn't crash. - Votes: - +1: jcorvel - Veto-blocked changes: = @@ -104,3 +97,10 @@ Approved changes: Bugfix; 'svn info --xml' should give correct results; user complained. Votes: +1: hartmannathan, stsp, jcorvel + + * r1886029 + Fix several crashes and JNI warnings in javahl TunnelAgent. + Justification: + JavaHL shouldn't crash. + Votes: + +1: jcorvel, amiloslavskiy
svn commit: r1885955 - /subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/tests/org/apache/subversion/javahl/BasicTests.java
Author: amiloslavskiy
Date: Thu Jan 28 00:13:48 2021
New Revision: 1885955
URL: http://svn.apache.org/viewvc?rev=1885955=rev
Log:
JavaHL: Trivial changes in tests
Improved code formatting and code comments according to code review.
Modified:
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/tests/org/apache/subversion/javahl/BasicTests.java
Modified:
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/tests/org/apache/subversion/javahl/BasicTests.java
URL:
http://svn.apache.org/viewvc/subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/tests/org/apache/subversion/javahl/BasicTests.java?rev=1885955=1885954=1885955=diff
==
---
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/tests/org/apache/subversion/javahl/BasicTests.java
(original)
+++
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/tests/org/apache/subversion/javahl/BasicTests.java
Thu Jan 28 00:13:48 2021
@@ -4449,7 +4449,7 @@ public class BasicTests extends SVNTests
String error = null;
ReadableByteChannel request;
WritableByteChannel response;
-
+
final CloseTunnelCallback closeTunnelCallback = () ->
{
if ((flags & FLAG_ECHO) != 0)
@@ -4484,23 +4484,23 @@ public class BasicTests extends SVNTests
}
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));
- }
+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
@@ -4523,8 +4523,8 @@ public class BasicTests extends SVNTests
System.err.flush();
// Unblock the SVN thread by emulating a server error
- final String serverError = "( success (
( ) 0: ) ) ( failure ( ( 16 39:Test script received unexpected request 0: 0
) ) ) ";
- emulateServer(serverError);
+final String serverError = "( success ( ( ) 0: ) ) (
failure ( ( 16 39:Test script received unexpected request 0: 0 ) ) ) ";
+emulateServer(serverError);
fail("Unexpected client request");
}
@@ -4536,7 +4536,7 @@ public class BasicTests extends SVNTests
System.out.flush();
}
- emulateServer(scriptItem.value);
+emulateServer(scriptItem.value);
break;
case WAIT_TUNNEL:
// The loop will end with an exception when tunnel is closed
@@ -4554,17 +4554,24 @@ public class BasicTests extends SVNTests
for (ScriptItem scriptItem : script)
{
-try {
+try
+{
doScriptItem(scriptItem, readBuffer);
-} catch (ClosedChannelException ex) {
+}
+catch (ClosedChannelException ex)
+{
// Expected when closed properly
-} catch (IOException e) {
+}
+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) {
+}
+catch (Throwable t)
+{
// No other exception
svn propchange: r1882522 - svn:log
Author: amiloslavskiy Revision: 1882522 Modified property: svn:log Modified: svn:log at Wed Jan 27 23:39:20 2021 -- --- svn:log (original) +++ svn:log Wed Jan 27 23:39:20 2021 @@ -12,6 +12,11 @@ This crash is demonstrated by the follow [in subversion/bindings/javahl] * native/OperationContext.cpp (callCloseTunnelCallback): Extract function to facilitate changes in - further commits. - (openTunnel): Add NewGlobalRef() for kept jobject. +further commits. + (openTunnel): Add NewGlobalRef() for kept jobject. (callCloseTunnelCallback): Add a matching DeleteGlobalRef(). + (closeTunnel): Remove early return. This becomes necessary in next commit +to make sure that every object is cleaned up regardless of errors in other + objects. Removing early return in this commit reduces patch sizes. Extra + JNIUtil calls are merely a very small performance change in an unlikely + scenario.
svn propchange: r1882523 - svn:log
Author: amiloslavskiy Revision: 1882523 Modified property: svn:log Modified: svn:log at Wed Jan 27 21:42:33 2021 -- --- svn:log (original) +++ svn:log Wed Jan 27 21:42:33 2021 @@ -17,6 +17,8 @@ subsequent commits as well. [in subversion/bindings/javahl] * native/OperationContext.cpp + (jrequest,jresponse): Keep references to tunnels to inform them later + (create_Channel): Keeping a reference requires NewGlobalRef() (close_TunnelChannel): New function to inform Java side. (openTunnel): Keep references to Java tunnel objects. (closeTunnel): Inform Java side when tunnel is closed.
svn commit: r1882525 - /subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/OperationContext.cpp
Author: amiloslavskiy
Date: Thu Oct 15 10:18:28 2020
New Revision: 1882525
URL: http://svn.apache.org/viewvc?rev=1882525=rev
Log:
JavaHL: Fix crash when TunnelAgent.openTunnel() throws exception
See the previous commit for explanation why cleanup is important.
The problem occurs because when 'OperationContext::openTunnel()'
returns an error, SVN considers it as not constructed and will not
clean it up.
This crash is demonstrated by the following JavaHL test:
* testCrash_RequestChannel_nativeRead_AfterException
[in subversion/bindings/javahl]
* native/OperationContext.cpp
Clean up after exception in 'TunnelAgent.openTunnel()'
Modified:
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/OperationContext.cpp
Modified:
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/OperationContext.cpp
URL:
http://svn.apache.org/viewvc/subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/OperationContext.cpp?rev=1882525=1882524=1882525=diff
==
---
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/OperationContext.cpp
(original)
+++
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/OperationContext.cpp
Thu Oct 15 10:18:28 2020
@@ -648,11 +648,20 @@ OperationContext::openTunnel(svn_stream_
}
jobject jtunnelcb = jobject(tunnel_baton);
- SVN_JNI_CATCH(
- tc->jclosecb = env->CallObjectMethod(
- jtunnelcb, mid, tc->jrequest, tc->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)
{
svn commit: r1882524 - /subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/OperationContext.cpp
Author: amiloslavskiy
Date: Thu Oct 15 10:17:19 2020
New Revision: 1882524
URL: http://svn.apache.org/viewvc?rev=1882524=rev
Log:
JavaHL: Make sure that TunnelAgent is cleaned up on pending exception
See the previous commit for explanation why cleanup is important.
Before this commit, if there was a pending Java exception (such as SVN
error), 'OperationContext::closeTunnel()' early-returned on
'isJavaExceptionThrown()'.
At the same time, calling Java methods in 'close_TunnelChannel()'
requires that there are no pending Java exceptions. Use
'StashException' to temporarily move it out of the way.
This crash is demonstrated by the following JavaHL tests:
* testCrash_RequestChannel_nativeRead_AfterException
* testCrash_RequestChannel_nativeRead_AfterSvnError
This commit alone does not fix all problems in these tests, see
previous and next commits as well.
[in subversion/bindings/javahl]
* native/OperationContext.cpp
Use 'StashException' to move temporarily exception out of the way.
Modified:
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/OperationContext.cpp
Modified:
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/OperationContext.cpp
URL:
http://svn.apache.org/viewvc/subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/OperationContext.cpp?rev=1882524=1882523=1882524=diff
==
---
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/OperationContext.cpp
(original)
+++
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/OperationContext.cpp
Thu Oct 15 10:17:19 2020
@@ -691,19 +691,25 @@ OperationContext::closeTunnel(void *tunn
delete tc;
JNIEnv *env = JNIUtil::getEnv();
- if (JNIUtil::isJavaExceptionThrown())
-return;
+
+ // 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);
}
}
svn commit: r1882523 - in /subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl: native/ src/org/apache/subversion/javahl/util/
Author: amiloslavskiy
Date: Thu Oct 15 10:16:39 2020
New Revision: 1882523
URL: http://svn.apache.org/viewvc?rev=1882523=rev
Log:
JavaHL: Fix crash when TunnelAgent continues reading after being closed
The problem here is that when 'RemoteSession' is destroyed, it also
does 'apr_pool_destroy()' which frees memory behind 'apr_file_t', which
is represented by 'TunnelChannel.nativeChannel' in Java.
'TunnelChannel' runs on a different thread and is unaware that
'apr_file_t' pointer is now invalid.
Fix this by informing 'TunnelChannel' before 'apr_file_t' is freed.
This crash is demonstrated by the following JavaHL tests:
* testCrash_RequestChannel_nativeRead_AfterException
* testCrash_RequestChannel_nativeRead_AfterSvnError
This commit alone does not fix all problems in these tests, see
subsequent commits as well.
[in subversion/bindings/javahl]
* native/OperationContext.cpp
(close_TunnelChannel): New function to inform Java side.
(openTunnel): Keep references to Java tunnel objects.
(closeTunnel): Inform Java side when tunnel is closed.
* src/org/apache/subversion/javahl/util/RequestChannel.java
* src/org/apache/subversion/javahl/util/ResponseChannel.java
Add 'synchronized' to avoid error described in 'syncClose()'
* src/org/apache/subversion/javahl/util/Tunnel.java
A new function to clean up. I decided not to change old '.close()'
because the new function can lead to a deadlock if used incorrectly.
Modified:
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/OperationContext.cpp
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/src/org/apache/subversion/javahl/util/RequestChannel.java
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/src/org/apache/subversion/javahl/util/ResponseChannel.java
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/src/org/apache/subversion/javahl/util/TunnelChannel.java
Modified:
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/OperationContext.cpp
URL:
http://svn.apache.org/viewvc/subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/OperationContext.cpp?rev=1882523=1882522=1882523=diff
==
---
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/OperationContext.cpp
(original)
+++
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/OperationContext.cpp
Thu Oct 15 10:16:39 2020
@@ -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(_in, _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, "", "(J)V");
if (JNIUtil::isJavaExceptionThrown())
return NULL;
- return env->NewObject(cls, ctor, reinterpret_cast(fd));
+ jobject channel = env->NewObject(cls, ctor, reinterpret_cast(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);
@@ -625,7 +650,7 @@ OperationContext::openTunnel(svn_stream_
jobject jtunnelcb = jobject(tunnel_baton);
SVN_
svn commit: r1882522 - /subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/OperationContext.cpp
Author: amiloslavskiy
Date: Thu Oct 15 10:15:47 2020
New Revision: 1882522
URL: http://svn.apache.org/viewvc?rev=1882522=rev
Log:
JavaHL: Fix crash in TunnelAgent.CloseTunnelCallback after GC
When jobject reference is kept across different JNI calls, a new global
reference must be requested with NewGlobalRef(). Otherwise, GC is free
to remove the object. Even if Java code keeps a reference to the object,
GC can still move the object around, invalidating the kept jobject,
which results in a native crash when trying to access it.
This crash is demonstrated by the following JavaHL test:
'testCrash_RemoteSession_nativeDispose'
[in subversion/bindings/javahl]
* native/OperationContext.cpp
(callCloseTunnelCallback): Extract function to facilitate changes in
further commits.
(openTunnel): Add NewGlobalRef() for kept jobject.
(callCloseTunnelCallback): Add a matching DeleteGlobalRef().
Modified:
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/OperationContext.cpp
Modified:
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/OperationContext.cpp
URL:
http://svn.apache.org/viewvc/subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/OperationContext.cpp?rev=1882522=1882521=1882522=diff
==
---
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/OperationContext.cpp
(original)
+++
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/OperationContext.cpp
Thu Oct 15 10:15:47 2020
@@ -629,23 +629,17 @@ OperationContext::openTunnel(svn_stream_
jtunnel_name, juser, jhostname, jint(port)),
SVN_ERR_BASE);
+ 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(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 +650,20 @@ 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(tunnel_context);
+ jobject jclosecb = tc->jclosecb;
+ delete tc;
+
+ JNIEnv *env = JNIUtil::getEnv();
+ if (JNIUtil::isJavaExceptionThrown())
+return;
+
+ if (jclosecb)
+callCloseTunnelCallback(env, jclosecb);
}
svn commit: r1882521 - /subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/JNIUtil.cpp
Author: amiloslavskiy
Date: Thu Oct 15 10:15:14 2020
New Revision: 1882521
URL: http://svn.apache.org/viewvc?rev=1882521=rev
Log:
JavaHL: Fix 'JNI call made without checking exceptions when required to'
Java JNI documentation says:
The JNI functions that invoke a Java method return the result of
the Java method. The programmer must call ExceptionOccurred() to
check for possible exceptions that occurred during the execution
of the Java method.
These warnings are seen in JavaHL tests due to '-Xcheck:jni' option.
[in subversion/bindings/javahl]
* native/JNIUtil.cpp
Properly check for exceptions after every 'CallObjectMethod()'. In
case of exception, return NULL, which is valid, because the callers
are prepared for it:
* exception_to_cstring()
JNIUtil::thrownExceptionToCString()
ClientContext::resolve()
Passes result to 'svn_error_create()'
Documented to be able to deal with NULL.
* JNIUtil::checkJavaException()
Explicitly handles returned NULL.
Modified:
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/JNIUtil.cpp
Modified:
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/JNIUtil.cpp
URL:
http://svn.apache.org/viewvc/subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/JNIUtil.cpp?rev=1882521=1882520=1882521=diff
==
---
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/JNIUtil.cpp
(original)
+++
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/JNIUtil.cpp
Thu Oct 15 10:15:14 2020
@@ -779,9 +779,14 @@ const char* known_exception_to_cstring(a
{
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;
@@ -789,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);
svn commit: r1882520 - /subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/JNIUtil.cpp
Author: amiloslavskiy
Date: Thu Oct 15 10:14:30 2020
New Revision: 1882520
URL: http://svn.apache.org/viewvc?rev=1882520=rev
Log:
JavaHL: Fix 'JNI call made with exception pending' error
It is incorrect to call Java method with 'CallObjectMethod' when there
is an unhandled Java exception already pending. The fix is to
temporarily move exception out of the way.
This error is easily seen when running JavaHL tests.
[in subversion/bindings/javahl]
* native/JNIUtil.cpp
Use 'StashException' helper class to temporarily move exception out
of the way in two functions which are designed to run when there is
an exception pending.
Modified:
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/JNIUtil.cpp
Modified:
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/JNIUtil.cpp
URL:
http://svn.apache.org/viewvc/subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/JNIUtil.cpp?rev=1882520=1882519=1882520=diff
==
---
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/JNIUtil.cpp
(original)
+++
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/JNIUtil.cpp
Thu Oct 15 10:14:30 2020
@@ -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,7 +766,13 @@ 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;
svn commit: r1882519 - in /subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native: JNIUtil.cpp JNIUtil.h
Author: amiloslavskiy
Date: Thu Oct 15 10:13:54 2020
New Revision: 1882519
URL: http://svn.apache.org/viewvc?rev=1882519=rev
Log:
JavaHL: Introduce new helper class to temporarily stash Java exceptions
This will be used in subsequent commits. Committed separately to
facilitate code review and cherry-picks.
[in subversion/bindings/javahl]
* native/JNIUtil.cpp
* native/JNIUtil.h
New class 'StashException', please refer to code comments.
Modified:
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/JNIUtil.cpp
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/JNIUtil.h
Modified:
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/JNIUtil.cpp
URL:
http://svn.apache.org/viewvc/subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/JNIUtil.cpp?rev=1882519=1882518=1882519=diff
==
---
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/JNIUtil.cpp
(original)
+++
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/JNIUtil.cpp
Thu Oct 15 10:13:54 2020
@@ -1169,3 +1169,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/javahl-1.14-fixes/subversion/bindings/javahl/native/JNIUtil.h
URL:
http://svn.apache.org/viewvc/subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/JNIUtil.h?rev=1882519=1882518=1882519=diff
==
---
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/JNIUtil.h
(original)
+++
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/JNIUtil.h
Thu Oct 15 10:13:54 2020
@@ -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
svn commit: r1882518 - /subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/tests/org/apache/subversion/javahl/BasicTests.java
Author: amiloslavskiy
Date: Thu Oct 15 10:12:52 2020
New Revision: 1882518
URL: http://svn.apache.org/viewvc?rev=1882518=rev
Log:
JavaHL: Introduce tests showing JVM crashes with TunnelAgent
The crashes will be addressed in subsequent commits.
[in subversion/bindings/javahl]
* tests/org/apache/subversion/javahl/BasicTests.java
Add helper class 'TestTunnelAgent' to emulate server replies in test
environment.
Add new tests which currently causes JVM to crash:
* testCrash_RemoteSession_nativeDispose
* testCrash_RequestChannel_nativeRead_AfterException
* testCrash_RequestChannel_nativeRead_AfterSvnError
Modified:
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/tests/org/apache/subversion/javahl/BasicTests.java
Modified:
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/tests/org/apache/subversion/javahl/BasicTests.java
URL:
http://svn.apache.org/viewvc/subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/tests/org/apache/subversion/javahl/BasicTests.java?rev=1882518=1882517=1882518=diff
==
---
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/tests/org/apache/subversion/javahl/BasicTests.java
(original)
+++
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/tests/org/apache/subversion/javahl/BasicTests.java
Thu Oct 15 10:12:52 2020
@@ -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,298 @@ public class BasicTests extends SVNTests
assertEquals("fake", new String(revprop));
}
+public static int FLAG_ECHO = 0x0001;
+public static int FLAG_THROW_IN_OPEN = 0x0002;
+
+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: " + scrip
svn commit: r1882126 - /subversion/branches/javahl-1.14-fixes/
Author: amiloslavskiy Date: Tue Sep 29 13:45:37 2020 New Revision: 1882126 URL: http://svn.apache.org/viewvc?rev=1882126=rev Log: Created branch javahl-1.14-fixes. Added: subversion/branches/javahl-1.14-fixes/ (props changed) - copied from r1882125, subversion/trunk/ Propchange: subversion/branches/javahl-1.14-fixes/ -- --- svn:auto-props (added) +++ svn:auto-props Tue Sep 29 13:45:37 2020 @@ -0,0 +1,13 @@ +*.c = svn:eol-style=native +*.cpp = svn:eol-style=native +*.h = svn:eol-style=native +*.hpp = svn:eol-style=native +*.java = svn:eol-style=native +*.py = svn:eol-style=native +*.pl = svn:eol-style=native +*.rb = svn:eol-style=native +*.sql = svn:eol-style=native +*.txt = svn:eol-style=native +README = svn:eol-style=native +BRANCH-README = svn:eol-style=native +STATUS = svn:eol-style=native Propchange: subversion/branches/javahl-1.14-fixes/ -- --- svn:ignore (added) +++ svn:ignore Tue Sep 29 13:45:37 2020 @@ -0,0 +1,58 @@ +ChangeLog* +Makefile +config.cache +config.log +config.nice +config.status +configure +libtool +.gdb_history +.swig_checked +*.orig +*.rej +TAGS +tags +neon +build-outputs.mk +autogen-standalone.mk +autom4te.cache +gen-make.opts +tests.log* +fails.log* +db4-win32 +db +*.o +*~ +.*~ +apr +apr-util +apr-iconv +Release +Debug +ipch +subversion_msvc.dsw +subversion_msvc.ncb +subversion_msvc.opt +subversion_msvc.plg +subversion_vcnet.* +mkmf.log +.project +.classpath +.cdtproject +.settings +.cproject +py3c +zlib +sqlite-amalgamation +serf +googlemock +.git +.gitignore +.idea +compile_commands.json +.kdev4 +*.kdev4 +.vs +.swig_pl_checked +.swig_py_checked +.swig_rb_checked Propchange: subversion/branches/javahl-1.14-fixes/ -- --- svn:mergeinfo (added) +++ svn:mergeinfo Tue Sep 29 13:45:37 2020 @@ -0,0 +1,106 @@ +/subversion/branches/1.10-cache-improvements:1669168-1694487 +/subversion/branches/1.11.x:1841316,1841548 +/subversion/branches/1.5.x-r30215:870312 +/subversion/branches/1.7.x-fs-verify:1146708,1161180 +/subversion/branches/1.9-cache-improvements:1678948-1679863 +/subversion/branches/1.9.x:1735680 +/subversion/branches/10Gb:1388102,1388163-1388190,1388195,1388202,1388205,1388211,1388276,1388362,1388375,1388394,1388636,1388639-1388640,1388643-1388644,1388654,1388720,1388789,1388795,1388801,1388805,1388807,1388810,1388816,1389044,1389276,1389289,1389662,1389867,1390017,1390209,1390216,1390407,1390409,1390414,1390419,1390955 +/subversion/branches/atomic-revprop:965046-1000689 +/subversion/branches/authzperf:1613053-1776831 +/subversion/branches/auto-props-sdc:1384106-1401643 +/subversion/branches/bdb-reverse-deltas:872050-872529 +/subversion/branches/cache-server:1458643-1476567 +/subversion/branches/decouple-shelving-cli:1874630-1875035 +/subversion/branches/diff-callbacks3:870059-870761 +/subversion/branches/diff-optimizations:1031270-1037352 +/subversion/branches/diff-optimizations-bytes:1037353-1067789 +/subversion/branches/dont-save-plaintext-passwords-by-default:870728-871118 +/subversion/branches/double-delete:870511-872970 +/subversion/branches/dump-load-cross-check:1654853-1657295 +/subversion/branches/ev2-export:1325914,1332738,1413107 +/subversion/branches/explore-wc:875486,875493,875497,875507,875511,875514,875559,875580-875581,875584,875587,875611,875627,875647,875667-875668,875711-875712,875733-875734,875736,875744-875748,875751,875758,875782,875795-875796,875830,875836,875838,875842,875852,875855,875864,875870,875873,875880,875885-875888,875890,875897-875898,875905,875907-875909,875935,875943-875944,875946,875979,875982-875983,875985-875986,875990,875997 +/subversion/branches/file-externals:871779-873302 +/subversion/branches/fs-rep-sharing:869036-873803 +/subversion/branches/fsfs-format7:1426304,1430673,1433848,1438408,1438982,1441129,1442051,1442068,1442504,1442910,1443171,1443803,1444690,1444693,1444695,1445040,1445080,1446103,1451129,1453590,1454307,1460579,1461851,1461865,1462837,1462904,1463120,1467362,1467382,1469487,1471208,1477166,1478055,1481447,1489817,1489949,1490673-1490674,1491784,1493042,1498029,1498103,1498155,1500054,1507729-1507731,1507735-1507736 +/subversion/branches/fsfs-improvements:1499981-1547039 +/subversion/branches/fsfs-lock-many:1571740-1577217 +/subversion/branches/fsfs-pack:873717-874575 +/subversion/branches/fsx:1507845-1509914 +/subversion/branches/fsx-1.10:1658219-1694500 +/subversion/branches/fsx-id:1645603-1649011 +/subversion/branches/gnome-keyring:870558-871410 +/subversion/branches/gpg-agent-password-store:1005036-1150766 +/subversion/branches/gtest_addition:1452117-1502138 +/subversion/branches/http-protocol-v2:874395-876041 +/subversion/branches/in-memory-cache:869829-871452 +/subversion/branches/in-repo-authz:1414342-1424779 +/subversion/branches/inheritable-props:1297080-1395089
svn commit: r1882115 - /subversion/trunk/subversion/bindings/javahl/native/SVNBase.cpp
Author: amiloslavskiy
Date: Tue Sep 29 12:12:59 2020
New Revision: 1882115
URL: http://svn.apache.org/viewvc?rev=1882115=rev
Log:
JavaHL: Fix incorrect cache in SVNBase::createCppBoundObject
The problem here is that 'SVNBase::createCppBoundObject' can work
with different classes (see argument), but it cached methodID of
'' for the first class processed. When invoked with a
different class later, it will call wring '' method.
The error is seen when running JavaHL tests with JDK14.
Error message is:
FATAL ERROR in native method: Wrong object class or methodID passed to JNI call
at <...>.javahl.util.SubstLib.translateOutputStream(Native Method)
at <...>.javahl.SVNUtil.translateStream(SVNUtil.java:1046)
at <...>.javahl.UtilTests.testTranslateStream(UtilTests.java:521)
<...>
[in subversion/bindings/javahl]
* native/SVNBase.cpp
(createCppBoundObject): Do not cache methodID.
Modified:
subversion/trunk/subversion/bindings/javahl/native/SVNBase.cpp
Modified: subversion/trunk/subversion/bindings/javahl/native/SVNBase.cpp
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/SVNBase.cpp?rev=1882115=1882114=1882115=diff
==
--- subversion/trunk/subversion/bindings/javahl/native/SVNBase.cpp (original)
+++ subversion/trunk/subversion/bindings/javahl/native/SVNBase.cpp Tue Sep 29
12:12:59 2020
@@ -107,13 +107,9 @@ jobject SVNBase::createCppBoundObject(co
if (JNIUtil::isJavaExceptionThrown())
return NULL;
- static jmethodID ctor = 0;
- if (ctor == 0)
-{
- ctor = env->GetMethodID(clazz, "", "(J)V");
- if (JNIUtil::isJavaExceptionThrown())
-return NULL;
-}
+ jmethodID ctor = env->GetMethodID(clazz, "", "(J)V");
+ if (JNIUtil::isJavaExceptionThrown())
+return NULL;
jlong cppAddr = this->getCppAddr();
svn commit: r1882113 - /subversion/trunk/COMMITTERS
Author: amiloslavskiy Date: Tue Sep 29 10:07:50 2020 New Revision: 1882113 URL: http://svn.apache.org/viewvc?rev=1882113=rev Log: * COMMITTERS: (amiloslavskiy) Adding myself as partial committer (JavaHL bindings). Modified: subversion/trunk/COMMITTERS Modified: subversion/trunk/COMMITTERS URL: http://svn.apache.org/viewvc/subversion/trunk/COMMITTERS?rev=1882113=1882112=1882113=diff == --- subversion/trunk/COMMITTERS [UTF-8] (original) +++ subversion/trunk/COMMITTERS [UTF-8] Tue Sep 29 10:07:50 2020 @@ -108,6 +108,7 @@ Commit access for specific areas: rschupp Roderich Schupp (Swig bindings) stilor Alexey Neyman (Python bindings, svn-vendor.py) + amiloslavskiy Alexandr Miloslavskiy (JavaHL bindings) Packages:
