Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PythonAPI: Fix segfaults due to incorrect GIL locking #6718

Merged
merged 4 commits into from
Oct 27, 2023

Conversation

wentasah
Copy link
Contributor

@wentasah wentasah commented Aug 16, 2023

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::to_python_value<boost::python::list const&>, boost::python::list (*)(carla::client::Client const&), boost::python::arg_from_python<carla::client::Client const&> > (rc=..., f=<optimized out>, ac0=...)
    at include/boost/python/detail/invoke.hpp:73
#7  boost::python::detail::caller_arity<1u>::impl<boost::python::list (*)(carla::client::Client const&), boost::python::default_call_policies, boost::mpl::vector2<boost::python::list, carla::client::Client const&> >::operator() (args_=<optimized out>, this=<optimized out>)
    at include/boost/python/detail/caller.hpp:233
#8  boost::python::objects::caller_py_function_impl<boost::python::detail::caller<boost::python::list (*)(carla::client::Client const&), boost::python::default_call_policies, boost::mpl::vector2<boost::python::list, carla::client::Client const&> > >::operator() (
    this=<optimized out>, args=<optimized out>, kw=<optimized out>) 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<boost::python::objects::(anonymous namespace)::bind_return, void>::invoke(boost::detail::function::function_buffer&) ()
   from /nix/store/c95f3nrkz3sflvycihyw1c8q4nk47p4m-boost-1.79.0/lib/libboost_python310.so.1.79.0
...

Description

Fixes #

Where has this been tested?

  • Platform(s): Linux
  • Python version(s): 3.10
  • Unreal Engine version(s): default in 0.9.14

Possible Drawbacks


This change is Reviewable

@wentasah wentasah requested a review from a team as a code owner August 16, 2023 20:13
@update-docs
Copy link

update-docs bot commented Aug 16, 2023

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would update our CHANGELOG.md based on your changes.

wentasah added a commit to CTU-IIG/carla-simulator.nix that referenced this pull request Aug 16, 2023
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
    matejm42#1  get_list_state () at Objects/listobject.c:26
    carla-simulator#2  PyList_New (size=0) at Objects/listobject.c:159
    carla-simulator#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
    carla-simulator#4  0x00007fe6ef9ecfc4 in boost::python::list::list (this=0x7ffd8a8aae28) at include/boost/python/list.hpp:61
    carla-simulator#5  GetAvailableMaps (self=...) at source/libcarla/Client.cpp:26
    carla-simulator#6  0x00007fe6efb6a8fe in boost::python::detail::invoke<boost::python::to_python_value<boost::python::list const&>, boost::python::list (*)(carla::client::Client const&), boost::python::arg_from_python<carla::client::Client const&> > (rc=..., f=<optimized out>, ac0=...)
        at include/boost/python/detail/invoke.hpp:73
    carla-simulator#7  boost::python::detail::caller_arity<1u>::impl<boost::python::list (*)(carla::client::Client const&), boost::python::default_call_policies, boost::mpl::vector2<boost::python::list, carla::client::Client const&> >::operator() (args_=<optimized out>, this=<optimized out>)
        at include/boost/python/detail/caller.hpp:233
    carla-simulator#8  boost::python::objects::caller_py_function_impl<boost::python::detail::caller<boost::python::list (*)(carla::client::Client const&), boost::python::default_call_policies, boost::mpl::vector2<boost::python::list, carla::client::Client const&> > >::operator() (
        this=<optimized out>, args=<optimized out>, kw=<optimized out>) at include/boost/python/object/py_function.hpp:38
    carla-simulator#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
    carla-simulator#10 0x00007fe6fdc1b6a8 in boost::detail::function::void_function_ref_invoker0<boost::python::objects::(anonymous namespace)::bind_return, void>::invoke(boost::detail::function::function_buffer&) ()
       from /nix/store/c95f3nrkz3sflvycihyw1c8q4nk47p4m-boost-1.79.0/lib/libboost_python310.so.1.79.0
    ...
@wentasah wentasah changed the base branch from master to dev September 3, 2023 14:15
@wentasah
Copy link
Contributor Author

wentasah commented Sep 3, 2023

Added a changelog entry and rebased to dev.

javiergrCS
javiergrCS previously approved these changes Sep 21, 2023
Copy link
Contributor

@javiergrCS javiergrCS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code changes proposed by the collaborator wentasah have been successfully tested, verifying that they fix the segfault affecting the GetAvailableMaps function when using Python 3.10. The changes are ready to be merged and integrated into the 'dev' branch.

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
    matejm42#1  0x00007f04552adaa1 in PyGILState_Ensure () from /nix/store/zqj9irpw63pal9r4671p1gjd9jiw5sid-ros-env/lib/libpython3.10.so.1.0
    carla-simulator#2  0x00007f040afd4f32 in std::_Function_handler<void (carla::client::WorldSnapshot), MakeCallback(boost::python::api::object)::{lambda(auto:1)matejm42#1}>::_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
    carla-simulator#3  0x00007f040b1d4ab1 in carla::client::detail::CallbackList<carla::client::WorldSnapshot>::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
    carla-simulator#4  0x00007f040b1d424a in std::_Function_handler<void (carla::Buffer), carla::client::detail::Episode::Listen()::{lambda(auto:1)matejm42#1}>::_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
    carla-simulator#5  0x00007f040b23fc41 in boost::asio::detail::completion_handler<boost::asio::detail::binder0<carla::streaming::detail::tcp::Client::ReadData()::{lambda()matejm42#1}::operator()() const::{lambda(boost::system::error_code, unsigned long)matejm42#1}::operator()(boost::system::error_code, unsigned long) const::{lambda()matejm42#1}>, boost::asio::io_context::basic_executor_type<std::allocator<void>, 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
    carla-simulator#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
    carla-simulator#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
    carla-simulator#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
    carla-simulator#9  0x00007f040b1ac1cb in std::thread::_State_impl<std::thread::_Invoker<std::tuple<carla::ThreadPool::AsyncRun(unsigned long)::{lambda()matejm42#1}> > >::_M_run() ()
       from /nix/store/zqj9irpw63pal9r4671p1gjd9jiw5sid-ros-env/lib/python3.10/site-packages/carla/libcarla.cpython-310-x86_64-linux-gnu.so
    carla-simulator#10 0x00007f040bce05c3 in execute_native_thread_routine () from /nix/store/2fpmbk0g0ggm9zq89af7phvvvv8dnm7n-gcc-12.3.0-lib/lib/libstdc++.so.6
    carla-simulator#11 0x00007f045509fdd4 in start_thread () from /nix/store/1x4ijm9r1a88qk7zcmbbfza324gx1aac-glibc-2.37-8/lib/libc.so.6
    carla-simulator#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.
@wentasah
Copy link
Contributor Author

Thanks for the review @javiergrCS.

In the mean time, I've found a similar segfault in get_random_location_from_navigation(). I added a commit fixing this segfault too.

@wentasah wentasah changed the title PythonAPI: Fix segfault in GetAvailableMaps PythonAPI: Fix segfaults due to incorrect GIL locking Sep 25, 2023
Resolve conflict in CHANGELOG.md and update the entry text.
Copy link
Contributor

@javiergrCS javiergrCS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes proposed by the collaborator wentasah for the function get_random_location_from_navigation() have been successfully tested, and it has been verified that these changes fix the issue with that function. The changes are ready to be merged into the 'dev' branch.

Thank you for your collaboration, wentasah.

Copy link
Contributor

@bernatx bernatx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r2, 1 of 2 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Harin22)

Copy link
Contributor

@bernatx bernatx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Harin22)

@bernatx bernatx merged commit 2de8ad0 into carla-simulator:dev Oct 27, 2023
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants