svn commit: r1882522 - /subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/OperationContext.cpp

[amiloslavskiy]

Author: amiloslavskiy
Date: Thu Oct 15 10:15:47 2020
New Revision: 1882522

URL: http://svn.apache.org/viewvc?rev=1882522&view=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&r1=1882521&r2=1882522&view=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<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 +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<TunnelContext*>(tunnel_context);
+  jobject jclosecb = tc->jclosecb;
+  delete tc;
+
+  JNIEnv *env = JNIUtil::getEnv();
+  if (JNIUtil::isJavaExceptionThrown())
+    return;
+
+  if (jclosecb)
+    callCloseTunnelCallback(env, jclosecb);
 }