fix: add GIL for pybind11::bytes object accessing (AimRT#35)

* fix: add GIL for pybind11::bytes object accessing Protect the access to the pybind11::bytes object with a GIL lock to avoid potential memory errors, and unify the handling of empty and non-empty strings. Eliminate unused functions for empty byte objects in the export channel and export RPC modules to enhance code clarity. * docs: resolve occasional server crash during multi-threaded RPC calls Address a rare issue causing server crashes in aimrt_py under multi-threaded RPC operations, enhancing stability and reliability.
whjgwf · Oct 17, 2024 · 5ca412c · 5ca412c
1 parent fabac84
commit 5ca412c
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 39 deletions.
diff --git a/document/sphinx-cn/release_notes/v0_9_0.md b/document/sphinx-cn/release_notes/v0_9_0.md
@@ -8,6 +8,7 @@
   - 现在可以传入 zenoh 原生配置；
 - mqtt 新增配置项以支持加密传输；
 - 新增了第三方库 asio，runtime::core 不再引用 boost，改为引用独立的 asio 库，以减轻依赖；
+- 修复 aimrt_py 多线程 rpc 调用 server 端概率性崩溃的问题；
 
 **次要修改**：
 - 缩短了一些 examples 的文件路径长度；

diff --git a/src/runtime/python_runtime/export_channel.h b/src/runtime/python_runtime/export_channel.h
@@ -41,11 +41,6 @@ inline void ExportPublisherRef(pybind11::object m) {
       .def("Publish", &PyPublish);
 }
 
-inline const pybind11::bytes& GetChannelEmptyPyBytes() {
-  static const pybind11::bytes kChannelEmptyPyBytes = pybind11::bytes("");
-  return kChannelEmptyPyBytes;
-}
-
 inline bool PySubscribe(
     aimrt::channel::SubscriberRef& subscriber_ref,
     const std::shared_ptr<const PyTypeSupport>& msg_type_support,
@@ -64,17 +59,13 @@ inline bool PySubscribe(
         const std::string& msg_buf = *static_cast<const std::string*>(msg_ptr);
         auto ctx_ref = aimrt::channel::ContextRef(ctx_ptr);
 
-        if (msg_buf.empty()) [[unlikely]] {
-          callback(ctx_ref.GetSerializationType(), GetChannelEmptyPyBytes());
-        } else {
-          auto msg_buf_bytes = pybind11::bytes(msg_buf);
+        pybind11::gil_scoped_acquire acquire;
 
-          callback(ctx_ref.GetSerializationType(), msg_buf_bytes);
+        auto msg_buf_bytes = pybind11::bytes(msg_buf);
+        callback(ctx_ref.GetSerializationType(), msg_buf_bytes);
+        msg_buf_bytes.release();
 
-          pybind11::gil_scoped_acquire acquire;
-          msg_buf_bytes.release();
-          pybind11::gil_scoped_release release;
-        }
+        pybind11::gil_scoped_release release;
 
         release_callback();
       });

diff --git a/src/runtime/python_runtime/export_rpc.h b/src/runtime/python_runtime/export_rpc.h
@@ -90,11 +90,6 @@ inline void ExportRpcContext(const pybind11::object& m) {
       .def("ToString", &ContextRef::ToString);
 }
 
-inline const pybind11::bytes& GetRpcEmptyPyBytes() {
-  static const pybind11::bytes kRpcEmptyPyBytes = pybind11::bytes("");
-  return kRpcEmptyPyBytes;
-}
-
 inline void PyRpcServiceBaseRegisterServiceFunc(
     aimrt::rpc::ServiceBase& service,
     std::string_view func_name,
@@ -116,29 +111,17 @@ inline void PyRpcServiceBaseRegisterServiceFunc(
           const std::string& req_buf = *static_cast<const std::string*>(req_ptr);
           std::string& rsp_buf = *static_cast<std::string*>(rsp_ptr);
 
-          // TODO，未知原因，在此处使用空字符串构造pybind11::bytes时会挂掉。但是在外面构造没有问题
-          if (req_buf.empty()) [[unlikely]] {
-            auto [status, rsp_buf_tmp] = service_func(ctx_ref, GetRpcEmptyPyBytes());
-
-            rsp_buf = std::move(rsp_buf_tmp);
-
-            callback_f(status.Code());
-            return;
-          } else {
-            auto req_buf_bytes = pybind11::bytes(req_buf);
+          pybind11::gil_scoped_acquire acquire;
 
-            auto [status, rsp_buf_tmp] = service_func(ctx_ref, req_buf_bytes);
+          auto req_buf_bytes = pybind11::bytes(req_buf);
+          auto [status, rsp_buf_tmp] = service_func(ctx_ref, req_buf_bytes);
+          req_buf_bytes.release();
 
-            pybind11::gil_scoped_acquire acquire;
-            req_buf_bytes.release();
-            pybind11::gil_scoped_release release;
-
-            rsp_buf = std::move(rsp_buf_tmp);
-
-            callback_f(status.Code());
-            return;
-          }
+          pybind11::gil_scoped_release release;
 
+          rsp_buf = std::move(rsp_buf_tmp);
+          callback_f(status.Code());
+          return;
         } catch (const std::exception& e) {
           callback_f(AIMRT_RPC_STATUS_SVR_HANDLE_FAILED);
           return;