diff --git a/bindings/c/fdb_c.cpp b/bindings/c/fdb_c.cpp index a9981003e8..c18b9948a4 100644 --- a/bindings/c/fdb_c.cpp +++ b/bindings/c/fdb_c.cpp @@ -124,6 +124,12 @@ fdb_error_t fdb_stop_network() { CATCH_AND_RETURN( API->stopNetwork(); ); } +extern "C" DLLEXPORT +fdb_error_t fdb_add_shutdown_hook(void (*hook)(void)) { + CATCH_AND_RETURN( API->addShutdownHook(hook); ); +} + + extern "C" DLLEXPORT FDBFuture* fdb_cluster_configure_database( FDBCluster* c, int config_type, int config_mode, uint8_t const* db_name, diff --git a/bindings/c/foundationdb/fdb_c.h b/bindings/c/foundationdb/fdb_c.h index 9a9a60578b..3940970387 100644 --- a/bindings/c/foundationdb/fdb_c.h +++ b/bindings/c/foundationdb/fdb_c.h @@ -89,6 +89,8 @@ extern "C" { DLLEXPORT WARN_UNUSED_RESULT fdb_error_t fdb_stop_network(); + DLLEXPORT WARN_UNUSED_RESULT fdb_error_t fdb_add_shutdown_hook(void (*hook)()); + #pragma pack(push, 4) typedef struct keyvalue { const void* key; diff --git a/bindings/java/fdbJNI.cpp b/bindings/java/fdbJNI.cpp index e58cb84e77..5e8329e7de 100644 --- a/bindings/java/fdbJNI.cpp +++ b/bindings/java/fdbJNI.cpp @@ -41,6 +41,16 @@ static JavaVM* g_jvm = 0; static thread_local JNIEnv* g_thread_jenv = 0; // Defined for the network thread once it is running, and for any thread that has called registerCallback static thread_local jmethodID g_IFutureCallback_call_methodID = 0; +static thread_local bool is_external = false; + +void detachIfExternalThread() { + if(is_external && g_thread_jenv != 0) { + JNIEnv *local_jenv; + g_thread_jenv = 0; + g_IFutureCallback_call_methodID = 0; + g_jvm->DetachCurrentThread(); + } +} void throwOutOfMem(JNIEnv *jenv) { const char *className = "java/lang/OutOfMemoryError"; @@ -121,15 +131,15 @@ static bool findCallbackMethods(JNIEnv *jenv) { static void callCallback( FDBFuture* f, void* data ) { if (g_thread_jenv == 0) { // We are on an external thread and must attach to the JVM. - // Best practice is to detach at some point, but we make one - // of these per external client and cache it, so we *should* - // be fine. + // The shutdown hook will later detach this thread. + is_external = true; if( g_jvm != 0 && g_jvm->AttachCurrentThreadAsDaemon((void **) &g_thread_jenv, JNI_NULL) == JNI_OK ) { if( !findCallbackMethods( g_thread_jenv ) ) { g_thread_jenv->FatalError("FDB: Could not find callback method.\n"); } } else { // Can't call FatalError, because we don't have a pointer to the jenv... + // There will be a segmentation fault from the attempt to call the callback method. fprintf(stderr, "FDB: Could not attach external client thread to the JVM as daemon.\n"); } } @@ -1062,6 +1072,11 @@ JNIEXPORT void JNICALL Java_com_apple_cie_foundationdb_FDB_Network_1run(JNIEnv * return; } + fdb_error_t hookErr = fdb_add_shutdown_hook( &detachIfExternalThread ); + if( hookErr ) { + safeThrow( jenv, getThrowable( jenv, hookErr ) ); + } + fdb_error_t err = fdb_run_network(); if( err ) { safeThrow( jenv, getThrowable( jenv, err ) ); diff --git a/bindings/java/src-completable/test/com/apple/cie/foundationdb/test/TesterArgs.java b/bindings/java/src-completable/test/com/apple/cie/foundationdb/test/TesterArgs.java index a2b27457b2..c0c004c32d 100644 --- a/bindings/java/src-completable/test/com/apple/cie/foundationdb/test/TesterArgs.java +++ b/bindings/java/src-completable/test/com/apple/cie/foundationdb/test/TesterArgs.java @@ -106,7 +106,7 @@ public class TesterArgs { for (j = i + 1; j < args.length && args[j].charAt(0) != '-'; j++) { testsToRun.add(args[j]); } - i = j; + i = j - 1; } else { System.out.println("No tests specified with argument " + arg + "\n"); printUsage(); diff --git a/fdbclient/MultiVersionTransaction.actor.cpp b/fdbclient/MultiVersionTransaction.actor.cpp index daad55dd77..e53b35905a 100644 --- a/fdbclient/MultiVersionTransaction.actor.cpp +++ b/fdbclient/MultiVersionTransaction.actor.cpp @@ -1134,12 +1134,22 @@ void MultiVersionApi::setupNetwork() { THREAD_FUNC_RETURN runNetworkThread(void *param) { try { - ((IClientApi*)param)->runNetwork(); + ((ClientInfo*)param)->api->runNetwork(); } catch(Error &e) { TraceEvent(SevError, "RunNetworkError").error(e); } + std::vector &hooks = ((ClientInfo*)param)->shutdownHooks; + for(auto hook : hooks) { + try { + hook(); + } + catch(Error &e) { + TraceEvent(SevError, "NetworkShutdownHookError").error(e); + } + } + THREAD_RETURN; } @@ -1156,12 +1166,31 @@ void MultiVersionApi::runNetwork() { if(!bypassMultiClientApi) { runOnExternalClients([&handles](Reference client) { if(client->external) { - handles.push_back(g_network->startThread(&runNetworkThread, client->api)); + handles.push_back(g_network->startThread(&runNetworkThread, client.getPtr())); } }); } - localClient->api->runNetwork(); + Error *runErr = NULL; + try { + localClient->api->runNetwork(); + } + catch(Error &e) { + runErr = &e; + } + + for(auto &hook : localClient->shutdownHooks) { + try { + hook(); + } + catch(Error &e) { + TraceEvent(SevError, "NetworkShutdownHookError").error(e); + } + } + + if(runErr != NULL) { + throw *runErr; + } for(auto h : handles) { waitThread(h); @@ -1185,6 +1214,23 @@ void MultiVersionApi::stopNetwork() { } } +void MultiVersionApi::addShutdownHook(void (*hook)(void)) { + lock.enter(); + if(!networkSetup) { + lock.leave(); + throw network_not_setup(); + } + lock.leave(); + + shutdownHooks.push_back(hook); + + if(!bypassMultiClientApi) { + for( auto it : externalClients ) { + it.second->shutdownHooks.push_back(hook); + } + } +} + ThreadFuture> MultiVersionApi::createCluster(const char *clusterFilePath) { lock.enter(); if(!networkSetup) { diff --git a/fdbclient/MultiVersionTransaction.h b/fdbclient/MultiVersionTransaction.h index 7ca1379189..e3f153195b 100644 --- a/fdbclient/MultiVersionTransaction.h +++ b/fdbclient/MultiVersionTransaction.h @@ -320,6 +320,7 @@ struct ClientInfo : ThreadSafeReferenceCounted { std::string libPath; bool external; bool failed; + vector shutdownHooks; ClientInfo() : protocolVersion(0), api(NULL), external(false), failed(true) {} ClientInfo(IClientApi *api) : protocolVersion(0), api(api), libPath("internal"), external(false), failed(false) {} @@ -402,6 +403,7 @@ public: void setupNetwork(); void runNetwork(); void stopNetwork(); + void addShutdownHook(void (*hook)(void)); ThreadFuture> createCluster(const char *clusterFilePath); static MultiVersionApi* api; @@ -441,7 +443,8 @@ private: std::vector>>> options; std::map>> setEnvOptions; volatile bool envOptionsLoaded; + std::vector shutdownHooks; }; -#endif \ No newline at end of file +#endif