Fix issues related to the destruction of objects

LibCarla/source/carla/PythonUtil.h

@@ -71,8 +71,25 @@ namespace carla {
 
 #endif // LIBCARLA_WITH_PYTHON_SUPPORT
 
-    /// A deleter that can be passed to a shared_ptr to release the GIL before
-    /// destroying the object.
+    /// A deleter that can be passed to a smart pointer to acquire the GIL
+    /// before destroying the object.
+    class AcquireGILDeleter {
+    public:
+
+      template <typename T>
+      void operator()(T *ptr) const {
+#ifdef LIBCARLA_WITH_PYTHON_SUPPORT
+        if (ptr != nullptr && !PythonUtil::ThisThreadHasTheGIL()) {
+          AcquireGIL lock;
+          delete ptr;
+        } else
+#endif // LIBCARLA_WITH_PYTHON_SUPPORT
+        delete ptr;
+      }
+    };
+
+    /// A deleter that can be passed to a smart pointer to release the GIL
+    /// before destroying the object.
     class ReleaseGILDeleter {
     public:
 

LibCarla/source/carla/ThreadGroup.h

@@ -37,6 +37,7 @@ namespace carla {
 
     void JoinAll() {
       for (auto &thread : _threads) {
+        DEBUG_ASSERT_NE(thread.get_id(), std::this_thread::get_id());
         if (thread.joinable()) {
           thread.join();
         }

LibCarla/source/carla/client/detail/ActorState.h

@@ -49,6 +49,8 @@ namespace detail {
 
   private:
 
+    friend class detail::Client;
+
     ActorState(rpc::Actor description, Episode episode)
       : _description(std::move(description)),
         _episode(std::move(episode)) {}

LibCarla/source/carla/client/detail/Client.cpp

@@ -120,7 +120,10 @@ namespace detail {
   }
 
   bool Client::DestroyActor(Actor &actor) {
-    return _pimpl->CallAndWait<bool>("destroy_actor", actor.Serialize());
+    auto result = _pimpl->CallAndWait<bool>("destroy_actor", actor.Serialize());
+    // Remove it's persistent state so it cannot access the client anymore.
+    actor.GetEpisode().ClearState();
+    return result;
   }
 
   void Client::SubscribeToStream(

LibCarla/source/carla/client/detail/Episode.cpp

@@ -8,18 +8,29 @@
 
 #include "carla/client/detail/Client.h"
 
+#include <boost/atomic.hpp>
+
 #include <exception>
 
 namespace carla {
 namespace client {
 namespace detail {
 
-  Episode::Episode(SharedPtr<PersistentState> state)
+  EpisodeImpl::EpisodeImpl(SharedPtr<PersistentState> state)
     : _state(std::move(state)),
       _episode_id(_state->GetCurrentEpisodeId()) {}
 
-  PersistentState &Episode::GetPersistentStateWithChecks() const {
-    DEBUG_ASSERT(_state != nullptr);
+  void EpisodeImpl::ClearState() {
+    boost::atomic_store_explicit(&_state, {nullptr}, boost::memory_order_relaxed);
+  }
+
+  PersistentState &EpisodeImpl::GetPersistentStateWithChecks() const {
+    auto state = boost::atomic_load_explicit(&_state, boost::memory_order_relaxed);
+    if (state == nullptr) {
+      throw std::runtime_error(
+          "trying to operate on a destroyed actor; an actor's function "
+          "was called, but the actor is already destroyed.");
+    }
     if (_episode_id != _state->GetCurrentEpisodeId()) {
       throw std::runtime_error(
           "trying to access an expired episode; a new episode was started "

LibCarla/source/carla/client/detail/Episode.h

@@ -14,7 +14,7 @@ namespace carla {
 namespace client {
 namespace detail {
 
-  class Episode {
+  class EpisodeImpl {
   public:
 
     PersistentState &operator*() const {
@@ -25,12 +25,14 @@ namespace detail {
       return &GetPersistentStateWithChecks();
     }
 
+  protected:
+
+    EpisodeImpl(SharedPtr<PersistentState> state);
+
+    void ClearState();
+
   private:
 
-    friend PersistentState;
-
-    Episode(SharedPtr<PersistentState> state);
-
     PersistentState &GetPersistentStateWithChecks() const;
 
     SharedPtr<PersistentState> _state;
@@ -38,6 +40,19 @@ namespace detail {
     size_t _episode_id;
   };
 
+  class Episode : private EpisodeImpl {
+  public:
+
+    using EpisodeImpl::operator*;
+    using EpisodeImpl::operator->;
+
+  private:
+
+    friend PersistentState;
+
+    using EpisodeImpl::EpisodeImpl;
+  };
+
 } // namespace detail
 } // namespace client
 } // namespace carla

PythonAPI/source/libcarla/Sensor.cpp

@@ -8,19 +8,24 @@
 #include <carla/client/Sensor.h>
 
 static void SubscribeToStream(carla::client::Sensor &self, boost::python::object callback) {
+  namespace py = boost::python;
   // Make sure the callback is actually callable.
   if (!PyCallable_Check(callback.ptr())) {
     PyErr_SetString(PyExc_TypeError, "callback argument must be callable!");
-    boost::python::throw_error_already_set();
+    py::throw_error_already_set();
     return;
   }
 
+  // We need to delete the callback while holding the GIL.
+  using Deleter = carla::PythonUtil::AcquireGILDeleter;
+  auto callback_ptr = carla::SharedPtr<py::object>{new py::object(callback), Deleter()};
+
   // Subscribe to the sensor.
-  self.Listen([callback](auto message) {
+  self.Listen([callback=std::move(callback_ptr)](auto message) {
     carla::PythonUtil::AcquireGIL lock;
     try {
-      boost::python::call<void>(callback.ptr(), boost::python::object(message));
-    } catch (const boost::python::error_already_set &e) {
+      py::call<void>(callback->ptr(), py::object(message));
+    } catch (const py::error_already_set &e) {
       PyErr_Print();
     }
   });