impr: thread safety of bridge, monitor and hci

eigen-value · eigen-value · commit 703566fc6b15 · 2025-12-04T17:02:31.000+01:00
test: threading test rpc call
diff --git a/examples/test_rpc_thread/python/main.py b/examples/test_rpc_thread/python/main.py
@@ -0,0 +1,15 @@
+# SPDX-FileCopyrightText: Copyright (C) 2025 ARDUINO SA <http://www.arduino.cc>
+#
+# SPDX-License-Identifier: MPL-2.0
+
+import time
+from arduino.app_utils import *
+
+led_state = False
+
+def loopback(message):
+    time.sleep(1)
+    return message
+
+Bridge.provide("loopback", loopback)
+App.run()
diff --git a/examples/test_rpc_thread/test_rpc_thread.ino b/examples/test_rpc_thread/test_rpc_thread.ino
@@ -0,0 +1,112 @@
+#include <Arduino_RouterBridge.h>
+#include <zephyr/kernel.h>
+
+// Thread config
+#define THREAD_STACK_SIZE 500
+#define THREAD_PRIORITY   5
+
+// 2nd thread definition
+struct k_thread rpc_thread;
+k_thread_stack_t *rpc_stack_area = k_thread_stack_alloc(THREAD_STACK_SIZE, 0);
+
+// RPC call and its mutex
+struct k_mutex mtx;
+
+
+void rpc_thread_entry(void *p1, void *p2, void *p3) {
+    RpcCall<MsgPack::str_t> *call  = reinterpret_cast<RpcCall<MsgPack::str_t>*>(p1);
+
+    k_sleep(K_MSEC(400));
+
+    Serial.println("\n*** Second Thread ***");
+    Serial.println("*** Calling result() again...");
+
+    k_mutex_lock(&mtx, K_FOREVER);
+
+    MsgPack::str_t msg;
+    bool ok = call->result(msg);
+
+    if (ok) {
+        Serial.println("*** Second call succeeded (unexpected!)");
+        Serial.print("**** Message: ");
+        Serial.println(msg.c_str());
+    } else {
+        Serial.println("*** Second call FAILED as expected (already executed)");
+        Serial.print("*** Error Code: 0x");
+        Serial.println(call->getErrorCode(), HEX);
+        Serial.print("*** Error Message: ");
+        Serial.println(call->getErrorMessage().c_str());
+    }
+
+    k_mutex_unlock(&mtx);
+
+    Serial.println("*** Second Thread End ***\n");
+}
+
+
+void setup() {
+    Serial.begin(115200);
+    k_sleep(K_MSEC(2000));
+
+    Serial.println("\n=== Threaded RPC Test ===\n");
+
+    Bridge.begin();
+    Monitor.begin();
+
+    k_mutex_init(&mtx);
+
+    // ---- First result() call in main thread ----
+    Serial.println("--- First thread waits for the other side ---");
+    k_sleep(K_MSEC(5000));
+    Serial.println("--- First result() call (main thread) ---");
+    RpcCall loopback_call = Bridge.call("loopback", "TEST");
+
+    if (loopback_call.isError()) {
+        Serial.println("--- Bridge call before execution");
+        Serial.print("--- Error Code: 0x");
+        Serial.println(loopback_call.getErrorCode(), HEX);
+        Serial.print("--- Error Message: ");
+        Serial.println(loopback_call.getErrorMessage().c_str());
+    }
+
+    MsgPack::str_t msg;
+    k_mutex_lock(&mtx, K_FOREVER);
+    bool ok = loopback_call.result(msg);
+    k_mutex_unlock(&mtx);
+
+    if (ok) {
+        Serial.println("--- First call succeeded.");
+        Serial.print("--- Message: ");
+        Serial.println(msg.c_str());
+    } else {
+        Serial.println("--- First call FAILED (unexpected).");
+    }
+
+    // ---- Launch second thread ----
+    Serial.println("\n--- Starting second thread...");
+
+    k_tid_t rpc_tid = k_thread_create(
+                                        &rpc_thread,
+                                        rpc_stack_area,
+                                        THREAD_STACK_SIZE,
+                                        rpc_thread_entry,
+                                        &loopback_call,
+                                        NULL,
+                                        NULL,
+                                        THREAD_PRIORITY,
+                                        0,
+                                        K_NO_WAIT
+                                    );
+
+    k_thread_name_set(rpc_tid, "test");
+
+    Serial.println("--- Second thread launched.\n");
+
+    //k_thread_join(rpc_tid, K_NO_WAIT); // Works in ISRs only!
+    k_sleep(K_MSEC(5000));
+
+}
+
+void loop() {
+    k_sleep(K_MSEC(5000));
+}
diff --git a/src/bridge.h b/src/bridge.h
@@ -182,6 +182,8 @@ class BridgeClass {
 
         if (is_started()) return true;
 
+        k_mutex_lock(&bridge_mutex, K_FOREVER);
+
         serial_ptr->begin(baud);
         transport = new SerialTransport(*serial_ptr);
 
@@ -196,7 +198,6 @@ class BridgeClass {
                                 UPDATE_THREAD_PRIORITY, 0, K_NO_WAIT);
         k_thread_name_set(upd_tid, "bridge");
 
-        k_mutex_lock(&bridge_mutex, K_FOREVER);
         bool res = false;
         started = call(RESET_METHOD).result(res) && res;
         k_mutex_unlock(&bridge_mutex);
diff --git a/src/hci.h b/src/hci.h
@@ -53,10 +53,12 @@ template<size_t BufferSize=HCI_BUFFER_SIZE> class BridgeHCI {
     bool begin(const char *device = "hci0") {
         k_mutex_init(&hci_mutex);
 
+        k_mutex_lock(&hci_mutex, K_FOREVER);
         // Pre-allocate recv buffer to avoid allocations during recv calls
         recv_buffer.reserve(BufferSize);
 
         if (!(*bridge) && !bridge->begin()) {
+            k_mutex_unlock(&hci_mutex);
             return false;
         }
 
@@ -65,16 +67,18 @@ template<size_t BufferSize=HCI_BUFFER_SIZE> class BridgeHCI {
             initialized = result;
         }
 
-        return initialized;
+        k_mutex_unlock(&hci_mutex);
+        return result;
     }
 
     void end() {
+        k_mutex_lock(&hci_mutex, K_FOREVER);
+
         if (!initialized) {
+            k_mutex_unlock(&hci_mutex);
             return;
         }
 
-        k_mutex_lock(&hci_mutex, K_FOREVER);
-
         bool result;
         bridge->call(HCI_CLOSE_METHOD).result(result);
         initialized = false;
@@ -83,16 +87,20 @@ template<size_t BufferSize=HCI_BUFFER_SIZE> class BridgeHCI {
     }
 
     explicit operator bool() const {
-        return initialized;
+        k_mutex_lock(&hci_mutex, K_FOREVER);
+        bool out = initialized;
+        k_mutex_unlock(&hci_mutex);
+        return out;
     }
 
     int send(const uint8_t *buffer, size_t size) {
+        k_mutex_lock(&hci_mutex, K_FOREVER);
+
         if (!initialized) {
+            k_mutex_unlock(&hci_mutex);
             return -1;
         }
 
-        k_mutex_lock(&hci_mutex, K_FOREVER);
-
         BinaryView send_buffer(buffer, size);
         size_t bytes_sent;
         const bool ret = bridge->call(HCI_SEND_METHOD, send_buffer).result(bytes_sent);
@@ -106,12 +114,13 @@ template<size_t BufferSize=HCI_BUFFER_SIZE> class BridgeHCI {
     }
 
     int recv(uint8_t *buffer, size_t max_size) {
+        k_mutex_lock(&hci_mutex, K_FOREVER);
+
         if (!initialized) {
+            k_mutex_unlock(&hci_mutex);
             return -1;
         }
 
-        k_mutex_lock(&hci_mutex, K_FOREVER);
-
         recv_buffer.clear();
         bool ret = bridge->call(HCI_RECV_METHOD, max_size).result(recv_buffer);
 
@@ -130,12 +139,14 @@ template<size_t BufferSize=HCI_BUFFER_SIZE> class BridgeHCI {
     }
 
     int available() {
+
+        k_mutex_lock(&hci_mutex, K_FOREVER);
+
         if (!initialized) {
+            k_mutex_unlock(&hci_mutex);
             return 0;
         }
 
-        k_mutex_lock(&hci_mutex, K_FOREVER);
-
         bool result;
         bool ret = bridge->call(HCI_AVAIL_METHOD).result(result);
 
diff --git a/src/monitor.h b/src/monitor.h
@@ -42,14 +42,17 @@ class BridgeMonitor: public Stream {
 
         if (is_connected()) return true;
 
+        k_mutex_lock(&monitor_mutex, K_FOREVER);
         bool bridge_started = (*bridge);
         if (!bridge_started) {
             bridge_started = bridge->begin();
         }
 
-        if (!bridge_started) return false;
+        if (!bridge_started) {
+            k_mutex_unlock(&monitor_mutex);
+            return false;
+        }
 
-        k_mutex_lock(&monitor_mutex, K_FOREVER);
         bool out = false;
         _connected = bridge->call(MON_CONNECTED_METHOD).result(out) && out;
         k_mutex_unlock(&monitor_mutex);
@@ -121,9 +124,9 @@ class BridgeMonitor: public Stream {
     }
 
     bool reset() {
+        k_mutex_lock(&monitor_mutex, K_FOREVER);
         bool res;
         bool ok = bridge->call(MON_RESET_METHOD).result(res) && res;
-        k_mutex_lock(&monitor_mutex, K_FOREVER);
         _connected = !ok;
         k_mutex_unlock(&monitor_mutex);
         return ok;
