From 2983e2d4248b6cdb5c7df3b1bd9b35540e7a32b8 Mon Sep 17 00:00:00 2001
From: Martin Quinson <martin.quinson@loria.fr>
Date: Sun, 16 Oct 2016 11:57:34 +0200
Subject: [PATCH] do standard C++

Destroy the elements explicitly instead of having them destroyed when
they are removed from the dict container.
---
 include/simgrid/s4u/host.hpp   |  7 +++++++
 src/s4u/s4u_host.cpp           | 34 ++++++++++++++++++++++++++--------
 src/simgrid/host.cpp           |  4 ++++
 src/simix/smx_vm.cpp           |  3 +--
 src/surf/network_interface.cpp |  4 ++--
 5 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/include/simgrid/s4u/host.hpp b/include/simgrid/s4u/host.hpp
index 4e59267257..3583defaa0 100644
--- a/include/simgrid/s4u/host.hpp
+++ b/include/simgrid/s4u/host.hpp
@@ -44,7 +44,14 @@ XBT_PUBLIC_CLASS Host :
 
 public:
   explicit Host(const char *name);
+  /** Host destruction logic */
+protected:
   ~Host(); // TODO, make me private
+private:
+  bool currentlyDestroying_ = false;
+
+public:
+  void destroy();
 
   /** Retrieves an host from its name, or return nullptr */
   static Host* by_name_or_null(const char* name);
diff --git a/src/s4u/s4u_host.cpp b/src/s4u/s4u_host.cpp
index 3181507957..cbd28f1cfa 100644
--- a/src/s4u/s4u_host.cpp
+++ b/src/s4u/s4u_host.cpp
@@ -44,27 +44,45 @@ Host::Host(const char* name)
   xbt_dict_set(host_list, name, this, nullptr);
 }
 
-Host::~Host() {
+Host::~Host()
+{
+  xbt_assert(currentlyDestroying_, "Please call h->destroy() instead of manually deleting it.");
+
   delete pimpl_cpu;
   delete pimpl_netcard;
   delete mounts;
 }
 
-Host *Host::by_name(std::string name) {
+/** @brief Fire the required callbacks and destroy the object
+ *
+ * Don't delete directly an Host, call h->destroy() instead.
+ *
+ * This is cumbersome but there is the simplest solution to ensure that the
+ * onDestruction() callback receives a valid object (because of the destructor
+ * order in a class hierarchy).
+ */
+void Host::destroy()
+{
+  if (!currentlyDestroying_) {
+    currentlyDestroying_ = true;
+    xbt_dict_remove(host_list, name().c_str());
+    onDestruction(*this);
+    delete this;
+  }
+}
+
+Host* Host::by_name(std::string name)
+{
   Host* host = Host::by_name_or_null(name.c_str());
   // TODO, raise an exception instead?
   if (host == nullptr)
-    xbt_die("No such host: %s", name.c_str());
+    xbt_die("No such host: '%s'", name.c_str());
   return host;
 }
 Host* Host::by_name_or_null(const char* name)
 {
   if (host_list == nullptr)
-    host_list = xbt_dict_new_homogeneous([](void*p) {
-      simgrid::s4u::Host* host = static_cast<simgrid::s4u::Host*>(p);
-      simgrid::s4u::Host::onDestruction(*host);
-      delete host;
-    });
+    host_list = xbt_dict_new_homogeneous(nullptr);
   return (Host*) xbt_dict_get_or_null(host_list, name);
 }
 
diff --git a/src/simgrid/host.cpp b/src/simgrid/host.cpp
index f3701a734c..fdceaacf89 100644
--- a/src/simgrid/host.cpp
+++ b/src/simgrid/host.cpp
@@ -18,6 +18,10 @@ extern xbt_dict_t host_list; // FIXME:killme don't dupplicate the content of s4u
 
 void sg_host_exit()
 {
+  xbt_dict_cursor_t cursor = nullptr;
+  const char* name         = nullptr;
+  simgrid::s4u::Host* host = nullptr;
+  xbt_dict_foreach(host_list, cursor, name, host) host->destroy();
   xbt_dict_free(&host_list);
 }
 
diff --git a/src/simix/smx_vm.cpp b/src/simix/smx_vm.cpp
index 6c99ae8ded..77c24f0616 100644
--- a/src/simix/smx_vm.cpp
+++ b/src/simix/smx_vm.cpp
@@ -321,6 +321,5 @@ void SIMIX_vm_destroy(sg_host_t vm)
   vm->pimpl_cpu = nullptr;
   vm->pimpl_netcard = nullptr;
 
-  if (xbt_dict_get_or_null(host_list, vm->name().c_str()))
-    xbt_dict_remove(host_list, vm->name().c_str());
+  vm->destroy();
 }
diff --git a/src/surf/network_interface.cpp b/src/surf/network_interface.cpp
index d8f2f15285..a961155827 100644
--- a/src/surf/network_interface.cpp
+++ b/src/surf/network_interface.cpp
@@ -177,9 +177,9 @@ namespace simgrid {
     Link::~Link() {
       xbt_assert(currentlyDestroying_, "Don't delete Links directly. Call destroy() instead.");
     }
-    /** @brief Fire the require callbacks and destroy the object
+    /** @brief Fire the required callbacks and destroy the object
      *
-     * Don't delete directly an Link, call l->destroy() instead.
+     * Don't delete directly a Link, call l->destroy() instead.
      */
     void Link::destroy()
     {
-- 
2.20.1