From 6ddde10b4006ece33bc358a391a13e108a35f6fa Mon Sep 17 00:00:00 2001 From: Stephan Bergmann Date: Tue, 9 Dec 2014 12:51:40 +0100 Subject: rhbz#1036877: Join Java AsynchronousFinalizer thread well before exit AsynchronousFinalizer was originally added as 870a4401c05beec3d31c1f6055a64591edd0a9d9 "INTEGRATION: CWS mtg1: #i57753# Avoid long-running finalize methods" referring to " 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 ( "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 --- .../com/sun/star/bridges/jni_uno/JNI_proxy.java | 24 ++-- bridges/source/jni_uno/jni_base.h | 3 +- bridges/source/jni_uno/jni_bridge.cxx | 123 ++++++++++++++++----- bridges/source/jni_uno/jni_bridge.h | 11 ++ bridges/source/jni_uno/jni_info.cxx | 3 +- bridges/source/jni_uno/jni_java2uno.cxx | 16 ++- bridges/source/jni_uno/jni_uno2java.cxx | 10 +- 7 files changed, 141 insertions(+), 49 deletions(-) (limited to 'bridges/source') diff --git a/bridges/source/jni_uno/java/com/sun/star/bridges/jni_uno/JNI_proxy.java b/bridges/source/jni_uno/java/com/sun/star/bridges/jni_uno/JNI_proxy.java index 732d30b925b5..12817e57eeda 100644 --- a/bridges/source/jni_uno/java/com/sun/star/bridges/jni_uno/JNI_proxy.java +++ b/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() { - public void run() throws Throwable { - JNI_proxy.this.finalize( m_bridge_handle ); - } - }); + if (m_finalizer != null) { + m_finalizer.add(new AsynchronousFinalizer.Job() { + 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 ); } diff --git a/bridges/source/jni_uno/jni_base.h b/bridges/source/jni_uno/jni_base.h index 5e29c12674b0..25dee559c98a 100644 --- a/bridges/source/jni_uno/jni_base.h +++ b/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 const & vm_access) : AttachGuard( vm_access->getVirtualMachine() ), JNI_context( jni_info, AttachGuard::getEnvironment(), diff --git a/bridges/source/jni_uno/jni_bridge.cxx b/bridges/source/jni_uno/jni_bridge.cxx index da3a33be415d..809eb3b96c90 100644 --- a/bridges/source/jni_uno/jni_bridge.cxx +++ b/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 * >( - bridge->m_java_env->pContext ) ); + (static_cast(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 * >( - bridge->m_java_env->pContext ) ); + (static_cast( + 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 * >( - bridge->m_java_env->pContext ) ); + (static_cast(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 * >( - m_java_env->pContext ) ); + static_cast(m_java_env->pContext)->machine); 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" -{ -namespace -{ - +extern "C" { -void SAL_CALL java_env_disposing( uno_Environment * java_env ) - SAL_THROW_EXTERN_C() -{ - ::jvmaccess::UnoVirtualMachine * machine = - reinterpret_cast< ::jvmaccess::UnoVirtualMachine * >( - java_env->pContext ); - java_env->pContext = 0; - machine->release(); +void SAL_CALL java_env_dispose(uno_Environment * env) { + jni_uno::Context * context = static_cast(env->pContext); + jobject async; + { + osl::MutexGuard g(context->mutex); + async = context->asynchronousFinalizer; + context->asynchronousFinalizer = nullptr; + } + 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(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(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); - - ::jvmaccess::UnoVirtualMachine * machine = - reinterpret_cast< ::jvmaccess::UnoVirtualMachine * >( - java_env->pContext ); - machine->acquire(); + try { + jvmaccess::VirtualMachine::AttachGuard g( + static_cast(java_env->pContext)->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"); + //TODO: report failure + } else { + jmethodID id = jniEnv->GetMethodID(cl, "", "()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(java_env->pContext)-> + asynchronousFinalizer) + = o; + } + } + } + } + } catch (jvmaccess::VirtualMachine::AttachGuard::CreationException &) { + SAL_WARN( + "bridges", + "jvmaccess::VirtualMachine::AttachGuard::CreationException"); + } } #ifdef DISABLE_DYNLOADING diff --git a/bridges/source/jni_uno/jni_bridge.h b/bridges/source/jni_uno/jni_bridge.h index e1a2aadecbb6..7c5bf379a301 100644 --- a/bridges/source/jni_uno/jni_bridge.h +++ b/bridges/source/jni_uno/jni_bridge.h @@ -36,6 +36,17 @@ namespace jni_uno { +struct Context: boost::noncopyable { + explicit Context( + rtl::Reference const & theMachine): + machine(theMachine), asynchronousFinalizer(nullptr) + {} + + rtl::Reference machine; + osl::Mutex mutex; + jobject asynchronousFinalizer; +}; + //==== holds environments and mappings ========================================= struct Bridge; struct Mapping : public uno_Mapping diff --git a/bridges/source/jni_uno/jni_info.cxx b/bridges/source/jni_uno/jni_info.cxx index 038d971dc271..aa910f12d5b2 100644 --- a/bridges/source/jni_uno/jni_info.cxx +++ b/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 diff --git a/bridges/source/jni_uno/jni_java2uno.cxx b/bridges/source/jni_uno/jni_java2uno.cxx index 9aaa02f13994..086399e26dd0 100644 --- a/bridges/source/jni_uno/jni_java2uno.cxx +++ b/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( + 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 * >( - bridge->m_java_env->pContext )->getClassLoader() ) ); + static_cast(bridge->m_java_env->pContext)->machine + ->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 * >( - bridge->m_java_env->pContext )->getClassLoader() ) ); + static_cast(bridge->m_java_env->pContext)->machine + ->getClassLoader())); uno_Interface * pUnoI = reinterpret_cast< uno_Interface * >( jni->GetLongField( diff --git a/bridges/source/jni_uno/jni_uno2java.cxx b/bridges/source/jni_uno/jni_uno2java.cxx index d84acf3a931c..c50be5de9a37 100644 --- a/bridges/source/jni_uno/jni_uno2java.cxx +++ b/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_java_env->pContext ) ); + m_jni_info, static_cast(m_java_env->pContext)->machine); // 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 * >( - bridge->m_java_env->pContext ) ); + static_cast(bridge->m_java_env->pContext)->machine); 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 * >( - bridge->m_java_env->pContext ) ); + (static_cast(bridge->m_java_env->pContext) + ->machine)); JNI_interface_type_info const * info = static_cast< JNI_interface_type_info const * >( -- cgit