rhbz#1036877: Join Java AsynchronousFinalizer thread well before exit

AsynchronousFinalizer was originally added as
870a4401c0 "INTEGRATION: CWS mtg1: #i57753# Avoid
long-running finalize methods" referring to
<https://issues.apache.org/ooo/show_bug.cgi?id=57753> " Fix JNI-UNO bridge so
that the JVM doesn't run out of memory when a destructor locks the SolarMutex."

It is unclear to me how relevant "If JVMs are getting more mature and should no
longer have problems with long-running finalize methods, this class could be
removed again" really is in practice.  After all, advice on hotspot-gc-devel is
to avoid finalize() if possible
(<http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2014-June/010215.html>
"Re: History of finalizer execution and gc progress?").  So stick with this
approach of home-grown draining for now (where a home-grown approach using
PhantomReferencens would need a dedicated draining thread, too, so would not
have much benefit over the existing code in practice).

Timely termination of AsynchronousFinalizer threads is achieved by using a
dedicated thread per bridge and joining it in the remote bridge's dispose()
resp. the JNI environment's new java_env_dispose.

Change-Id: Idcef2dbf361a1de22f60db73828f59e85711aea7

bridges/source/jni_uno/java/com/sun/star/bridges/jni_uno/JNI_proxy.java

@@ -63,7 +63,7 @@ public final class JNI_proxy implements java.lang.reflect.InvocationHandler
     private final Type m_type;
     private final String m_oid;
     private final Class m_class;
-
+    private final AsynchronousFinalizer m_finalizer;
 
     public static String get_stack_trace( Throwable throwable )
         throws Throwable
@@ -98,16 +98,19 @@ public final class JNI_proxy implements java.lang.reflect.InvocationHandler
     @Override
     protected void finalize()
     {
-        AsynchronousFinalizer.add(new AsynchronousFinalizer.Job() {
+        if (m_finalizer != null) {
-                public void run() throws Throwable {
+            m_finalizer.add(new AsynchronousFinalizer.Job() {
-                    JNI_proxy.this.finalize( m_bridge_handle );
+                    public void run() throws Throwable {
-                }
+                        JNI_proxy.this.finalize( m_bridge_handle );
-            });
+                    }
+                });
+        }
     }
 
     private JNI_proxy(
         long bridge_handle, IEnvironment java_env,
-        long receiver_handle, long td_handle, Type type, String oid )
+        long receiver_handle, long td_handle, Type type, String oid,
+        AsynchronousFinalizer finalizer)
     {
         m_bridge_handle = bridge_handle;
         m_java_env = java_env;
@@ -116,16 +119,19 @@ public final class JNI_proxy implements java.lang.reflect.InvocationHandler
         m_type = type;
         m_oid = oid;
         m_class = m_type.getZClass();
+        m_finalizer = finalizer;
     }
 
     public static Object create(
         long bridge_handle, IEnvironment java_env,
         long receiver_handle, long td_handle, Type type, String oid,
-        java.lang.reflect.Constructor proxy_ctor )
+        java.lang.reflect.Constructor proxy_ctor,
+        AsynchronousFinalizer finalizer)
         throws Throwable
     {
         JNI_proxy handler = new JNI_proxy(
-            bridge_handle, java_env, receiver_handle, td_handle, type, oid );
+            bridge_handle, java_env, receiver_handle, td_handle, type, oid,
+            finalizer);
         Object proxy = proxy_ctor.newInstance( new Object [] { handler } );
         return java_env.registerInterface( proxy, new String [] { oid }, type );
     }

bridges/source/jni_uno/jni_base.h

@@ -125,7 +125,8 @@ class JNI_guarded_context
 
 public:
     inline explicit JNI_guarded_context(
-        JNI_info const * jni_info, ::jvmaccess::UnoVirtualMachine * vm_access )
+        JNI_info const * jni_info,
+        rtl::Reference<jvmaccess::UnoVirtualMachine> const & vm_access)
         : AttachGuard( vm_access->getVirtualMachine() ),
           JNI_context(
               jni_info, AttachGuard::getEnvironment(),

bridges/source/jni_uno/jni_bridge.cxx

@@ -84,8 +84,8 @@ void SAL_CALL Mapping_map_to_uno(
                 static_cast< Mapping const * >( mapping )->m_bridge;
             JNI_guarded_context jni(
                 bridge->m_jni_info,
-                reinterpret_cast< ::jvmaccess::UnoVirtualMachine * >(
+                (static_cast<jni_uno::Context *>(bridge->m_java_env->pContext)
-                    bridge->m_java_env->pContext ) );
+                 ->machine));
 
             JNI_interface_type_info const * info =
                 static_cast< JNI_interface_type_info const * >(
@@ -135,8 +135,9 @@ void SAL_CALL Mapping_map_to_java(
                     static_cast< Mapping const * >( mapping )->m_bridge;
                 JNI_guarded_context jni(
                     bridge->m_jni_info,
-                    reinterpret_cast< ::jvmaccess::UnoVirtualMachine * >(
+                    (static_cast<jni_uno::Context *>(
-                        bridge->m_java_env->pContext ) );
+                        bridge->m_java_env->pContext)
+                     ->machine));
                 jni->DeleteGlobalRef( *ppJavaI );
                 *ppJavaI = 0;
             }
@@ -147,8 +148,8 @@ void SAL_CALL Mapping_map_to_java(
                 static_cast< Mapping const * >( mapping )->m_bridge;
             JNI_guarded_context jni(
                 bridge->m_jni_info,
-                reinterpret_cast< ::jvmaccess::UnoVirtualMachine * >(
+                (static_cast<jni_uno::Context *>(bridge->m_java_env->pContext)
-                    bridge->m_java_env->pContext ) );
+                 ->machine));
 
             JNI_interface_type_info const * info =
                 static_cast< JNI_interface_type_info const * >(
@@ -233,8 +234,7 @@ Bridge::Bridge(
 {
     // bootstrapping bridge jni_info
     m_jni_info = JNI_info::get_jni_info(
-        reinterpret_cast< ::jvmaccess::UnoVirtualMachine * >(
+        static_cast<jni_uno::Context *>(m_java_env->pContext)->machine);
-            m_java_env->pContext ) );
 
     assert(m_java_env != 0);
     assert(m_uno_env != 0);
@@ -409,21 +409,51 @@ OUString JNI_context::get_stack_trace( jobject jo_exc ) const
 
 using namespace ::jni_uno;
 
-extern "C"
+extern "C" {
-{
-namespace
-{
 
-
+void SAL_CALL java_env_dispose(uno_Environment * env) {
-void SAL_CALL java_env_disposing( uno_Environment * java_env )
+    jni_uno::Context * context = static_cast<jni_uno::Context *>(env->pContext);
-    SAL_THROW_EXTERN_C()
+    jobject async;
-{
+    {
-    ::jvmaccess::UnoVirtualMachine * machine =
+        osl::MutexGuard g(context->mutex);
-          reinterpret_cast< ::jvmaccess::UnoVirtualMachine * >(
+        async = context->asynchronousFinalizer;
-              java_env->pContext );
+        context->asynchronousFinalizer = nullptr;
-    java_env->pContext = 0;
+    }
-    machine->release();
+    if (async != nullptr) {
+        try {
+            jvmaccess::VirtualMachine::AttachGuard g(
+                context->machine->getVirtualMachine());
+            JNIEnv * jniEnv = g.getEnvironment();
+            jclass cl = jniEnv->FindClass(
+                "com/sun/star/lib/util/AsynchronousFinalizer");
+            if (cl == nullptr) {
+                jniEnv->ExceptionClear();
+                SAL_WARN("bridges", "exception in FindClass");
+            } else {
+                jmethodID id = jniEnv->GetMethodID(cl, "drain", "()V");
+                if (id == nullptr) {
+                    jniEnv->ExceptionClear();
+                    SAL_WARN("bridges", "exception in GetMethodID");
+                } else {
+                    jniEnv->CallObjectMethod(async, id);
+                    if (jniEnv->ExceptionOccurred()) {
+                        jniEnv->ExceptionClear();
+                        SAL_WARN("bridges", "exception in CallObjectMethod");
+                    }
+                }
+            }
+            jniEnv->DeleteGlobalRef(async);
+        } catch (jvmaccess::VirtualMachine::AttachGuard::CreationException &) {
+            SAL_WARN(
+                "bridges",
+                "jvmaccess::VirtualMachine::AttachGuard::CreationException");
+        }
+    }
 }
+
+void SAL_CALL java_env_disposing(uno_Environment * env) {
+    java_env_dispose(env);
+    delete static_cast<jni_uno::Context *>(env->pContext);
 }
 
 #ifdef DISABLE_DYNLOADING
@@ -434,14 +464,53 @@ void SAL_CALL java_env_disposing( uno_Environment * java_env )
 SAL_DLLPUBLIC_EXPORT void SAL_CALL uno_initEnvironment( uno_Environment * java_env )
     SAL_THROW_EXTERN_C()
 {
+    java_env->pContext = new jni_uno::Context(
+        static_cast<jvmaccess::UnoVirtualMachine *>(java_env->pContext));
+    java_env->dispose = java_env_dispose;
     java_env->environmentDisposing = java_env_disposing;
     java_env->pExtEnv = 0; // no extended support
-    assert(java_env->pContext != 0);
+    try {
-
+        jvmaccess::VirtualMachine::AttachGuard g(
-    ::jvmaccess::UnoVirtualMachine * machine =
+            static_cast<jni_uno::Context *>(java_env->pContext)->machine
-          reinterpret_cast< ::jvmaccess::UnoVirtualMachine * >(
+            ->getVirtualMachine());
-              java_env->pContext );
+        JNIEnv * jniEnv = g.getEnvironment();
-    machine->acquire();
+        jclass cl = jniEnv->FindClass(
+            "com/sun/star/lib/util/AsynchronousFinalizer");
+        if (cl == nullptr) {
+            jniEnv->ExceptionClear();
+            SAL_WARN("bridges", "exception in FindClass");
+                //TODO: report failure
+        } else {
+            jmethodID id = jniEnv->GetMethodID(cl, "<init>", "()V");
+            if (id == nullptr) {
+                jniEnv->ExceptionClear();
+                SAL_WARN("bridges", "exception in GetMethodID");
+                    //TODO: report failure
+            } else {
+                jobject o = jniEnv->NewObject(cl, id);
+                if (o == nullptr) {
+                    jniEnv->ExceptionClear();
+                    SAL_WARN("bridges", "exception in NewObject");
+                        //TODO: report failure
+                } else {
+                    o = jniEnv->NewGlobalRef(o);
+                    if (o == nullptr) {
+                        jniEnv->ExceptionClear();
+                        SAL_WARN("bridges", "exception in NewGlobalRef");
+                            //TODO: report failure
+                    } else {
+                        (static_cast<jni_uno::Context *>(java_env->pContext)->
+                         asynchronousFinalizer)
+                            = o;
+                    }
+                }
+            }
+        }
+    } catch (jvmaccess::VirtualMachine::AttachGuard::CreationException &) {
+        SAL_WARN(
+            "bridges",
+            "jvmaccess::VirtualMachine::AttachGuard::CreationException");
+    }
 }
 
 #ifdef DISABLE_DYNLOADING

bridges/source/jni_uno/jni_bridge.h

@@ -36,6 +36,17 @@
 namespace jni_uno
 {
 
+struct Context: boost::noncopyable {
+    explicit Context(
+        rtl::Reference<jvmaccess::UnoVirtualMachine> const & theMachine):
+        machine(theMachine), asynchronousFinalizer(nullptr)
+    {}
+
+    rtl::Reference<jvmaccess::UnoVirtualMachine> machine;
+    osl::Mutex mutex;
+    jobject asynchronousFinalizer;
+};
+
 //==== holds environments and mappings =========================================
 struct Bridge;
 struct Mapping : public uno_Mapping

bridges/source/jni_uno/jni_info.cxx

@@ -724,7 +724,8 @@ JNI_info::JNI_info(
     m_method_JNI_proxy_create = jni->GetStaticMethodID(
         (jclass) jo_JNI_proxy.get(), "create",
         "(JLcom/sun/star/uno/IEnvironment;JJLcom/sun/star/uno/Type;Ljava/lang"
-        "/String;Ljava/lang/reflect/Constructor;)Ljava/lang/Object;" );
+        "/String;Ljava/lang/reflect/Constructor;"
+        "Lcom/sun/star/lib/util/AsynchronousFinalizer;)Ljava/lang/Object;" );
     jni.ensure_no_exception();
     assert( 0 != m_method_JNI_proxy_create );
     // field JNI_proxy.m_receiver_handle

bridges/source/jni_uno/jni_java2uno.cxx

@@ -58,7 +58,7 @@ jobject Bridge::map_to_java(
             oid.pData, (typelib_InterfaceTypeDescription *)info->m_td.get() );
 
         // create java and register java proxy
-        jvalue args2[ 7 ];
+        jvalue args2[ 8 ];
         acquire();
         args2[ 0 ].j = reinterpret_cast< sal_Int64 >( this );
         (*pUnoI->acquire)( pUnoI );
@@ -69,6 +69,12 @@ jobject Bridge::map_to_java(
         args2[ 4 ].l = info->m_type;
         args2[ 5 ].l = jo_oid.get();
         args2[ 6 ].l = info->m_proxy_ctor;
+        jni_uno::Context * context = static_cast<jni_uno::Context *>(
+            m_java_env->pContext);
+        {
+            osl::MutexGuard g(context->mutex);
+            args2[ 7 ].l = context->asynchronousFinalizer;
+        }
         jo_iface = jni->CallStaticObjectMethodA(
             m_jni_info->m_class_JNI_proxy,
             m_jni_info->m_method_JNI_proxy_create, args2 );
@@ -373,8 +379,8 @@ JNICALL Java_com_sun_star_bridges_jni_1uno_JNI_1proxy_dispatch_1call(
     JNI_context jni(
         jni_info, jni_env,
         static_cast< jobject >(
-            reinterpret_cast< ::jvmaccess::UnoVirtualMachine * >(
+            static_cast<Context *>(bridge->m_java_env->pContext)->machine
-                bridge->m_java_env->pContext )->getClassLoader() ) );
+            ->getClassLoader()));
 
     OUString method_name;
 
@@ -620,8 +626,8 @@ JNICALL Java_com_sun_star_bridges_jni_1uno_JNI_1proxy_finalize__J(
     JNI_context jni(
         jni_info, jni_env,
         static_cast< jobject >(
-            reinterpret_cast< ::jvmaccess::UnoVirtualMachine * >(
+            static_cast<Context *>(bridge->m_java_env->pContext)->machine
-                bridge->m_java_env->pContext )->getClassLoader() ) );
+            ->getClassLoader()));
 
     uno_Interface * pUnoI = reinterpret_cast< uno_Interface * >(
         jni->GetLongField(

bridges/source/jni_uno/jni_uno2java.cxx

@@ -128,8 +128,7 @@ void Bridge::call_java(
     assert( function_pos_offset == 0 || function_pos_offset == 1 );
 
     JNI_guarded_context jni(
-        m_jni_info, reinterpret_cast< ::jvmaccess::UnoVirtualMachine * >(
+        m_jni_info, static_cast<Context *>(m_java_env->pContext)->machine);
-            m_java_env->pContext ) );
 
     // assure fully initialized iface_td:
     ::com::sun::star::uno::TypeDescription iface_holder;
@@ -529,8 +528,7 @@ void SAL_CALL UNO_proxy_free( uno_ExtEnvironment * env, void * proxy )
     {
         JNI_guarded_context jni(
             bridge->m_jni_info,
-            reinterpret_cast< ::jvmaccess::UnoVirtualMachine * >(
+            static_cast<Context *>(bridge->m_java_env->pContext)->machine);
-                bridge->m_java_env->pContext ) );
 
         jni->DeleteGlobalRef( that->m_javaI );
         jni->DeleteGlobalRef( that->m_jo_oid );
@@ -674,8 +672,8 @@ void SAL_CALL UNO_proxy_dispatch(
                     JNI_info const * jni_info = bridge->m_jni_info;
                     JNI_guarded_context jni(
                         jni_info,
-                        reinterpret_cast< ::jvmaccess::UnoVirtualMachine * >(
+                        (static_cast<Context *>(bridge->m_java_env->pContext)
-                            bridge->m_java_env->pContext ) );
+                         ->machine));
 
                     JNI_interface_type_info const * info =
                         static_cast< JNI_interface_type_info const * >(

cppuhelper/source/component_context.cxx

@@ -731,6 +731,22 @@ void ComponentContext::disposing()
     for ( ; iPos != iEnd; ++iPos )
         delete iPos->second;
     m_map.clear();
+
+    // Hack to terminate any JNI bridge's AsynchronousFinalizer thread (as JNI
+    // proxies get finalized with arbitrary delay, so the bridge typically does
+    // not dispose itself early enough before the process exits):
+    uno_Environment ** envs;
+    sal_Int32 envCount;
+    uno_getRegisteredEnvironments(
+        &envs, &envCount, &rtl_allocateMemory, OUString("java").pData);
+    assert(envCount >= 0);
+    assert(envCount == 0 || envs != nullptr);
+    for (sal_Int32 i = 0; i != envCount; ++i) {
+        assert(envs[i] != nullptr);
+        assert(envs[i]->dispose != nullptr);
+        (*envs[i]->dispose)(envs[i]);
+    }
+    rtl_freeMemory(envs);
 }
 
 ComponentContext::ComponentContext(

jurt/com/sun/star/lib/uno/bridges/java_remote/ProxyFactory.java

@@ -57,6 +57,10 @@ final class ProxyFactory {
         }
     }
 
+    public void dispose() throws InterruptedException {
+        asynchronousFinalizer.drain();
+    }
+
     public static XBridge getBridge(Object obj) {
         if (Proxy.isProxyClass(obj.getClass())) {
             InvocationHandler h = Proxy.getInvocationHandler(obj);
@@ -126,13 +130,10 @@ final class ProxyFactory {
 
         @Override
         protected void finalize() {
-            AsynchronousFinalizer.add(new AsynchronousFinalizer.Job() {
+            decrementDebugCount();
+            asynchronousFinalizer.add(new AsynchronousFinalizer.Job() {
                     public void run() throws Throwable {
-                        try {
+                        request("release", null);
-                            request("release", null);
-                        } finally {
-                            decrementDebugCount();
-                        }
                     }
                 });
         }
@@ -187,4 +188,6 @@ final class ProxyFactory {
 
     private final RequestHandler requestHandler;
     private final XBridge bridge;
+    private final AsynchronousFinalizer asynchronousFinalizer =
+        new AsynchronousFinalizer();
 }

jurt/com/sun/star/lib/uno/bridges/java_remote/java_remote_bridge.java

@@ -499,7 +499,12 @@ public class java_remote_bridge
         try {
             _messageDispatcher.terminate();
 
-            _xConnection.close();
+            try {
+                _xConnection.close();
+            } catch (com.sun.star.io.IOException e) {
+                System.err.println(
+                    getClass().getName() + ".dispose - IOException:" + e);
+            }
 
             if (Thread.currentThread() != _messageDispatcher
                 && _messageDispatcher.isAlive())
@@ -519,6 +524,8 @@ public class java_remote_bridge
             // assert _java_environment instanceof java_environment;
             ((java_environment) _java_environment).revokeAllProxies();
 
+            proxyFactory.dispose();
+
             if (DEBUG) {
                 if (_life_count != 0) {
                     System.err.println(getClass().getName()
@@ -535,9 +542,6 @@ public class java_remote_bridge
         } catch (InterruptedException e) {
             System.err.println(getClass().getName()
                                + ".dispose - InterruptedException:" + e);
-        } catch (com.sun.star.io.IOException e) {
-            System.err.println(getClass().getName() + ".dispose - IOException:"
-                               + e);
         }
     }
 

jurt/com/sun/star/lib/util/AsynchronousFinalizer.java

@@ -35,6 +35,37 @@ import java.util.LinkedList;
  * long-running finalize methods, this class could be removed again.</p>
  */
 public final class AsynchronousFinalizer {
+    public AsynchronousFinalizer() {
+        thread = new Thread("AsynchronousFinalizer") {
+                @Override
+                public void run() {
+                    for (;;) {
+                        Job j;
+                        synchronized (queue) {
+                            for (;;) {
+                                if (done) {
+                                    return;
+                                }
+                                if (!queue.isEmpty()) {
+                                    break;
+                                }
+                                try {
+                                    queue.wait();
+                                } catch (InterruptedException e) {
+                                    return;
+                                }
+                            }
+                            j = queue.remove(0);
+                        }
+                        try {
+                            j.run();
+                        } catch (Throwable e) {}
+                    }
+                }
+            };
+        thread.start();
+    }
+
     /**
      * Add a job to be executed asynchronously.
      *
@@ -43,7 +74,7 @@ public final class AsynchronousFinalizer {
      *
      * @param job represents the body of some finalize method; must not be null.
      */
-    public static void add(Job job) {
+    public void add(Job job) {
         synchronized (queue) {
             boolean first = queue.isEmpty();
             queue.add(job);
@@ -53,6 +84,14 @@ public final class AsynchronousFinalizer {
         }
     }
 
+    public void drain() throws InterruptedException {
+        synchronized (queue) {
+            done = true;
+            queue.notify();
+        }
+        thread.join();
+    }
+
     /**
      * An interface to represent bodies of finalize methods.
      *
@@ -65,31 +104,7 @@ public final class AsynchronousFinalizer {
         void run() throws Throwable;
     }
 
-    private static final LinkedList<Job> queue = new LinkedList<Job>();
+    private final LinkedList<Job> queue = new LinkedList<Job>();
-
+    private final Thread thread;
-    static {
+    private boolean done = false;
-        Thread t = new Thread("AsynchronousFinalizer") {
-                @Override
-                public void run() {
-                    for (;;) {
-                        Job j;
-                        synchronized (queue) {
-                            while (queue.isEmpty()) {
-                                try {
-                                    queue.wait();
-                                } catch (InterruptedException e) {}
-                            }
-                            j = queue.remove(0);
-                        }
-                        try {
-                            j.run();
-                        } catch (Throwable e) {}
-                    }
-                }
-            };
-        t.setDaemon(true);
-        t.start();
-    }
-
-    private AsynchronousFinalizer() {}
 }