Per object c++ mutex locking for the Engine class (java).

All calls to an Engine obj are locking the mutex for their respective object.
A unordered_map holds the mutex per java obj hash. The mutexes are created/deleted in constructor/destructor of an engine object.
A global mutex is being locked for write operations on the unordered map (not thread safe).
Cleanup of #includes

src/basic_api.cc

@@ -1,6 +1,5 @@
 #include <pEp/keymanagement.h>
 #include <pEp/blacklist.h>
-#include <pEp/sync_api.h>
 #include <pEp/Adapter.hh>
 
 #ifndef ANDROID
@@ -20,7 +19,9 @@ JNIEXPORT jbyteArray JNICALL Java_foundation_pEp_jniadapter_Engine_trustwords(
         jobject ident
     )
 {
-    pEpLog("called");
+    pEpLog("called with lock_guard");
+    std::lock_guard<std::mutex> l(*get_engine_java_object_mutex(env, obj));
+
     pEp_identity *_ident = to_identity(env, ident);
     char *words;
     size_t wsize;
@@ -58,7 +59,9 @@ JNIEXPORT jobject JNICALL Java_foundation_pEp_jniadapter_Engine_myself(
         jobject ident
     )
 {
-    pEpLog("called");
+    pEpLog("called with lock_guard");
+    std::lock_guard<std::mutex> l(*get_engine_java_object_mutex(env, obj));
+
     pEp_identity *_ident = to_identity(env, ident);
 
     PEP_STATUS status = ::myself(session(), _ident);
@@ -77,7 +80,9 @@ JNIEXPORT jobject JNICALL Java_foundation_pEp_jniadapter_Engine_updateIdentity(
         jobject ident
     )
 {
-    pEpLog("called");
+    pEpLog("called with lock_guard");
+    std::lock_guard<std::mutex> l(*get_engine_java_object_mutex(env, obj));
+
     pEp_identity *_ident = to_identity(env, ident);
 
     ::update_identity(session(), _ident);
@@ -92,7 +97,9 @@ JNIEXPORT jobject JNICALL Java_foundation_pEp_jniadapter_Engine_setOwnKey(
         jbyteArray fpr
     )
 {
-    pEpLog("called");
+    pEpLog("called with lock_guard");
+    std::lock_guard<std::mutex> l(*get_engine_java_object_mutex(env, obj));
+
     pEp_identity *_ident = to_identity(env, ident);
     char *_fpr = to_string(env, fpr);
 
@@ -113,7 +120,9 @@ JNIEXPORT void JNICALL Java_foundation_pEp_jniadapter_Engine_keyMistrusted(
         jobject ident
     )
 {
-    pEpLog("called");
+    pEpLog("called with lock_guard");
+    std::lock_guard<std::mutex> l(*get_engine_java_object_mutex(env, obj));
+
     pEp_identity *_ident = to_identity(env, ident);
 
     if (_ident->fpr == NULL || _ident->fpr[0] == 0) {
@@ -137,7 +146,9 @@ JNIEXPORT void JNICALL Java_foundation_pEp_jniadapter_Engine_keyResetTrust(
         jobject ident
     )
 {
-    pEpLog("called");
+    pEpLog("called with lock_guard");
+    std::lock_guard<std::mutex> l(*get_engine_java_object_mutex(env, obj));
+
     pEp_identity *_ident = to_identity(env, ident);
 
     if (_ident->fpr == NULL || _ident->fpr[0] == 0) {
@@ -161,7 +172,9 @@ JNIEXPORT void JNICALL Java_foundation_pEp_jniadapter_Engine_trustPersonalKey(
         jobject ident
     )
 {
-    pEpLog("called");
+    pEpLog("called with lock_guard");
+    std::lock_guard<std::mutex> l(*get_engine_java_object_mutex(env, obj));
+
     pEp_identity *_ident = to_identity(env, ident);
 
     if (_ident->fpr == NULL || _ident->fpr[0] == 0) {
@@ -185,7 +198,9 @@ JNIEXPORT void JNICALL Java_foundation_pEp_jniadapter_Engine_trustOwnKey(
         jobject ident
     )
 {
-    pEpLog("called");
+    pEpLog("called with lock_guard");
+    std::lock_guard<std::mutex> l(*get_engine_java_object_mutex(env, obj));
+
     pEp_identity *_ident = to_identity(env, ident);
 
     if (_ident->fpr == NULL || _ident->fpr[0] == 0) {
@@ -202,7 +217,9 @@ JNIEXPORT void JNICALL Java_foundation_pEp_jniadapter_Engine_importKey(
         jbyteArray key
     )
 {
-    pEpLog("called");
+    pEpLog("called with lock_guard");
+    std::lock_guard<std::mutex> l(*get_engine_java_object_mutex(env, obj));
+
     size_t _size = (size_t) env->GetArrayLength(key);
     char *_key = (char *) env->GetByteArrayElements(key, NULL);
 
@@ -226,7 +243,9 @@ JNIEXPORT void JNICALL Java_foundation_pEp_jniadapter_Engine_config_1passive_1mo
         jboolean enable
     )
 {
-    pEpLog("called");
+    pEpLog("called with lock_guard");
+    std::lock_guard<std::mutex> l(*get_engine_java_object_mutex(env, obj));
+
     ::config_passive_mode(session(), (bool)enable);
 }
 
@@ -237,7 +256,9 @@ JNIEXPORT void JNICALL Java_foundation_pEp_jniadapter_Engine_config_1unencrypted
         jboolean enable
     )
 {
-    pEpLog("called");
+    pEpLog("called with lock_guard");
+    std::lock_guard<std::mutex> l(*get_engine_java_object_mutex(env, obj));
+
     ::config_unencrypted_subject(session(), (bool)enable);
 }
 
@@ -247,7 +268,9 @@ JNIEXPORT void JNICALL Java_foundation_pEp_jniadapter_Engine_blacklist_1add(
         jbyteArray fpr
     )
 {
-    pEpLog("called");
+    pEpLog("called with lock_guard");
+    std::lock_guard<std::mutex> l(*get_engine_java_object_mutex(env, obj));
+
     char *_fpr = to_string(env, fpr);
 
     if(_fpr == NULL){
@@ -269,7 +292,9 @@ JNIEXPORT void JNICALL Java_foundation_pEp_jniadapter_Engine_blacklist_1delete(
         jbyteArray fpr
     )
 {
-    pEpLog("called");
+    pEpLog("called with lock_guard");
+    std::lock_guard<std::mutex> l(*get_engine_java_object_mutex(env, obj));
+
     char *_fpr = to_string(env, fpr);
 
     if(_fpr == NULL){
@@ -291,7 +316,9 @@ JNIEXPORT jboolean JNICALL Java_foundation_pEp_jniadapter_Engine_blacklist_1is_1
         jbyteArray fpr
     )
 {
-    pEpLog("called");
+    pEpLog("called with lock_guard");
+    std::lock_guard<std::mutex> l(*get_engine_java_object_mutex(env, obj));
+
     char *_fpr = to_string(env, fpr);
     bool _listed = 0;
 
@@ -316,7 +343,9 @@ JNIEXPORT jbyteArray JNICALL Java_foundation_pEp_jniadapter_Engine_getCrashdumpL
         jint maxlines
     )
 {
-    pEpLog("called");
+    pEpLog("called with lock_guard");
+    std::lock_guard<std::mutex> l(*get_engine_java_object_mutex(env, obj));
+
     int _maxlines = (int) maxlines;
     char *_logdata;
 

src/foundation_pEp_jniadapter_AbstractEngine.cc

@@ -1,8 +1,5 @@
 #include "foundation_pEp_jniadapter_AbstractEngine.h"
-#include <stdexcept>
 #include <unistd.h>
-#include <assert.h>
-#include <pthread.h>
 #include <pEp/keymanagement.h>
 #include <pEp/message_api.h>
 #include <pEp/sync_api.h>
@@ -28,7 +25,7 @@ jmethodID notifyHandShakeMethodID = nullptr;
 jmethodID needsFastPollMethodID = nullptr;
 jmethodID method_values = nullptr;
 
-jobject obj = nullptr;
+jobject objj = nullptr;
 
 jclass messageClass = nullptr;
 jclass identityClass = nullptr;;
@@ -105,11 +102,11 @@ PEP_STATUS messageToSend(message *msg)
     pEpLog("############### messageToSend() called");
     jobject msg_ = nullptr;
 
-    assert(messageClass && messageConstructorMethodID && obj && messageToSendMethodID);
+    assert(messageClass && messageConstructorMethodID && objj && messageToSendMethodID);
 
     msg_ = o.env()->NewObject(messageClass, messageConstructorMethodID, (jlong) msg);
 
-    PEP_STATUS status = (PEP_STATUS) o.env()->CallIntMethod(obj, messageToSendMethodID, msg_);
+    PEP_STATUS status = (PEP_STATUS) o.env()->CallIntMethod(objj, messageToSendMethodID, msg_);
     if (o.env()->ExceptionCheck()) {
         o.env()->ExceptionDescribe();
         status = PEP_UNKNOWN_ERROR;
@@ -158,9 +155,9 @@ PEP_STATUS notifyHandshake(pEp_identity *me, pEp_identity *partner, sync_handsha
         }
     }
 
-    assert(obj && notifyHandShakeMethodID);
+    assert(objj && notifyHandShakeMethodID);
 
-    PEP_STATUS status = (PEP_STATUS) o.env()->CallIntMethod(obj, notifyHandShakeMethodID, me_, partner_, signal_);
+    PEP_STATUS status = (PEP_STATUS) o.env()->CallIntMethod(objj, notifyHandShakeMethodID, me_, partner_, signal_);
     if (o.env()->ExceptionCheck()) {
         o.env()->ExceptionClear();
         return PEP_UNKNOWN_ERROR;
@@ -179,14 +176,18 @@ JNIEXPORT void JNICALL Java_foundation_pEp_jniadapter_AbstractEngine_init(
     )
 {
     pEpLog("called");
+    std::lock_guard<std::mutex> l(global_mutex); // global mutex for write access to <unordered_map>
+
     if (first) {
         pEpLog("first Engine instance");
         first = false;
         env->GetJavaVM(&jvm);
         jni_init();
-        obj = env->NewGlobalRef(me);
+        objj = env->NewGlobalRef(me);
         Adapter::_messageToSend = messageToSend;
     }
+
+    create_engine_java_object_mutex(env, me);  // Create a mutex per java object
     Adapter::session();
 }
 
@@ -196,24 +197,30 @@ JNIEXPORT void JNICALL Java_foundation_pEp_jniadapter_AbstractEngine_release(
     )
 {
     pEpLog("called");
+    std::lock_guard<std::mutex> l(global_mutex);  // global mutex for write access to <unordered_map>
+    release_engine_java_object_mutex(env, me);
     Adapter::session(pEp::Adapter::release);
 }
 
 JNIEXPORT jstring JNICALL Java_foundation_pEp_jniadapter_AbstractEngine_getVersion(
         JNIEnv *env,
-        jobject
+        jobject me
     )
 {
-    pEpLog("called");
+    pEpLog("called with lock_guard");
+    std::lock_guard<std::mutex> l(*get_engine_java_object_mutex(env, me));
+
     return env->NewStringUTF(::get_engine_version());
 }
 
 JNIEXPORT jstring JNICALL Java_foundation_pEp_jniadapter_AbstractEngine_getProtocolVersion(
         JNIEnv *env,
-        jobject
+        jobject me
     )
 {
-    pEpLog("called");
+    pEpLog("called with lock_guard");
+    std::lock_guard<std::mutex> l(*get_engine_java_object_mutex(env, me));
+
     return env->NewStringUTF(::get_protocol_version());
 }
 
@@ -259,7 +266,9 @@ JNIEXPORT void JNICALL Java_foundation_pEp_jniadapter_AbstractEngine_startKeyser
         jobject me
     )
 {
-    pEpLog("called");
+    pEpLog("called with lock_guard");
+    std::lock_guard<std::mutex> l(*get_engine_java_object_mutex(env, me));
+
     pthread_t *thread = nullptr;
     locked_queue< pEp_identity * > *queue = nullptr;
 
@@ -296,7 +305,9 @@ JNIEXPORT void JNICALL Java_foundation_pEp_jniadapter_AbstractEngine_stopKeyserv
         jobject me
     )
 {
-    pEpLog("called");
+    pEpLog("called with lock_guard");
+    std::lock_guard<std::mutex> l(*get_engine_java_object_mutex(env, me));
+
     pthread_t *thread = nullptr;
     locked_queue< pEp_identity * > *queue = nullptr;
 
@@ -333,7 +344,9 @@ JNIEXPORT void JNICALL Java_foundation_pEp_jniadapter_AbstractEngine_startSync(
         jobject me
     )
 {
-    pEpLog("called");
+    pEpLog("called with lock_guard");
+    std::lock_guard<std::mutex> l(*get_engine_java_object_mutex(env, me));
+
     pEpLog("######## starting sync");
     try {
         Adapter::startup<JNISync>(messageToSend, notifyHandshake, &o, &JNISync::onSyncStartup, &JNISync::onSyncShutdown);
@@ -349,7 +362,9 @@ JNIEXPORT void JNICALL Java_foundation_pEp_jniadapter_AbstractEngine_stopSync(
         jobject me
     )
 {
-    pEpLog("called");
+    pEpLog("called with lock_guard");
+    std::lock_guard<std::mutex> l(*get_engine_java_object_mutex(env, me));
+
     Adapter::shutdown();
 }
 
@@ -358,7 +373,9 @@ JNIEXPORT jboolean JNICALL Java_foundation_pEp_jniadapter_AbstractEngine_isSyncR
         jobject me
     )
 {
-    pEpLog("called");
+    pEpLog("called with lock_guard");
+    std::lock_guard<std::mutex> l(*get_engine_java_object_mutex(env, me));
+
     return (jboolean) Adapter::is_sync_running();
 }
 

src/foundation_pEp_jniadapter__Blob.cc

@@ -1,11 +1,6 @@
-#include <pEp/bloblist.h>
 #include <pEp/sync_codec.h>
 #include <pEp/distribution_codec.h>
 #include <pEp/utils.hh>
-#include <iostream>
-#include <cstring>
-#include <cassert>
-
 #include "jniutils.hh"
 #include "throw_pEp_exception.hh"
 #include "foundation_pEp_jniadapter__Blob.h"

src/gen_cpp_Engine.ysl2

@@ -9,14 +9,11 @@ tstylesheet {
 
     template "interface" document("foundation_pEp_jniadapter_{@name}.cc", "text")
         ||
-        #include <assert.h>
-        #include <pEp/message_api.h>
         #include <pEp/blacklist.h>
         #include <pEp/openpgp_compat.h>
         #include <pEp/key_reset.h>
         #include <pEp/Adapter.hh>
-        #include <pEp/utils.hh>
-        #include "foundation_pEp_jniadapter_«@name».h"
+        #include "foundation_pEp_jniadapter_Engine.h"
         #include "throw_pEp_exception.hh"
         #include "jniutils.hh"
 
@@ -50,7 +47,9 @@ tstylesheet {
                 jobject obj`apply "parm[in|inout]", mode=sig`
             )
         {
-            pEpLog("called");
+            pEpLog("called with lock_guard");
+            std::lock_guard<std::mutex> l(*get_engine_java_object_mutex(env, obj));
+
         ||
 
         apply "parm[in|inout]", mode=in;

src/gen_cpp_Message.ysl2

@@ -12,15 +12,8 @@ tstylesheet {
 
         document("foundation_pEp_jniadapter_{$jname}.cc", "text") {
             ||
-            #include <exception>
-            #include <new>
-            #include <stdint.h>
-            #include <stdlib.h>
-            #include <assert.h>
-            #include <pEp/«@name».h>
             #include <pEp/mime.h>
             #include <pEp/utils.hh>
-
             #include "jniutils.hh"
             #include "throw_pEp_exception.hh"
             #include "foundation_pEp_jniadapter_«$jname».h"

src/identity_api.cc

@@ -1,7 +1,4 @@
-#include <pEp/message_api.h>
 #include <pEp/utils.hh>
-#include "jniutils.hh"
-
 
 extern "C" {
 

src/jniutils.cc

@@ -1,11 +1,5 @@
 #include "jniutils.hh"
-#include <stdexcept>
-#include <typeinfo>
-#include <time.h>
-#include <stdlib.h>
-#include <assert.h>
-#include <string>
-
+#include <pEp/utils.hh>
 #ifndef __LP64__
 #include <time64.h>
 #define time_t time64_t
@@ -17,6 +11,53 @@
 
 namespace pEp {
     namespace JNIAdapter {
+        std::mutex global_mutex;
+        std::unordered_map<int, std::mutex*> engine_objhash_mutex;
+
+        std::mutex* get_engine_java_object_mutex(
+                JNIEnv *env,
+                jobject obj
+            )
+        {
+            int engine_obj_hash = (int)callIntMethod(env, obj, "hashCode");
+            assert(engine_obj_hash);
+            std::mutex *engine_obj_mutex = engine_objhash_mutex.at(engine_obj_hash);
+            pEpLog(engine_obj_mutex << " with native_handle: " << engine_obj_mutex->native_handle() << " for java object hash: " << engine_obj_hash);
+            assert(engine_obj_mutex);
+            return engine_obj_mutex;
+        }
+
+        void create_engine_java_object_mutex(
+                JNIEnv *env,
+                jobject obj
+            )
+        {
+            int engine_obj_hash = (int)callIntMethod(env, obj, "hashCode");
+            assert(engine_obj_hash);
+            std::mutex *engine_obj_mutex = new std::mutex();
+            pEpLog(engine_obj_mutex << " with native_handle: " << engine_obj_mutex->native_handle() << " for java object hash: " << engine_obj_hash);
+            assert(engine_obj_mutex);
+            if(get_engine_java_object_mutex(env, obj) != nullptr ) {
+                pEpLog("Fatal: mutex already existing for this object");
+                assert(0);
+            }
+            engine_objhash_mutex.insert(std::make_pair(engine_obj_hash, engine_obj_mutex ));
+        }
+
+        void release_engine_java_object_mutex(
+                JNIEnv *env,
+                jobject obj
+            )
+        {
+            int engine_obj_hash = (int)callIntMethod(env, obj, "hashCode");
+            assert(engine_obj_hash);
+            std::mutex *engine_obj_mutex = engine_objhash_mutex.at(engine_obj_hash);
+            pEpLog(engine_obj_mutex << " with native_handle: " << engine_obj_mutex->native_handle() << " for java object hash: " << engine_obj_hash);
+            assert(engine_obj_mutex);
+            engine_objhash_mutex.erase(engine_obj_hash);
+            delete engine_obj_mutex;
+        }
+
         jclass findClass(JNIEnv *env, const char *classname)
         {
             jclass clazz = env->FindClass(classname);

src/jniutils.hh

@@ -1,8 +1,6 @@
 #pragma once
-
-#include <list>
-#include <pthread.h>
-#include <errno.h>
+#include <unordered_map>
+#include <thread>
 #include <jni.h>
 #include <pEp/stringpair.h>
 #include <pEp/identity_list.h>
@@ -15,11 +13,42 @@
 #define  LOG_TAG    "pEpJNIAdapter"
 #define  LOGD(...)  __android_log_print(ANDROID_LOG_ERROR,LOG_TAG,__VA_ARGS__)
 #else
-#define  LOGD(...)
+#define  LOGD(...) do{}while(0)
 #endif
 
 namespace pEp {
     namespace JNIAdapter {
+
+        // Global mutex needs to be locked in all constructors which insert their own mutex object
+        // into the unordered_map (which is thread safe for read, but not for write)
+        extern std::mutex global_mutex;
+
+        // Stores mutex per java object
+        extern std::unordered_map<int, std::mutex*> engine_objhash_mutex;
+
+        // needs to be called after create_engine_java_object_mutex()
+        // and before release_engine_java_object_mutex()
+        // Thread safe
+        std::mutex* get_engine_java_object_mutex(
+                JNIEnv *env,
+                jobject me
+            );
+
+        // Needs to be called exactly once per obj, in the constructor of the obj
+        // You need to lock a global mutex before calling this function (write to unordered_map)
+        void create_engine_java_object_mutex(
+                JNIEnv *env,
+                jobject me
+            );
+
+        // Needs to be called exactly once per obj, in the destructor of this obj
+        // You need to lock a global mutex before calling this function (write to unordered_map)
+        void release_engine_java_object_mutex(
+                JNIEnv *env,
+                jobject me
+            );
+
+
         jclass findClass(JNIEnv *env, const char *classname);
 
         jfieldID getFieldID(

test/java/foundation/pEp/jniadapter/test/jni88/TestMain.java

@@ -89,8 +89,8 @@ class TestThread extends Thread {
 class TestMain {
     public static void main(String[] args) {
         // Test parameters
-        boolean useSharedEngine = true;
-        int numThreads = 2;
+        boolean useSharedEngine = false;
+        int numThreads = 200;
         int numIters = 1000000000;
 
         Engine sharedEngine = null;

test/java/foundation/pEp/jniadapter/test/jni92/TestMain.java

@@ -4,6 +4,7 @@ import foundation.pEp.jniadapter.*;
 
 import java.lang.Thread;
 import java.util.Vector;
+import java.util.function.Consumer;
 
 
 /*
@@ -14,13 +15,15 @@ https://pep.foundation/jira/browse/JNI-81
 */
 
 class TestThread extends Thread {
-    TestThread(String threadName) {
+    int nrEngines = 1;
+    TestThread(String threadName, int nrEngines) {
         Thread.currentThread().setName(threadName);
+        this.nrEngines = nrEngines;
     }
 
     public void run() {
         TestUtils.logH1( "Thread Starting");
-        TestMain.TestMainRun(2);
+        TestMain.TestMainRun(nrEngines);
     }
 }
 
@@ -31,7 +34,7 @@ class TestMain {
         Engine e;
         TestUtils.logH2("Creating new Engine");
         e = new Engine();
-        TestUtils.log("Engine created\n");
+        TestUtils.log("Engine created with java object hash: " + e.hashCode());
         return e;
     }
 
@@ -43,36 +46,48 @@ class TestMain {
         return ev;
     }
 
-    public static void own_identities_retrieve_on_EngineVector(Vector<Engine> ev) {
+    public static void engineConsumer(Vector<Engine> ev, Consumer<Engine> ec) {
         ev.forEach(e -> {
-            TestUtils.logH2("own_identities_retrieve()");
-            e.own_identities_retrieve();
-            TestUtils.log("\n");
+            TestUtils.logH2("engineConsumer: on engine object hash: " + e.hashCode());
+            ec.accept(e);
         });
     }
 
 
-
     public static void TestMainRun(int nrEngines) {
         Vector<Engine> engineVector = TestMain.createEngines(nrEngines);
 //        TestUtils.sleep(200);
-        TestMain.own_identities_retrieve_on_EngineVector(engineVector);
+        Consumer<Engine> c = (e) -> {
+//           Vector<Identity> v = e.own_identities_retrieve();
+//
+//            TestUtils.log("own idents: " + v.size());
+//            v.forEach( i -> {
+//                TestUtils.log(TestUtils.identityToString(i));
+//            });
+            e.getVersion();
+//            e.OpenPGP_list_keyinfo("");
+        };
+//        TestMain.engineConsumer(engineVector, c);
     }
 
-    public static void main(String[] args) throws Exception {
+    public static void main(String[] args) {
         TestUtils.logH1("JNI-92 Starting");
+
+        int nrTestruns = 1000;
         boolean multiThreaded = true;
-        int nrEngines = 3;
+        int nrThreads = 200;
+        int nrEnginesPerThread = 1000;
 
+        for (int run = 0; run < nrTestruns; run++ ) {
+             TestUtils.logH1("Testrun Nr: " + run);
             if (!multiThreaded) {
                 // Single Threaded
-            TestMainRun(nrEngines);
+                TestMainRun(nrEnginesPerThread);
             } else {
                 // Mutli Threaded
                 Vector<TestThread> tts = new Vector<TestThread>();
-            int nrThreads = nrEngines;
                 for (int i = 0; i < nrThreads; i++) {
-                tts.add(new TestThread("TestThread-" + i));
+                    tts.add(new TestThread("TestThread-" + i, nrEnginesPerThread));
 //                TestUtils.sleep(200);
                 }
 
@@ -90,6 +105,7 @@ class TestMain {
                 });
             }
         }
+    }
 }