fix(engine/broker): segfault relations child/parent host (#1846)

broker/neb/src/callbacks.cc

@@ -2602,12 +2602,8 @@ int neb::callback_relation(int callback_type, void* data) {
       if (relation->hst && relation->dep_hst && !relation->svc &&
           !relation->dep_svc) {
         // Find host IDs.
-        int host_id;
-        int parent_id;
-        {
-          host_id = engine::get_host_id(relation->dep_hst->name());
-          parent_id = engine::get_host_id(relation->hst->name());
-        }
+        int host_id = relation->dep_hst->host_id();
+        int parent_id = relation->hst->host_id();
         if (host_id && parent_id) {
           // Generate parent event.
           auto new_host_parent{std::make_shared<host_parent>()};
@@ -2658,10 +2654,8 @@ int neb::callback_pb_relation(int callback_type [[maybe_unused]], void* data) {
       if (relation->hst && relation->dep_hst && !relation->svc &&
           !relation->dep_svc) {
         // Find host IDs.
-        int host_id;
-        int parent_id;
-        host_id = engine::get_host_id(relation->dep_hst->name());
-        parent_id = engine::get_host_id(relation->hst->name());
+        int host_id = relation->dep_hst->host_id();
+        int parent_id = relation->hst->host_id();
         if (host_id && parent_id) {
           // Generate parent event.
           auto new_host_parent{std::make_shared<pb_host_parent>()};

broker/neb/src/initial.cc

@@ -288,19 +288,15 @@ static void send_host_parents_list(neb_sender sender = neb::callback_relation) {
 
   try {
     // Loop through all hosts.
-    for (host_map::iterator it{com::centreon::engine::host::hosts.begin()},
-         end{com::centreon::engine::host::hosts.end()};
-         it != end; ++it) {
+    for (const auto& [_, sptr_host] : com::centreon::engine::host::hosts) {
       // Loop through all parents.
-      for (host_map_unsafe::iterator pit{it->second->parent_hosts.begin()},
-           pend{it->second->parent_hosts.end()};
-           pit != pend; ++pit) {
+      for (const auto& [_, sptr_host_parent] : sptr_host->parent_hosts) {
         // Fill callback struct.
         nebstruct_relation_data nsrd;
         memset(&nsrd, 0, sizeof(nsrd));
         nsrd.type = NEBTYPE_PARENT_ADD;
-        nsrd.hst = pit->second;
-        nsrd.dep_hst = it->second.get();
+        nsrd.hst = sptr_host_parent.get();
+        nsrd.dep_hst = sptr_host.get();
 
         // Callback.
         sender(NEBTYPE_PARENT_ADD, &nsrd);

engine/inc/com/centreon/engine/host.hh

@@ -250,7 +250,7 @@ class host : public notifier {
   void set_check_command_ptr(
       const std::shared_ptr<commands::command>& cmd) override;
 
-  host_map_unsafe parent_hosts;
+  host_map parent_hosts;
   host_map_unsafe child_hosts;
   static host_map hosts;
   static host_id_map hosts_by_id;
@@ -309,6 +309,7 @@ int number_of_total_parent_hosts(com::centreon::engine::host* hst);
 std::ostream& operator<<(std::ostream& os,
                          com::centreon::engine::host const& obj);
 std::ostream& operator<<(std::ostream& os, host_map_unsafe const& obj);
+std::ostream& operator<<(std::ostream& os, host_map const& obj);
 
 namespace com::centreon::engine {
 
@@ -320,6 +321,4 @@ std::string get_host_name(const uint64_t host_id);
 
 }  // namespace com::centreon::engine
 
-std::ostream& operator<<(std::ostream& os, host_map_unsafe const& obj);
-
 #endif  // !CCE_HOST_HH

engine/src/command_manager.cc

@@ -411,19 +411,17 @@ void command_manager::schedule_and_propagate_downtime(
     unsigned long triggered_by,
     unsigned long duration) {
   /* check all child hosts... */
-  for (host_map_unsafe::iterator it(temp_host->child_hosts.begin()),
-       end(temp_host->child_hosts.end());
-       it != end; ++it) {
-    if (it->second == nullptr)
+  for (const auto& [_, ptr_host] : temp_host->child_hosts) {
+    if (ptr_host == nullptr)
       continue;
     /* recurse... */
-    schedule_and_propagate_downtime(it->second, entry_time, author,
-                                    comment_data, start_time, end_time, fixed,
-                                    triggered_by, duration);
+    schedule_and_propagate_downtime(ptr_host, entry_time, author, comment_data,
+                                    start_time, end_time, fixed, triggered_by,
+                                    duration);
 
     /* schedule downtime for this host */
     downtime_manager::instance().schedule_downtime(
-        downtime::host_downtime, it->second->host_id(), 0, entry_time, author,
+        downtime::host_downtime, ptr_host->host_id(), 0, entry_time, author,
         comment_data, start_time, end_time, fixed, triggered_by, duration,
         nullptr);
   }

engine/src/commands/commands.cc

@@ -2444,28 +2444,24 @@ void enable_and_propagate_notifications(host* hst,
     enable_host_notifications(hst);
 
   /* check all child hosts... */
-  for (host_map_unsafe::iterator it(hst->child_hosts.begin()),
-       end(hst->child_hosts.end());
-       it != end; ++it) {
-    if (it->second == nullptr)
+  for (const auto& [_, ptr_host] : hst->child_hosts) {
+    if (ptr_host == nullptr)
       continue;
 
     /* recurse... */
-    enable_and_propagate_notifications(it->second, level + 1, affect_top_host,
+    enable_and_propagate_notifications(ptr_host, level + 1, affect_top_host,
                                        affect_hosts, affect_services);
 
     /* enable notifications for this host */
     if (affect_hosts)
-      enable_host_notifications(it->second);
+      enable_host_notifications(ptr_host);
 
     /* enable notifications for all services on this host... */
     if (affect_services) {
-      for (service_map_unsafe::iterator it2(it->second->services.begin()),
-           end2(it->second->services.end());
-           it2 != end2; ++it2) {
-        if (!it2->second)
+      for (const auto& [_, ptr_srv] : ptr_host->services) {
+        if (!ptr_srv)
           continue;
-        enable_service_notifications(it2->second);
+        enable_service_notifications(ptr_srv);
       }
     }
   }
@@ -2485,28 +2481,24 @@ void disable_and_propagate_notifications(host* hst,
     disable_host_notifications(hst);
 
   /* check all child hosts... */
-  for (host_map_unsafe::iterator it(hst->child_hosts.begin()),
-       end(hst->child_hosts.begin());
-       it != end; ++it) {
-    if (!it->second)
+  for (const auto& [_, ptr_host] : hst->child_hosts) {
+    if (!ptr_host)
       continue;
 
     /* recurse... */
-    disable_and_propagate_notifications(it->second, level + 1, affect_top_host,
+    disable_and_propagate_notifications(ptr_host, level + 1, affect_top_host,
                                         affect_hosts, affect_services);
 
     /* disable notifications for this host */
     if (affect_hosts)
-      disable_host_notifications(it->second);
+      disable_host_notifications(ptr_host);
 
     /* disable notifications for all services on this host... */
     if (affect_services) {
-      for (service_map_unsafe::iterator it2(it->second->services.begin()),
-           end2(it->second->services.end());
-           it2 != end2; ++it2) {
-        if (!it2->second)
+      for (const auto& [_, ptr_srv] : ptr_host->services) {
+        if (!ptr_srv)
           continue;
-        disable_service_notifications(it2->second);
+        disable_service_notifications(ptr_srv);
       }
     }
   }
@@ -2627,20 +2619,18 @@ void schedule_and_propagate_downtime(host* temp_host,
                                      unsigned long triggered_by,
                                      unsigned long duration) {
   /* check all child hosts... */
-  for (host_map_unsafe::iterator it(temp_host->child_hosts.begin()),
-       end(temp_host->child_hosts.end());
-       it != end; ++it) {
-    if (it->second == nullptr)
+  for (const auto& [_, ptr_host] : temp_host->child_hosts) {
+    if (ptr_host == nullptr)
       continue;
 
     /* recurse... */
-    schedule_and_propagate_downtime(it->second, entry_time, author,
-                                    comment_data, start_time, end_time, fixed,
-                                    triggered_by, duration);
+    schedule_and_propagate_downtime(ptr_host, entry_time, author, comment_data,
+                                    start_time, end_time, fixed, triggered_by,
+                                    duration);
 
     /* schedule downtime for this host */
     downtime_manager::instance().schedule_downtime(
-        downtime::host_downtime, it->second->host_id(), 0, entry_time, author,
+        downtime::host_downtime, ptr_host->host_id(), 0, entry_time, author,
         comment_data, start_time, end_time, fixed, triggered_by, duration,
         nullptr);
   }

engine/src/config.cc

@@ -63,19 +63,17 @@ static int dfs_host_path(host* root) {
   dfs_set_status(root, DFS_TEMP_CHECKED);
 
   /* We are scanning the children */
-  for (host_map_unsafe::iterator it(root->child_hosts.begin()),
-       end(root->child_hosts.end());
-       it != end; it++) {
-    int child_status = dfs_get_status(it->second);
+  for (const auto& [_, ptr_host] : root->child_hosts) {
+    int child_status = dfs_get_status(ptr_host);
 
     /* If a child is not checked, check it */
     if (child_status == DFS_UNCHECKED)
-      child_status = dfs_host_path(it->second);
+      child_status = dfs_host_path(ptr_host);
 
     /* If a child already temporary checked, its a problem,
      * loop inside, and its a acked status */
     if (child_status == DFS_TEMP_CHECKED) {
-      dfs_set_status(it->second, DFS_LOOPY);
+      dfs_set_status(ptr_host, DFS_LOOPY);
       dfs_set_status(root, DFS_LOOPY);
     }
 
@@ -86,7 +84,7 @@ static int dfs_host_path(host* root) {
         dfs_set_status(root, DFS_NEAR_LOOP);
 
       /* we already saw this child, it's a problem */
-      dfs_set_status(it->second, DFS_LOOPY);
+      dfs_set_status(ptr_host, DFS_LOOPY);
     }
   }
 

engine/src/configuration/applier/host.cc

@@ -482,10 +482,8 @@ void applier::host::modify_object(configuration::host const& obj) {
   if (obj.parents() != obj_old.parents()) {
     // Delete old parents.
     {
-      for (host_map_unsafe::iterator it(it_obj->second->parent_hosts.begin()),
-           end(it_obj->second->parent_hosts.end());
-           it != end; it++)
-        broker_relation_data(NEBTYPE_PARENT_DELETE, it->second, nullptr,
+      for (const auto& [_, sptr_host] : it_obj->second->parent_hosts)
+        broker_relation_data(NEBTYPE_PARENT_DELETE, sptr_host.get(), nullptr,
                              it_obj->second.get(), nullptr);
     }
     it_obj->second->parent_hosts.clear();
@@ -727,10 +725,9 @@ void applier::host::modify_object(configuration::Host* old_obj,
 
   if (parents_changed) {
     // Delete old parents.
-    for (auto it = h->parent_hosts.begin(), end = h->parent_hosts.end();
-         it != end; it++)
-      broker_relation_data(NEBTYPE_PARENT_DELETE, it->second, nullptr, h.get(),
-                           nullptr);
+    for (const auto& [_, sptr_host] : h->parent_hosts)
+      broker_relation_data(NEBTYPE_PARENT_DELETE, sptr_host.get(), nullptr,
+                           h.get(), nullptr);
     h->parent_hosts.clear();
 
     // Create parents.
@@ -787,6 +784,11 @@ void applier::host::remove_object(configuration::host const& obj) {
     for (auto& it_h : it->second->get_parent_groups())
       it_h->members.erase(it->second->name());
 
+    // remove any relations
+    for (const auto& [_, sptr_host] : it->second->parent_hosts)
+      broker_relation_data(NEBTYPE_PARENT_DELETE, sptr_host.get(), nullptr,
+                           it->second.get(), nullptr);
+
     // Notify event broker.
     for (auto it_s = it->second->services.begin();
          it_s != it->second->services.end(); ++it_s)
@@ -833,6 +835,11 @@ void applier::host::remove_object(ssize_t idx) {
     for (auto& it_h : it->second->get_parent_groups())
       it_h->members.erase(it->second->name());
 
+    // remove any relations
+    for (const auto& [_, sptr_host] : it->second->parent_hosts)
+      broker_relation_data(NEBTYPE_PARENT_DELETE, sptr_host.get(), nullptr,
+                           it->second.get(), nullptr);
+
     // Notify event broker.
     for (auto it_s = it->second->services.begin();
          it_s != it->second->services.end(); ++it_s)
@@ -870,10 +877,8 @@ void applier::host::resolve_object(const configuration::host& obj,
   // It is necessary to do it only once to prevent the removal
   // of valid child backlinks.
   if (obj == *config->hosts().begin()) {
-    for (host_map::iterator it(engine::host::hosts.begin()),
-         end(engine::host::hosts.end());
-         it != end; ++it)
-      it->second->child_hosts.clear();
+    for (const auto& [_, sptr_host] : engine::host::hosts)
+      sptr_host->child_hosts.clear();
   }
 
   // Find host.
@@ -911,10 +916,8 @@ void applier::host::resolve_object(const configuration::Host& obj,
   // It is necessary to do it only once to prevent the removal
   // of valid child backlinks.
   if (&obj == &(*pb_config.hosts().begin())) {
-    for (host_map::iterator it(engine::host::hosts.begin()),
-         end(engine::host::hosts.end());
-         it != end; ++it)
-      it->second->child_hosts.clear();
+    for (const auto& [_, sptr_host] : engine::host::hosts)
+      sptr_host->child_hosts.clear();
   }
 
   // Find host.