Mix XML and C++ platforms

Fixes https://framagit.org/simgrid/simgrid/-/issues/99

Main changes:
- Allows adding resources to a netzone after loading the XML platform.
- Disallows adding resources to a sealed netzone.
- Move the seal of resources from s4u::Engine:run to EngineImpl::run.
- Remove seal from load_platform (to permit adding resources to netzones)

diff --git a/ChangeLog b/ChangeLog
index a563d40..d6c10fd 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -23,6 +23,10 @@ Python:
  - Thread contexts are used by default with Python bindings.  Other kinds of
    contexts revealed unstable, specially starting with pybind11 v2.8.0.
 
+Fixed bugs (FG#.. -> FramaGit bugs; FG!.. -> FG merge requests)
+ (FG: issues on Framagit; GF: issues on GForge; GH: issues on GitHub)
+ - FG#99: Weird segfault when not sealing an host
+
 ----------------------------------------------------------------------------
 
 SimGrid (3.29) October 7. 2021

diff --git a/include/simgrid/s4u/Engine.hpp b/include/simgrid/s4u/Engine.hpp
index 31f6a40..7b298a4 100644 (file)
--- a/include/simgrid/s4u/Engine.hpp
+++ b/include/simgrid/s4u/Engine.hpp
@@ -58,6 +58,7 @@ public:
   static bool has_instance() { return instance_ != nullptr; }
 
   void load_platform(const std::string& platf) const;
+  void seal_platform() const;
 
   void register_function(const std::string& name, const std::function<void(int, char**)>& code);
   void register_function(const std::string& name, const std::function<void(std::vector<std::string>)>& code);

diff --git a/src/kernel/EngineImpl.cpp b/src/kernel/EngineImpl.cpp
index 665e7cb..4a38f96 100644 (file)
--- a/src/kernel/EngineImpl.cpp
+++ b/src/kernel/EngineImpl.cpp
@@ -347,6 +347,15 @@ void EngineImpl::shutdown()
   instance_ = nullptr;
 }
 
+void EngineImpl::seal_platform() const
+{
+  /* sealing resources before run: links */
+  for (auto const& kv : links_)
+    kv.second->get_iface()->seal();
+  /* seal netzone root, recursively seal children netzones, hosts and disks */
+  netzone_root_->seal();
+}
+
 void EngineImpl::load_platform(const std::string& platf)
 {
   double start = xbt_os_time();
@@ -689,6 +698,8 @@ double EngineImpl::solve(double max_date) const
 
 void EngineImpl::run(double max_date)
 {
+  seal_platform();
+
   if (MC_record_replay_is_active()) {
     mc::replay(MC_record_path());
     empty_trash();

diff --git a/src/kernel/EngineImpl.hpp b/src/kernel/EngineImpl.hpp
index ed0c976..2fcab66 100644 (file)
--- a/src/kernel/EngineImpl.hpp
+++ b/src/kernel/EngineImpl.hpp
@@ -94,6 +94,7 @@ public:
   void initialize(int* argc, char** argv);
   void load_platform(const std::string& platf);
   void load_deployment(const std::string& file) const;
+  void seal_platform() const;
   void register_function(const std::string& name, const actor::ActorCodeFactory& code);
   void register_default(const actor::ActorCodeFactory& code);
 

diff --git a/src/kernel/routing/NetZoneImpl.cpp b/src/kernel/routing/NetZoneImpl.cpp
index 7f8f8ee..68768af 100644 (file)
--- a/src/kernel/routing/NetZoneImpl.cpp
+++ b/src/kernel/routing/NetZoneImpl.cpp
@@ -151,6 +151,7 @@ s4u::Host* NetZoneImpl::create_host(const std::string& name, const std::vector<d
   xbt_assert(cpu_model_pm_,
              "Impossible to create host: %s. Invalid CPU model: nullptr. Have you set the parent of this NetZone: %s?",
              name.c_str(), get_cname());
+  xbt_assert(not sealed_, "Impossible to create host: %s. NetZone %s already sealed", name.c_str(), get_cname());
   auto* res = (new resource::HostImpl(name))->get_iface();
   res->set_netpoint((new NetPoint(name, NetPoint::Type::Host))->set_englobing_zone(this));
 
@@ -165,12 +166,19 @@ s4u::Link* NetZoneImpl::create_link(const std::string& name, const std::vector<d
       network_model_,
       "Impossible to create link: %s. Invalid network model: nullptr. Have you set the parent of this NetZone: %s?",
       name.c_str(), get_cname());
+  xbt_assert(not sealed_, "Impossible to create link: %s. NetZone %s already sealed", name.c_str(), get_cname());
   return network_model_->create_link(name, bandwidths)->get_iface();
 }
 
 s4u::SplitDuplexLink* NetZoneImpl::create_split_duplex_link(const std::string& name,
                                                             const std::vector<double>& bandwidths)
 {
+  xbt_assert(
+      network_model_,
+      "Impossible to create link: %s. Invalid network model: nullptr. Have you set the parent of this NetZone: %s?",
+      name.c_str(), get_cname());
+  xbt_assert(not sealed_, "Impossible to create link: %s. NetZone %s already sealed", name.c_str(), get_cname());
+
   auto* link_up                  = network_model_->create_link(name + "_UP", bandwidths);
   auto* link_down                = network_model_->create_link(name + "_DOWN", bandwidths);
   auto link                      = std::make_unique<resource::SplitDuplexLinkImpl>(name, link_up, link_down);
@@ -184,6 +192,7 @@ s4u::Disk* NetZoneImpl::create_disk(const std::string& name, double read_bandwid
   xbt_assert(disk_model_,
              "Impossible to create disk: %s. Invalid disk model: nullptr. Have you set the parent of this NetZone: %s?",
              name.c_str(), get_cname());
+  xbt_assert(not sealed_, "Impossible to create disk: %s. NetZone %s already sealed", name.c_str(), get_cname());
   auto* l = disk_model_->create_disk(name, read_bandwidth, write_bandwidth);
 
   return l->get_iface();
@@ -193,6 +202,7 @@ NetPoint* NetZoneImpl::create_router(const std::string& name)
 {
   xbt_assert(nullptr == s4u::Engine::get_instance()->netpoint_by_name_or_null(name),
              "Refusing to create a router named '%s': this name already describes a node.", name.c_str());
+  xbt_assert(not sealed_, "Impossible to create router: %s. NetZone %s already sealed", name.c_str(), get_cname());
 
   return (new NetPoint(name, NetPoint::Type::Router))->set_englobing_zone(this);
 }

diff --git a/src/s4u/s4u_Engine.cpp b/src/s4u/s4u_Engine.cpp
index 0d1c8f8..68e1476 100644 (file)
--- a/src/s4u/s4u_Engine.cpp
+++ b/src/s4u/s4u_Engine.cpp
@@ -112,6 +112,16 @@ void Engine::load_platform(const std::string& platf) const
   pimpl->load_platform(platf);
 }
 
+/**
+ * @brief Seals the platform, finishing the creation of its resources.
+ *
+ * This method is optional. The seal() is done automatically when you call Engine::run.
+ */
+void Engine::seal_platform() const
+{
+  pimpl->seal_platform();
+}
+
 /** Registers the main function of an actor that will be launched from the deployment file */
 void Engine::register_function(const std::string& name, const std::function<void(int, char**)>& code)
 {
@@ -321,12 +331,6 @@ void Engine::run() const
 }
 void Engine::run_until(double max_date) const
 {
-  /* sealing resources before run: links */
-  for (auto* link : get_all_links())
-    link->seal();
-  /* seal netzone root, recursively seal children netzones, hosts and disks */
-  get_netzone_root()->seal();
-
   /* Clean IO before the run */
   fflush(stdout);
   fflush(stderr);

diff --git a/src/surf/sg_platf.cpp b/src/surf/sg_platf.cpp
index bf52327..dd91fbf 100644 (file)
--- a/src/surf/sg_platf.cpp
+++ b/src/surf/sg_platf.cpp
@@ -223,27 +223,25 @@ static void sg_platf_new_cluster_hierarchical(const simgrid::kernel::routing::Cl
   }
 
   simgrid::s4u::NetZone const* parent = current_routing ? current_routing->get_iface() : nullptr;
-  simgrid::s4u::NetZone* zone;
   switch (cluster->topology) {
     case simgrid::kernel::routing::ClusterTopology::TORUS:
-      zone = simgrid::s4u::create_torus_zone(
-          cluster->id, parent, TorusZone::parse_topo_parameters(cluster->topo_parameters),
-          {set_host, set_loopback, set_limiter}, cluster->bw, cluster->lat, cluster->sharing_policy);
+      simgrid::s4u::create_torus_zone(cluster->id, parent, TorusZone::parse_topo_parameters(cluster->topo_parameters),
+                                      {set_host, set_loopback, set_limiter}, cluster->bw, cluster->lat,
+                                      cluster->sharing_policy);
       break;
     case simgrid::kernel::routing::ClusterTopology::DRAGONFLY:
-      zone = simgrid::s4u::create_dragonfly_zone(
+      simgrid::s4u::create_dragonfly_zone(
           cluster->id, parent, DragonflyZone::parse_topo_parameters(cluster->topo_parameters),
           {set_host, set_loopback, set_limiter}, cluster->bw, cluster->lat, cluster->sharing_policy);
       break;
     case simgrid::kernel::routing::ClusterTopology::FAT_TREE:
-      zone = simgrid::s4u::create_fatTree_zone(
+      simgrid::s4u::create_fatTree_zone(
           cluster->id, parent, FatTreeZone::parse_topo_parameters(cluster->topo_parameters),
           {set_host, set_loopback, set_limiter}, cluster->bw, cluster->lat, cluster->sharing_policy);
       break;
     default:
       THROW_IMPOSSIBLE;
   }
-  zone->seal();
 }
 
 /*************************************************************************************************/
@@ -336,7 +334,6 @@ static void sg_platf_new_cluster_flat(simgrid::kernel::routing::ClusterCreationA
   auto* router = zone->create_router(cluster->router_id);
   zone->add_route(router, nullptr, nullptr, nullptr, {});
 
-  zone->seal();
   simgrid::kernel::routing::on_cluster_creation(*cluster);
 }
 
@@ -602,7 +599,6 @@ void sg_platf_new_zone_seal()
     zone_cluster.cabinets.clear();
     zone_cluster.backbone.reset();
   }
-  current_routing->seal();
   current_routing = current_routing->get_parent();
 }
 

diff --git a/teshsuite/platforms/flatifier.cpp b/teshsuite/platforms/flatifier.cpp
index 10bc1ae..2ab089a 100644 (file)
--- a/teshsuite/platforms/flatifier.cpp
+++ b/teshsuite/platforms/flatifier.cpp
@@ -40,6 +40,7 @@ static void create_environment(xbt_os_timer_t parse_time, const std::string& pla
 {
   xbt_os_cputimer_start(parse_time);
   sg4::Engine::get_instance()->load_platform(platformFile);
+  sg4::Engine::get_instance()->seal_platform();
   xbt_os_cputimer_stop(parse_time);
 }
 

diff --git a/teshsuite/s4u/basic-parsing-test/basic-parsing-test.cpp b/teshsuite/s4u/basic-parsing-test/basic-parsing-test.cpp
index 442faf3..cba0d49 100644 (file)
--- a/teshsuite/s4u/basic-parsing-test/basic-parsing-test.cpp
+++ b/teshsuite/s4u/basic-parsing-test/basic-parsing-test.cpp
@@ -63,6 +63,7 @@ int main(int argc, char** argv)
 
   /* creation of the environment */
   e.load_platform(argv[1]);
+  e.seal_platform();
   XBT_INFO("Workstation number: %zu, link number: %zu", e.get_host_count(), e.get_link_count());
 
   std::vector<sg4::Host*> hosts = e.get_all_hosts();