Delete timer on removal.

[simgrid.git] / src / simix / ActorImpl.cpp

diff --git a/src/simix/ActorImpl.cpp b/src/simix/ActorImpl.cpp
index 610678f..a55a783 100644 (file)
--- a/src/simix/ActorImpl.cpp
+++ b/src/simix/ActorImpl.cpp
@@ -73,8 +73,10 @@ void SIMIX_process_cleanup(smx_actor_t process)
   SIMIX_process_on_exit_runall(process);
 
   /* Unregister from the kill timer if any */
-  if (process->kill_timer != nullptr)
+  if (process->kill_timer != nullptr) {
     SIMIX_timer_remove(process->kill_timer);
+    process->kill_timer = nullptr;
+  }
 
   xbt_os_mutex_acquire(simix_global->mutex);
 
@@ -185,7 +187,7 @@ simgrid::s4u::Actor* ActorImpl::restart()
 
   // start the new process
   ActorImpl* actor = simix_global->create_process_function(arg.name.c_str(), std::move(arg.code), arg.data, arg.host,
-                                                           arg.properties, nullptr);
+                                                           arg.properties.get(), nullptr);
   if (arg.kill_time >= 0)
     simcall_process_set_kill_time(actor, arg.kill_time);
   if (arg.auto_restart)
@@ -330,7 +332,7 @@ smx_actor_t SIMIX_process_create(const char* name, std::function<void()> code, v
 
   /* Add properties */
   if (properties != nullptr)
-    for (auto kv : *properties)
+    for (auto const& kv : *properties)
       process->setProperty(kv.first, kv.second);
 
   /* Make sure that the process is initialized for simix, in case we are called from the Host::onCreation signal */
@@ -400,7 +402,7 @@ smx_actor_t SIMIX_process_attach(const char* name, void* data, const char* hostn
 
   /* Add properties */
   if (properties != nullptr)
-    for (auto kv : *properties)
+    for (auto const& kv : *properties)
       process->setProperty(kv.first, kv.second);
 
   /* Add the process to it's host process list */
@@ -428,16 +430,10 @@ void SIMIX_process_detach()
   if (not context)
     xbt_die("Not a suitable context");
 
-  simix_global->cleanup_process_function(context->process());
-
-  // Let maestro ignore we are still alive:
-  // xbt_swag_remove(context->process(), simix_global->process_list);
-
-  // TODO, Remove from proces list:
-  //   xbt_swag_remove(process, sg_host_simix(host)->process_list);
-
+  auto process = context->process();
+  simix_global->cleanup_process_function(process);
   context->attach_stop();
-  // delete context;
+  delete process;
 }
 
 /**
@@ -545,9 +541,8 @@ void SIMIX_process_throw(smx_actor_t process, xbt_errcat_t cat, int value, const
 
     simgrid::kernel::activity::ExecImplPtr exec =
         boost::dynamic_pointer_cast<simgrid::kernel::activity::ExecImpl>(process->waiting_synchro);
-    if (exec != nullptr) {
-      SIMIX_execution_cancel(process->waiting_synchro);
-    }
+    if (exec != nullptr && exec->surf_exec)
+      exec->surf_exec->cancel();
 
     simgrid::kernel::activity::CommImplPtr comm =
         boost::dynamic_pointer_cast<simgrid::kernel::activity::CommImpl>(process->waiting_synchro);
@@ -593,7 +588,7 @@ void simcall_HANDLER_process_killall(smx_simcall_t simcall, int reset_pid) {
  */
 void SIMIX_process_killall(smx_actor_t issuer, int reset_pid)
 {
-  for (auto kv : simix_global->process_list)
+  for (auto const& kv : simix_global->process_list)
     if (kv.second != issuer)
       SIMIX_process_kill(kv.second, issuer);
 
@@ -665,7 +660,7 @@ const char* SIMIX_process_self_get_name() {
 
 smx_actor_t SIMIX_process_get_by_name(const char* name)
 {
-  for (auto kv : simix_global->process_list)
+  for (auto const& kv : simix_global->process_list)
     if (kv.second->name == name)
       return kv.second;
   return nullptr;
@@ -684,10 +679,9 @@ void simcall_HANDLER_process_join(smx_simcall_t simcall, smx_actor_t process, do
   simcall->issuer->waiting_synchro = sync;
 }
 
-static int SIMIX_process_join_finish(smx_process_exit_status_t status, void* synchro)
+static int SIMIX_process_join_finish(smx_process_exit_status_t status, smx_actor_t process, smx_activity_t sleep_act)
 {
-  simgrid::kernel::activity::SleepImpl* sleep = static_cast<simgrid::kernel::activity::SleepImpl*>(synchro);
-
+  simgrid::kernel::activity::SleepImpl* sleep = static_cast<simgrid::kernel::activity::SleepImpl*>(sleep_act.get());
   if (sleep->surf_sleep) {
     sleep->surf_sleep->cancel();
 
@@ -707,7 +701,8 @@ static int SIMIX_process_join_finish(smx_process_exit_status_t status, void* syn
     sleep->surf_sleep->unref();
     sleep->surf_sleep = nullptr;
   }
-  // intrusive_ptr_release(process); // FIXME: We are leaking here. See comment in SIMIX_process_join()
+  intrusive_ptr_release(process);
+  intrusive_ptr_release(sleep_act.get());
   return 0;
 }
 
@@ -715,18 +710,16 @@ smx_activity_t SIMIX_process_join(smx_actor_t issuer, smx_actor_t process, doubl
 {
   smx_activity_t res = issuer->sleep(timeout);
   intrusive_ptr_add_ref(res.get());
-  /* We are leaking the process here, but if we don't take the ref, we get a "use after free".
-   * The correct solution would be to derivate the type SynchroSleep into a SynchroProcessJoin,
-   * but the code is not clean enough for now for this.
-   * The C API should first be properly replaced with the C++ one, which is a fair amount of work.
-   */
   intrusive_ptr_add_ref(process);
   SIMIX_process_on_exit(process,
                         [](void*, void* arg) {
-                          return simgrid::simix::kernelImmediate(
-                              [&] { return SIMIX_process_join_finish(SMX_EXIT_SUCCESS, arg); });
+                          auto argp = static_cast<std::pair<smx_actor_t, smx_activity_t>*>(arg);
+                          int res   = simgrid::simix::kernelImmediate(
+                              [&] { return SIMIX_process_join_finish(SMX_EXIT_SUCCESS, argp->first, argp->second); });
+                          delete argp;
+                          return res;
                         },
-                        &*res);
+                        new std::pair<smx_actor_t, smx_activity_t>(process, res));
   return res;
 }