From 06638d46b85710b4e95c44e60580e02d7a16a375 Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Tue, 24 Jan 2023 17:10:08 +0100 Subject: [PATCH] Remove thread lock. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This make the JNI wrapping *somehow* NOT threadsafe. Few things are threadsafe "by nature": - A lot of native method in libzim are threadsafe. - Wrapping internal are threadsafe (shared_ptr). What is not threadsafe is accessing the SAME java object from different thread. But accessing the same wrapped cpp object using two different java wrapper in two different thread is ok (assuming that the called cpp method is threadsafe itself). --- lib/src/main/cpp/CMakeLists.txt | 1 - lib/src/main/cpp/common.cpp | 4 ---- lib/src/main/cpp/libkiwix/kiwixicu.cpp | 3 --- lib/src/main/cpp/libkiwix/kiwixserver.cpp | 1 - lib/src/main/cpp/libzim/archive.cpp | 3 --- lib/src/main/cpp/libzim/query.cpp | 1 - lib/src/main/cpp/libzim/searcher.cpp | 2 -- lib/src/main/cpp/libzim/suggestion_searcher.cpp | 2 -- lib/src/main/cpp/utils.h | 8 -------- 9 files changed, 25 deletions(-) delete mode 100644 lib/src/main/cpp/common.cpp diff --git a/lib/src/main/cpp/CMakeLists.txt b/lib/src/main/cpp/CMakeLists.txt index d72816b..c335b10 100644 --- a/lib/src/main/cpp/CMakeLists.txt +++ b/lib/src/main/cpp/CMakeLists.txt @@ -10,7 +10,6 @@ add_library( zim_wrapper SHARED - common.cpp libzim/archive.cpp libzim/entry.cpp libzim/entry_iterator.cpp diff --git a/lib/src/main/cpp/common.cpp b/lib/src/main/cpp/common.cpp deleted file mode 100644 index f510798..0000000 --- a/lib/src/main/cpp/common.cpp +++ /dev/null @@ -1,4 +0,0 @@ - -#include - -std::mutex globalLock; diff --git a/lib/src/main/cpp/libkiwix/kiwixicu.cpp b/lib/src/main/cpp/libkiwix/kiwixicu.cpp index 3cc2a72..bc94cf6 100644 --- a/lib/src/main/cpp/libkiwix/kiwixicu.cpp +++ b/lib/src/main/cpp/libkiwix/kiwixicu.cpp @@ -27,12 +27,9 @@ #include "utils.h" #include "zim/tools.h" -std::mutex globalLock; - JNIEXPORT void JNICALL Java_org_kiwix_kiwixlib_JNIICU_setDataDirectory( JNIEnv* env, jclass kclass, jstring dirStr) { - Lock l; try { zim::setICUDataDirectory(TO_C(dirStr)); } catch (...) { diff --git a/lib/src/main/cpp/libkiwix/kiwixserver.cpp b/lib/src/main/cpp/libkiwix/kiwixserver.cpp index e584adb..289e203 100644 --- a/lib/src/main/cpp/libkiwix/kiwixserver.cpp +++ b/lib/src/main/cpp/libkiwix/kiwixserver.cpp @@ -35,7 +35,6 @@ METHOD(void, setNativeServer, jobject jLibrary) { LOG("Attempting to create server"); - Lock l; try { auto library = getPtr(env, jLibrary); SET_PTR(std::make_shared(library.get())); diff --git a/lib/src/main/cpp/libzim/archive.cpp b/lib/src/main/cpp/libzim/archive.cpp index bb49ee3..3392977 100644 --- a/lib/src/main/cpp/libzim/archive.cpp +++ b/lib/src/main/cpp/libzim/archive.cpp @@ -41,7 +41,6 @@ METHOD(void, setNativeArchive, jstring filename) std::string cPath = TO_C(filename); LOG("Attempting to create reader with: %s", cPath.c_str()); - Lock l; try { auto archive = std::make_shared(cPath); SET_PTR(archive); @@ -78,7 +77,6 @@ JNIEXPORT void JNICALL Java_org_kiwix_libzim_Archive_setNativeArchiveByFD( int fd = jni2fd(fdObj, env); LOG("Attempting to create reader with fd: %d", fd); - Lock l; try { auto archive = std::make_shared(fd); SET_PTR(archive); @@ -99,7 +97,6 @@ JNIEXPORT void JNICALL Java_org_kiwix_libzim_Archive_setNativeArchiveEmbedded( int fd = jni2fd(fdObj, env); LOG("Attempting to create reader with fd: %d", fd); - Lock l; try { auto archive = std::make_shared(fd, offset, size); SET_PTR(archive); diff --git a/lib/src/main/cpp/libzim/query.cpp b/lib/src/main/cpp/libzim/query.cpp index 2578d66..6117b4d 100644 --- a/lib/src/main/cpp/libzim/query.cpp +++ b/lib/src/main/cpp/libzim/query.cpp @@ -36,7 +36,6 @@ METHOD(void, setNativeQuery, jstring query) { auto cQuery = TO_C(query); - Lock l; try { auto query = std::make_shared(cQuery); SET_PTR(query); diff --git a/lib/src/main/cpp/libzim/searcher.cpp b/lib/src/main/cpp/libzim/searcher.cpp index cbad5c3..9810ba2 100644 --- a/lib/src/main/cpp/libzim/searcher.cpp +++ b/lib/src/main/cpp/libzim/searcher.cpp @@ -35,8 +35,6 @@ METHOD(void, setNativeSearcher, jobject archive) { - - Lock l; auto cArchive = getPtr(env, archive); try { auto searcher = std::make_shared(*cArchive); diff --git a/lib/src/main/cpp/libzim/suggestion_searcher.cpp b/lib/src/main/cpp/libzim/suggestion_searcher.cpp index 7e34c4f..792447e 100644 --- a/lib/src/main/cpp/libzim/suggestion_searcher.cpp +++ b/lib/src/main/cpp/libzim/suggestion_searcher.cpp @@ -35,8 +35,6 @@ METHOD(void, setNativeSearcher, jobject archive) { - - Lock l; auto cArchive = getPtr(env, archive); try { auto searcher = std::make_shared(*cArchive); diff --git a/lib/src/main/cpp/utils.h b/lib/src/main/cpp/utils.h index 220a317..00529dd 100644 --- a/lib/src/main/cpp/utils.h +++ b/lib/src/main/cpp/utils.h @@ -37,7 +37,6 @@ #define LOG(...) #endif -extern std::mutex globalLock; using std::shared_ptr; // Here is the wrapping structure. @@ -131,13 +130,6 @@ inline jobject buildWrapper(JNIEnv* env, const char* class_name, T&& obj, const #define BUILD_WRAPPER(CLASSNAME, OBJ) buildWrapper(env, CLASSNAME, std::move(OBJ)) -// A mixin class which will lock the globalLock when a instance is created -// This avoid the cration of two instance inheriting from Lock in the same time. -class Lock : public std::unique_lock -{ - public: - Lock() : std::unique_lock(globalLock) { } -}; // --------------------------------------------------------------------------- // Convert things to JAVA