From 9339e6022f6c337f94e9abd1dca895b9ec5fe1a5 Mon Sep 17 00:00:00 2001 From: Jarvis Huang Date: Tue, 7 Dec 2021 13:12:08 +0800 Subject: [PATCH] ipc/android: Refactor and cleanup IPC glue code --- .../freedesktop/monado/ipc/MonadoImpl.java | 7 - .../targets/service-lib/service_target.cpp | 143 ++++++++++-------- 2 files changed, 79 insertions(+), 71 deletions(-) diff --git a/src/xrt/ipc/android/src/main/java/org/freedesktop/monado/ipc/MonadoImpl.java b/src/xrt/ipc/android/src/main/java/org/freedesktop/monado/ipc/MonadoImpl.java index c0f69888b..5cfd1fb8c 100644 --- a/src/xrt/ipc/android/src/main/java/org/freedesktop/monado/ipc/MonadoImpl.java +++ b/src/xrt/ipc/android/src/main/java/org/freedesktop/monado/ipc/MonadoImpl.java @@ -65,7 +65,6 @@ public class MonadoImpl extends IMonado.Stub { @Override public void connect(@NotNull ParcelFileDescriptor parcelFileDescriptor) { - nativeWaitForServerStartup(); int fd = parcelFileDescriptor.getFd(); Log.i(TAG, "connect: given fd " + fd); if (nativeAddClient(fd) != 0) { @@ -115,12 +114,6 @@ public class MonadoImpl extends IMonado.Stub { @SuppressWarnings("JavaJniMissingFunction") private native void nativeStartServer(); - /** - * Native method that waits until the server reports that it is, in fact, started up. - */ - @SuppressWarnings("JavaJniMissingFunction") - private native void nativeWaitForServerStartup(); - /** * Native handling of receiving a surface: should convert it to an ANativeWindow then do stuff * with it. diff --git a/src/xrt/targets/service-lib/service_target.cpp b/src/xrt/targets/service-lib/service_target.cpp index fcaa710c5..a77401b55 100644 --- a/src/xrt/targets/service-lib/service_target.cpp +++ b/src/xrt/targets/service-lib/service_target.cpp @@ -21,78 +21,113 @@ #include "android/android_globals.h" +#include #include #include using wrap::android::view::Surface; +using namespace std::chrono_literals; + namespace { struct IpcServerHelper { public: - IpcServerHelper() {} - - void - waitForStartupComplete() + static IpcServerHelper & + instance() { - std::unique_lock lock{running_mutex}; - running_cond.wait(lock, [&]() { return this->startup_complete; }); + static IpcServerHelper instance; + return instance; } void signalStartupComplete() { - std::unique_lock lock{running_mutex}; + std::unique_lock lock{server_mutex}; startup_complete = true; - running_cond.notify_all(); + startup_cond.notify_all(); + } + + void + startServer() + { + std::unique_lock lock(server_mutex); + if (!server && !server_thread) { + server_thread = std::make_unique( + [&]() { ipc_server_main_android(&server, signalStartupCompleteTrampoline, this); }); + } + } + + static void + signalStartupCompleteTrampoline(void *data) + { + static_cast(data)->signalStartupComplete(); + } + + int32_t + addClient(int fd) + { + if (!waitForStartupComplete()) { + return -1; + } + return ipc_server_mainloop_add_fd(server, &server->ml, fd); + } + + int32_t + shutdownServer() + { + if (!server || !server_thread) { + // Should not happen. + U_LOG_E("service: shutdownServer called before server started up!"); + return -1; + } + + { + // Wait until IPC server stop + std::unique_lock lock(server_mutex); + ipc_server_handle_shutdown_signal(server); + server_thread->join(); + server_thread.reset(nullptr); + server = NULL; + } + + return 0; } private: + IpcServerHelper() {} + + bool + waitForStartupComplete() + { + std::unique_lock lock{server_mutex}; + return startup_cond.wait_for(lock, 2s, [&]() { return server != NULL && startup_complete; }); + } + + //! Reference to the ipc_server, managed by ipc_server_process + struct ipc_server *server = NULL; + //! Mutex for starting thread - std::mutex running_mutex; + std::mutex server_mutex; + + //! Server thread + std::unique_ptr server_thread{}; //! Condition variable for starting thread - std::condition_variable running_cond; + std::condition_variable startup_cond; + + //! Server startup state bool startup_complete = false; }; } // namespace -static struct ipc_server *server = NULL; -static IpcServerHelper *helper = nullptr; -static std::condition_variable helper_ready; -static std::unique_ptr server_thread{}; -static std::mutex server_thread_mutex; - -static void -signalStartupCompleteTrampoline(void *data) -{ - static_cast(data)->signalStartupComplete(); -} - extern "C" void Java_org_freedesktop_monado_ipc_MonadoImpl_nativeStartServer(JNIEnv *env, jobject thiz) { jni::init(env); jni::Object monadoImpl(thiz); - U_LOG_D("service: Called nativeThreadEntry"); + U_LOG_D("service: Called nativeStartServer"); - { - // Start IPC server - std::unique_lock lock(server_thread_mutex); - if (!server && !server_thread) { - helper = new IpcServerHelper(); - helper_ready.notify_all(); - server_thread = std::make_unique( - []() { ipc_server_main_android(&server, signalStartupCompleteTrampoline, helper); }); - } - } -} - -extern "C" JNIEXPORT void JNICALL -Java_org_freedesktop_monado_ipc_MonadoImpl_nativeWaitForServerStartup(JNIEnv *env, jobject thiz) -{ - std::unique_lock lock(server_thread_mutex); - helper_ready.wait(lock, [&] { return helper != nullptr; }); - helper->waitForStartupComplete(); + IpcServerHelper::instance().startServer(); } extern "C" JNIEXPORT jint JNICALL @@ -101,13 +136,9 @@ Java_org_freedesktop_monado_ipc_MonadoImpl_nativeAddClient(JNIEnv *env, jobject jni::init(env); jni::Object monadoImpl(thiz); U_LOG_D("service: Called nativeAddClient with fd %d", fd); - if (server == nullptr) { - // Should not happen. - U_LOG_E("service: nativeAddClient called before service started up!"); - return -1; - } + // We try pushing the fd number to the server. If and only if we get a 0 return, has the server taken ownership. - return ipc_server_mainloop_add_fd(server, &server->ml, fd); + return IpcServerHelper::instance().addClient(fd); } extern "C" JNIEXPORT void JNICALL @@ -127,22 +158,6 @@ Java_org_freedesktop_monado_ipc_MonadoImpl_nativeShutdownServer(JNIEnv *env, job { jni::init(env); jni::Object monadoImpl(thiz); - if (server == nullptr || !server_thread) { - // Should not happen. - U_LOG_E("service: nativeShutdownServer called before service started up!"); - return -1; - } - { - // Wait until IPC server stop - std::unique_lock lock(server_thread_mutex); - ipc_server_handle_shutdown_signal(server); - server_thread->join(); - server_thread.reset(nullptr); - delete helper; - helper = nullptr; - server = NULL; - } - - return 0; + return IpcServerHelper::instance().shutdownServer(); }