[PATCH] Fix JavaHL crash in TunnelAgent.CloseTunnelCallback after GC

[Alexandr Miloslavskiy]

Please find test snippet and patch attached.

[[[
Fix JavaHL 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.

[in subversion/bindings/javahl]
* native/OperationContext.cpp
  (OperationContext::openTunnel): Add NewGlobalRef() for kept jobject.
  (OperationContext::closeTunnel): Add a matching DeleteGlobalRef().
]]]

Index: subversion/bindings/javahl/native/OperationContext.cpp
===================================================================
--- subversion/bindings/javahl/native/OperationContext.cpp      (revision 
1880687)
+++ subversion/bindings/javahl/native/OperationContext.cpp      (working copy)
@@ -629,6 +629,12 @@
           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;
 }
 
@@ -656,4 +662,5 @@
       SVN_JNI_CATCH_VOID(mid = env->GetMethodID(cls, "closeTunnel", "()V"));
     }
   SVN_JNI_CATCH_VOID(env->CallVoidMethod(jclosecb, mid));
+  env->DeleteGlobalRef(jclosecb);
 }

import java.io.*;
import java.nio.*;
import java.nio.channels.*;
import java.nio.charset.*;

import org.apache.subversion.javahl.*;
import org.apache.subversion.javahl.callback.*;
import org.apache.subversion.javahl.remote.*;

/**
 * @author Marc Strapetz
 */
public class JavaHL_Crash_RemoteSession_nativeDispose {
        public static void main(String[] args) throws Exception {
                final SVNClient svnClient = new SVNClient();
                final TunnelAgent.CloseTunnelCallback closeTunnelCallback = () 
-> System.out.println("TunnelAgent.CloseTunnelCallback");

                final TunnelAgent tunnelAgent = new TunnelAgent() {
                        @Override
                        public boolean checkTunnel(String name) {
                                return true;
                        }

                        @Override
                        public CloseTunnelCallback 
openTunnel(ReadableByteChannel request, WritableByteChannel response, String 
name, String user, String hostname, int port) throws Throwable {
                                new Thread(() -> {
                                        try {
                                                sleep(100);
                                                
response.write(ByteBuffer.wrap("( success ( 2 2 ( ) ( edit-pipeline svndiff1 
absent-entries commit-revprops depth log-revprops atomic-revprops 
partial-replay inherited-props ephemeral-txnprops file-revs-reverse ) ) ) 
".getBytes(StandardCharsets.UTF_8)));
                                                
request.read(ByteBuffer.allocate(1024));
                                                sleep(100);
                                                
response.write(ByteBuffer.wrap("( success ( ( ANONYMOUS EXTERNAL ) 15:Test 
Repository ) ) ".getBytes(StandardCharsets.UTF_8)));
                                                
request.read(ByteBuffer.allocate(1024));
                                                sleep(100);
                                                
response.write(ByteBuffer.wrap("( success ( ) ) ( success ( 
36:00000000-0000-0000-0000-000000000000 
57:svn+ssh://testu...@domain.com/home/testUser/svnrepos/test ( mergeinfo ) ) ) 
".getBytes(StandardCharsets.UTF_8)));
                                                final ByteBuffer allocate = 
ByteBuffer.allocate(1024);
                                                request.read(allocate);
                                                sleep(100);
                                        }
                                        catch (IOException e) {
                                                e.printStackTrace();
                                        }
                                }).start();

                                return closeTunnelCallback;
                        }
                };
                svnClient.setTunnelAgent(tunnelAgent);

                try {
                        final RemoteFactory f = new RemoteFactory();
                        f.setTunnelAgent(tunnelAgent);

                        final ISVNRemote remote = 
f.openRemoteSession("svn+ssh://testu...@domain.com/home/testUser/svnrepos/test",
 4);

                        // 'OperationContext::openTunnel()' doesn't 
'NewGlobalRef()' callback returned by 'TunnelAgent.openTunnel()'.
                        // When GC runs, it disposes the callback. When JavaHL 
tries to call it in 'remote.dispose()', JVM crashes.
                        for (int index = 0; index < 5; index++) {
                                sleep(100);
                                System.gc();
                        }

                        remote.dispose();
                }
                catch (SubversionException ex) {
                        ex.printStackTrace();
                }
        }

        private static void sleep(int millis) {
                try {
                        Thread.sleep(millis);
                }
                catch (InterruptedException e) {
                        e.printStackTrace();
                }
        }
}