eradicate all remaining manual refcounting on ActivityImpl

diff --git a/src/kernel/activity/ActivityImpl.cpp b/src/kernel/activity/ActivityImpl.cpp
index ea655de..6a9b6e8 100644 (file)
--- a/src/kernel/activity/ActivityImpl.cpp
+++ b/src/kernel/activity/ActivityImpl.cpp
@@ -5,7 +5,7 @@
 
 #include "src/kernel/activity/ActivityImpl.hpp"
 
-XBT_LOG_EXTERNAL_CATEGORY(simix_network);
+XBT_LOG_EXTERNAL_CATEGORY(simix_process);
 
 namespace simgrid {
 namespace kernel {
@@ -14,35 +14,25 @@ namespace activity {
 ActivityImpl::ActivityImpl()  = default;
 ActivityImpl::~ActivityImpl() = default;
 
-void ActivityImpl::ref()
-{
-  xbt_assert(refcount_ >= 0);
-  refcount_++;
-  XBT_CDEBUG(simix_network, "%p->refcount++ ~> %d", this, (int)refcount_);
-  if (XBT_LOG_ISENABLED(simix_network, xbt_log_priority_trace))
-    xbt_backtrace_display_current();
-}
-
-void ActivityImpl::unref()
-{
-  XBT_CDEBUG(simix_network, "%p->refcount-- ~> %d", this, ((int)refcount_) - 1);
-  xbt_assert(refcount_ >= 0);
-  refcount_--;
-  if (XBT_LOG_ISENABLED(simix_network, xbt_log_priority_trace))
-    xbt_backtrace_display_current();
-  if (refcount_ <= 0)
-    delete this;
-}
-
 // boost::intrusive_ptr<Activity> support:
 void intrusive_ptr_add_ref(simgrid::kernel::activity::ActivityImpl* activity)
 {
-  activity->ref();
+  xbt_assert(activity->refcount_ >= 0);
+  activity->refcount_++;
+  XBT_CDEBUG(simix_process, "%p->refcount++ ~> %d", activity, (int)activity->refcount_);
+  if (XBT_LOG_ISENABLED(simix_process, xbt_log_priority_trace))
+    xbt_backtrace_display_current();
 }
 
 void intrusive_ptr_release(simgrid::kernel::activity::ActivityImpl* activity)
 {
-  activity->unref();
+  XBT_CDEBUG(simix_process, "%p->refcount-- ~> %d", activity, ((int)activity->refcount_) - 1);
+  xbt_assert(activity->refcount_ >= 0);
+  activity->refcount_--;
+  if (XBT_LOG_ISENABLED(simix_process, xbt_log_priority_trace))
+    xbt_backtrace_display_current();
+  if (activity->refcount_ <= 0)
+    delete activity;
 }
 }
 }

diff --git a/src/kernel/activity/ActivityImpl.hpp b/src/kernel/activity/ActivityImpl.hpp
index be03036..2a50de8 100644 (file)
--- a/src/kernel/activity/ActivityImpl.hpp
+++ b/src/kernel/activity/ActivityImpl.hpp
@@ -31,12 +31,7 @@ namespace activity {
     virtual void resume()=0;
     virtual void post() =0; // What to do when a simcall terminates
 
-    /** @brief Increases the refcount */
-    void ref();
-    /** @brief Reduces the refcount */
-    void unref();
-
-     // boost::intrusive_ptr<Activity> support:
+    // boost::intrusive_ptr<ActivityImpl> support:
     friend void intrusive_ptr_add_ref(ActivityImpl * activity);
     friend void intrusive_ptr_release(ActivityImpl * activity);
 

diff --git a/src/kernel/activity/CommImpl.cpp b/src/kernel/activity/CommImpl.cpp
index 1b61834..33d31e7 100644 (file)
--- a/src/kernel/activity/CommImpl.cpp
+++ b/src/kernel/activity/CommImpl.cpp
@@ -61,7 +61,6 @@ void simgrid::kernel::activity::CommImpl::cancel()
   if (state == SIMIX_WAITING) {
     mbox->remove(this);
     state = SIMIX_CANCELED;
-    this->unref();
   } else if (not MC_is_active() /* when running the MC there are no surf actions */
              && not MC_record_replay_is_active() && (state == SIMIX_READY || state == SIMIX_RUNNING)) {
 

diff --git a/src/kernel/activity/ExecImpl.cpp b/src/kernel/activity/ExecImpl.cpp
index 56c6436..aee76bc 100644 (file)
--- a/src/kernel/activity/ExecImpl.cpp
+++ b/src/kernel/activity/ExecImpl.cpp
@@ -16,6 +16,7 @@ simgrid::kernel::activity::ExecImpl::ExecImpl(const char* name, sg_host_t host)
   if (name)
     this->name = name;
   this->state  = SIMIX_RUNNING;
+  XBT_DEBUG("Create exec %p", this);
 }
 
 simgrid::kernel::activity::ExecImpl::~ExecImpl()
@@ -24,6 +25,7 @@ simgrid::kernel::activity::ExecImpl::~ExecImpl()
     surf_exec->unref();
   if (timeoutDetector)
     timeoutDetector->unref();
+  XBT_DEBUG("Destroy exec %p", this);
 }
 void simgrid::kernel::activity::ExecImpl::suspend()
 {

diff --git a/src/msg/msg_gos.cpp b/src/msg/msg_gos.cpp
index f3880e3..4ec5694 100644 (file)
--- a/src/msg/msg_gos.cpp
+++ b/src/msg/msg_gos.cpp
@@ -71,7 +71,7 @@ msg_error_t MSG_parallel_task_execute_with_timeout(msg_task_t task, double timeo
           boost::static_pointer_cast<simgrid::kernel::activity::ExecImpl>(simcall_execution_parallel_start(
               task->name, simdata->host_nb, simdata->host_list, simdata->flops_parallel_amount,
               simdata->bytes_parallel_amount, 1.0, -1.0, timeout));
-      XBT_DEBUG("Parallel execution action created: %p", simdata->compute);
+      XBT_DEBUG("Parallel execution action created: %p", simdata->compute.get());
     } else {
       simdata->compute = boost::static_pointer_cast<simgrid::kernel::activity::ExecImpl>(
           simcall_execution_start(task->name, simdata->flops_amount, simdata->priority, simdata->bound));

diff --git a/src/msg/msg_private.h b/src/msg/msg_private.h
index 00164db..2a45111 100644 (file)
--- a/src/msg/msg_private.h
+++ b/src/msg/msg_private.h
@@ -30,9 +30,6 @@ public:
 typedef struct simdata_task {
   ~simdata_task()
   {
-    if (this->compute)
-      this->compute->unref();
-
     /* parallel tasks only */
     xbt_free(this->host_list);
   }

diff --git a/src/simix/ActorImpl.cpp b/src/simix/ActorImpl.cpp
index ea43de9..0d207a8 100644 (file)
--- a/src/simix/ActorImpl.cpp
+++ b/src/simix/ActorImpl.cpp
@@ -106,11 +106,6 @@ void SIMIX_process_cleanup(smx_actor_t process)
           comm, comm->detached, (int)comm->state, comm->src_proc, comm->dst_proc);
       comm->src_proc = nullptr;
 
-      /* I'm not supposed to destroy a detached comm from the sender side, */
-      if (comm->detached)
-        XBT_DEBUG("Don't destroy it since it's a detached comm and I'm the sender");
-      else
-        comm->unref();
     } else if (comm->dst_proc == process) {
       XBT_DEBUG("Found an unfinished recv comm %p, state %d, src = %p, dst = %p",
           comm, (int)comm->state, comm->src_proc, comm->dst_proc);
@@ -423,7 +418,7 @@ void SIMIX_process_kill(smx_actor_t process, smx_actor_t issuer) {
   process->exception = nullptr;
 
   /* destroy the blocking synchro if any */
-  if (process->waiting_synchro) {
+  if (process->waiting_synchro != nullptr) {
 
     simgrid::kernel::activity::ExecImplPtr exec =
         boost::dynamic_pointer_cast<simgrid::kernel::activity::ExecImpl>(process->waiting_synchro);
@@ -437,7 +432,6 @@ void SIMIX_process_kill(smx_actor_t process, smx_actor_t issuer) {
         boost::dynamic_pointer_cast<simgrid::kernel::activity::IoImpl>(process->waiting_synchro);
 
     if (exec != nullptr) {
-      exec->unref();
 
     } else if (comm != nullptr) {
       process->comms.remove(process->waiting_synchro);
@@ -446,13 +440,11 @@ void SIMIX_process_kill(smx_actor_t process, smx_actor_t issuer) {
       auto i = boost::range::find(process->waiting_synchro->simcalls, &process->simcall);
       if (i != process->waiting_synchro->simcalls.end())
         process->waiting_synchro->simcalls.remove(&process->simcall);
-      comm->unref();
     } else if (sleep != nullptr) {
       SIMIX_process_sleep_destroy(process->waiting_synchro);
 
     } else if (raw != nullptr) {
       SIMIX_synchro_stop_waiting(process, &process->simcall);
-      process->waiting_synchro->unref();
 
     } else if (io != nullptr) {
       SIMIX_io_destroy(process->waiting_synchro);
@@ -723,7 +715,6 @@ static int SIMIX_process_join_finish(smx_process_exit_status_t status, void* syn
     sleep->surf_sleep->unref();
     sleep->surf_sleep = nullptr;
   }
-  sleep->unref();
   // intrusive_ptr_release(process); // FIXME: We are leaking here. See comment in SIMIX_process_join()
   return 0;
 }
@@ -731,7 +722,7 @@ static int SIMIX_process_join_finish(smx_process_exit_status_t status, void* syn
 smx_activity_t SIMIX_process_join(smx_actor_t issuer, smx_actor_t process, double timeout)
 {
   smx_activity_t res = SIMIX_process_sleep(issuer, timeout);
-  (&*res)->ref();
+  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.
@@ -773,7 +764,7 @@ smx_activity_t SIMIX_process_sleep(smx_actor_t process, double duration)
 
 void SIMIX_process_sleep_destroy(smx_activity_t synchro)
 {
-  XBT_DEBUG("Destroy synchro %p", synchro);
+  XBT_DEBUG("Destroy sleep synchro %p", synchro.get());
   simgrid::kernel::activity::SleepImplPtr sleep =
       boost::dynamic_pointer_cast<simgrid::kernel::activity::SleepImpl>(synchro);
 

diff --git a/src/simix/smx_host.cpp b/src/simix/smx_host.cpp
index 6fd7c62..5605745 100644 (file)
--- a/src/simix/smx_host.cpp
+++ b/src/simix/smx_host.cpp
@@ -161,13 +161,14 @@ smx_activity_t SIMIX_execution_start(smx_actor_t issuer, const char *name, doubl
                                     double bound){
 
   /* alloc structures and initialize */
-  simgrid::kernel::activity::ExecImpl* exec = new simgrid::kernel::activity::ExecImpl(name, issuer->host);
+  simgrid::kernel::activity::ExecImplPtr exec =
+      simgrid::kernel::activity::ExecImplPtr(new simgrid::kernel::activity::ExecImpl(name, issuer->host));
 
   /* set surf's action */
   if (not MC_is_active() && not MC_record_replay_is_active()) {
 
     exec->surf_exec = issuer->host->pimpl_cpu->execution_start(flops_amount);
-    exec->surf_exec->setData(exec);
+    exec->surf_exec->setData(exec.get());
     exec->surf_exec->setPriority(priority);
 
     if (bound > 0)
@@ -184,7 +185,8 @@ smx_activity_t SIMIX_execution_parallel_start(const char* name, int host_nb, sg_
 {
 
   /* alloc structures and initialize */
-  simgrid::kernel::activity::ExecImpl* exec = new simgrid::kernel::activity::ExecImpl(name, nullptr);
+  simgrid::kernel::activity::ExecImplPtr exec =
+      simgrid::kernel::activity::ExecImplPtr(new simgrid::kernel::activity::ExecImpl(name, nullptr));
 
   /* set surf's synchro */
   sg_host_t *host_list_cpy = xbt_new0(sg_host_t, host_nb);
@@ -201,10 +203,10 @@ smx_activity_t SIMIX_execution_parallel_start(const char* name, int host_nb, sg_
   /* set surf's synchro */
   if (not MC_is_active() && not MC_record_replay_is_active()) {
     exec->surf_exec = surf_host_model->executeParallelTask(host_nb, host_list_cpy, flops_amount, bytes_amount, rate);
-    exec->surf_exec->setData(exec);
+    exec->surf_exec->setData(exec.get());
     if (timeout > 0) {
       exec->timeoutDetector = host_list[0]->pimpl_cpu->sleep(timeout);
-      exec->timeoutDetector->setData(exec);
+      exec->timeoutDetector->setData(exec.get());
     }
   }
   XBT_DEBUG("Create parallel execute synchro %p", exec);
@@ -298,9 +300,6 @@ void SIMIX_execution_finish(simgrid::kernel::activity::ExecImplPtr exec)
     simcall_execution_wait__set__result(simcall, exec->state);
     SIMIX_simcall_answer(simcall);
   }
-
-  /* We no longer need it */
-  exec->unref();
 }
 
 void SIMIX_set_category(smx_activity_t synchro, const char *category)

diff --git a/src/simix/smx_io.cpp b/src/simix/smx_io.cpp
index 4371b8d..d28660d 100644 (file)
--- a/src/simix/smx_io.cpp
+++ b/src/simix/smx_io.cpp
@@ -182,7 +182,6 @@ void SIMIX_io_destroy(smx_activity_t synchro)
   XBT_DEBUG("Destroy synchro %p", synchro);
   if (io->surf_io)
     io->surf_io->unref();
-  io->unref();
 }
 
 void SIMIX_io_finish(smx_activity_t synchro)

diff --git a/src/simix/smx_synchro.cpp b/src/simix/smx_synchro.cpp
index 8ea26ce..1b26ac4 100644 (file)
--- a/src/simix/smx_synchro.cpp
+++ b/src/simix/smx_synchro.cpp
@@ -90,7 +90,6 @@ void SIMIX_synchro_finish(smx_activity_t synchro)
 
   SIMIX_synchro_stop_waiting(simcall->issuer, simcall);
   simcall->issuer->waiting_synchro = nullptr;
-  synchro->unref();
   SIMIX_simcall_answer(simcall);
   XBT_OUT();
 }
@@ -176,7 +175,6 @@ void MutexImpl::unlock(smx_actor_t issuer)
   if (xbt_swag_size(this->sleeping) > 0) {
     /*process to wake up */
     smx_actor_t p = (smx_actor_t) xbt_swag_extract(this->sleeping);
-    p->waiting_synchro->unref();
     p->waiting_synchro = nullptr;
     this->owner = p;
     SIMIX_simcall_answer(&p->simcall);
@@ -319,7 +317,6 @@ void SIMIX_cond_signal(smx_cond_t cond)
   if ((proc = (smx_actor_t) xbt_swag_extract(cond->sleeping))) {
 
     /* Destroy waiter's synchronization */
-    proc->waiting_synchro->unref();
     proc->waiting_synchro = nullptr;
 
     /* Now transform the cond wait simcall into a mutex lock one */
@@ -432,7 +429,6 @@ void SIMIX_sem_release(smx_sem_t sem)
 
   XBT_DEBUG("Sem release semaphore %p", sem);
   if ((proc = (smx_actor_t) xbt_swag_extract(sem->sleeping))) {
-    proc->waiting_synchro->unref();
     proc->waiting_synchro = nullptr;
     SIMIX_simcall_answer(&proc->simcall);
   } else {