From 2de8ad017d7fd7222957a6ef7e9f328f16c7bed5 Mon Sep 17 00:00:00 2001 From: Michal Sojka Date: Fri, 27 Oct 2023 14:37:46 +0200 Subject: [PATCH] PythonAPI: Fix segfaults due to incorrect GIL locking (#6718) * PythonAPI: Fix segfault in GetAvailableMaps When using CARLA with Python 3.10, I'm getting a segfault in GetAvailableMaps. The problem disappears when PyList manipulation does not happen with GIL unlocked, as done in this commit. The initial part of crash backtrace (from GDB) is below: Program terminated with signal SIGSEGV, Segmentation fault. warning: Section `.reg-xstate/49253' in core file too small. #0 _PyInterpreterState_GET () at ./Include/internal/pycore_pystate.h:117 117 return tstate->interp; [Current thread is 1 (Thread 0x7fe6fe48f740 (LWP 49253))] (gdb) bt #0 _PyInterpreterState_GET () at ./Include/internal/pycore_pystate.h:117 #1 get_list_state () at Objects/listobject.c:26 #2 PyList_New (size=0) at Objects/listobject.c:159 #3 0x00007fe6fdc0dab0 in boost::python::detail::list_base::list_base() () from /nix/store/c95f3nrkz3sflvycihyw1c8q4nk47p4m-boost-1.79.0/lib/libboost_python310.so.1.79.0 #4 0x00007fe6ef9ecfc4 in boost::python::list::list (this=0x7ffd8a8aae28) at include/boost/python/list.hpp:61 #5 GetAvailableMaps (self=...) at source/libcarla/Client.cpp:26 #6 0x00007fe6efb6a8fe in boost::python::detail::invoke, boost::python::list (*)(carla::client::Client const&), boost::python::arg_from_python > (rc=..., f=, ac0=...) at include/boost/python/detail/invoke.hpp:73 #7 boost::python::detail::caller_arity<1u>::impl >::operator() (args_=, this=) at include/boost/python/detail/caller.hpp:233 #8 boost::python::objects::caller_py_function_impl > >::operator() ( this=, args=, kw=) at include/boost/python/object/py_function.hpp:38 #9 0x00007fe6fdc1b4dd in boost::python::objects::function::call(_object*, _object*) const () from /nix/store/c95f3nrkz3sflvycihyw1c8q4nk47p4m-boost-1.79.0/lib/libboost_python310.so.1.79.0 #10 0x00007fe6fdc1b6a8 in boost::detail::function::void_function_ref_invoker0::invoke(boost::detail::function::function_buffer&) () from /nix/store/c95f3nrkz3sflvycihyw1c8q4nk47p4m-boost-1.79.0/lib/libboost_python310.so.1.79.0 ... * PythonAPI: Fix segfault in get_random_location_from_navigation() When I run generate_traffic.py under Python 3.10, I get a segfault at line: loc = world.get_random_location_from_navigation() The backtrace from gdb looks like this: #0 0x00007f04552ad7e7 in new_threadstate () from /nix/store/zqj9irpw63pal9r4671p1gjd9jiw5sid-ros-env/lib/libpython3.10.so.1.0 #1 0x00007f04552adaa1 in PyGILState_Ensure () from /nix/store/zqj9irpw63pal9r4671p1gjd9jiw5sid-ros-env/lib/libpython3.10.so.1.0 #2 0x00007f040afd4f32 in std::_Function_handler::_M_invoke(std::_Any_data const&, carla::client::WorldSnapshot&&) () from /nix/store/zqj9irpw63pal9r4671p1gjd9jiw5sid-ros-env/lib/python3.10/site-packages/carla/libcarla.cpython-310-x86_64-linux-gnu.so #3 0x00007f040b1d4ab1 in carla::client::detail::CallbackList::Call(carla::client::WorldSnapshot) const () from /nix/store/zqj9irpw63pal9r4671p1gjd9jiw5sid-ros-env/lib/python3.10/site-packages/carla/libcarla.cpython-310-x86_64-linux-gnu.so #4 0x00007f040b1d424a in std::_Function_handler::_M_invoke(std::_Any_data const&, carla::Buffer&&) () from /nix/store/zqj9irpw63pal9r4671p1gjd9jiw5sid-ros-env/lib/python3.10/site-packages/carla/libcarla.cpython-310-x86_64-linux-gnu.so #5 0x00007f040b23fc41 in boost::asio::detail::completion_handler, boost::asio::io_context::basic_executor_type, 0ul> >::do_complete(void*, boost::asio::detail::scheduler_operation*, boost::system::error_code const&, unsigned long) () from /nix/store/zqj9irpw63pal9r4671p1gjd9jiw5sid-ros-env/lib/python3.10/site-packages/carla/libcarla.cpython-310-x86_64-linux-gnu.so #6 0x00007f040b24ae85 in boost::asio::detail::strand_service::do_complete(void*, boost::asio::detail::scheduler_operation*, boost::system::error_code const&, unsigned long) () from /nix/store/zqj9irpw63pal9r4671p1gjd9jiw5sid-ros-env/lib/python3.10/site-packages/carla/libcarla.cpython-310-x86_64-linux-gnu.so #7 0x00007f040b1a94f5 in boost::asio::detail::scheduler::do_run_one(boost::asio::detail::conditionally_enabled_mutex::scoped_lock&, boost::asio::detail::scheduler_thread_info&, boost::system::error_code const&) () from /nix/store/zqj9irpw63pal9r4671p1gjd9jiw5sid-ros-env/lib/python3.10/site-packages/carla/libcarla.cpython-310-x86_64-linux-gnu.so #8 0x00007f040b199351 in boost::asio::detail::scheduler::run(boost::system::error_code&) [clone .isra.0] () from /nix/store/zqj9irpw63pal9r4671p1gjd9jiw5sid-ros-env/lib/python3.10/site-packages/carla/libcarla.cpython-310-x86_64-linux-gnu.so #9 0x00007f040b1ac1cb in std::thread::_State_impl > >::_M_run() () from /nix/store/zqj9irpw63pal9r4671p1gjd9jiw5sid-ros-env/lib/python3.10/site-packages/carla/libcarla.cpython-310-x86_64-linux-gnu.so #10 0x00007f040bce05c3 in execute_native_thread_routine () from /nix/store/2fpmbk0g0ggm9zq89af7phvvvv8dnm7n-gcc-12.3.0-lib/lib/libstdc++.so.6 #11 0x00007f045509fdd4 in start_thread () from /nix/store/1x4ijm9r1a88qk7zcmbbfza324gx1aac-glibc-2.37-8/lib/libc.so.6 #12 0x00007f04551219b0 in clone3 () from /nix/store/1x4ijm9r1a88qk7zcmbbfza324gx1aac-glibc-2.37-8/lib/libc.so.6 It turns out that its caused by releasing GIL for too long. We fix it by releasing the GIL only for the actual libcarla call and constructing Python objects with GIL locked. --------- Co-authored-by: bernat --- CHANGELOG.md | 1 + PythonAPI/carla/source/libcarla/Client.cpp | 8 ++++++-- PythonAPI/carla/source/libcarla/libcarla.cpp | 4 ++-- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 76037cde4..6f45b4b90 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ * Add keyword arguments for `carla.TrafficManager` Python API functions * Fixed bug causing the `FPixelReader::SavePixelsToDisk(PixelData, FilePath)` function to crash due to pixel array not set correctly. * Collisions detected by the CollisionSensor no longer generate more than one event per frame. + * Fixed segfaults in Python API due to incorrect GIL locking under Python 3.10. * Added API function to load a map only if it is different from the current one. ## CARLA 0.9.14 diff --git a/PythonAPI/carla/source/libcarla/Client.cpp b/PythonAPI/carla/source/libcarla/Client.cpp index f773b8a5f..34af2106a 100644 --- a/PythonAPI/carla/source/libcarla/Client.cpp +++ b/PythonAPI/carla/source/libcarla/Client.cpp @@ -22,9 +22,13 @@ static void SetTimeout(carla::client::Client &client, double seconds) { } static auto GetAvailableMaps(const carla::client::Client &self) { - carla::PythonUtil::ReleaseGIL unlock; boost::python::list result; - for (const auto &str : self.GetAvailableMaps()) { + std::vector maps; + { + carla::PythonUtil::ReleaseGIL unlock; + maps = self.GetAvailableMaps(); + } + for (const auto &str : maps) { result.append(str); } return result; diff --git a/PythonAPI/carla/source/libcarla/libcarla.cpp b/PythonAPI/carla/source/libcarla/libcarla.cpp index f56ea5c99..73998c2eb 100644 --- a/PythonAPI/carla/source/libcarla/libcarla.cpp +++ b/PythonAPI/carla/source/libcarla/libcarla.cpp @@ -139,8 +139,8 @@ std::vector PythonLitstToVector(boost::python::list &input) { } #define CALL_RETURNING_OPTIONAL_WITHOUT_GIL(cls, fn) +[](const cls &self) { \ - carla::PythonUtil::ReleaseGIL unlock; \ - auto optional = self.fn(); \ + auto call = CONST_CALL_WITHOUT_GIL(cls, fn); \ + auto optional = call(self); \ return optional.has_value() ? boost::python::object(*optional) : boost::python::object(); \ }