fix(engine): coredump when disabling a host with children relation (#…

…1863)

* fix(engine/broker): make parents_host shared ptr & remove relation db when child is deleted
* feat(test): add tests EBPN about relation parent child

REFS: MON-152645

broker/neb/src/callbacks.cc

@@ -2908,12 +2908,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>()};
@@ -2964,10 +2960,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

@@ -336,14 +336,12 @@ static void send_host_parents_list(neb_sender sender = neb::callback_relation) {
          end{com::centreon::engine::host::hosts.end()};
          it != end; ++it) {
       // 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 (auto [_, sptr_host] : it->second->parent_hosts) {
         // Fill callback struct.
         nebstruct_relation_data nsrd;
         memset(&nsrd, 0, sizeof(nsrd));
         nsrd.type = NEBTYPE_PARENT_ADD;
-        nsrd.hst = pit->second;
+        nsrd.hst = sptr_host.get();
         nsrd.dep_hst = it->second.get();
 
         // Callback.

engine/enginerpc/engine.proto

@@ -356,6 +356,9 @@ message EngineHost {
     UNREACHABLE = 2;
   }
   State current_state = 6;
+  string display_name = 7;
+  repeated string parent_hosts = 8;
+  repeated string child_hosts = 9;
 }
 
 message ContactIdentifier {

engine/enginerpc/engine_impl.cc

@@ -245,9 +245,19 @@ grpc::Status engine_impl::GetHost(grpc::ServerContext* context [[maybe_unused]],
         host->set_alias(selectedhost->get_alias());
         host->set_address(selectedhost->get_address());
         host->set_check_period(selectedhost->check_period());
+        host->set_id(selectedhost->host_id());
         host->set_current_state(
             static_cast<EngineHost::State>(selectedhost->get_current_state()));
-        host->set_id(selectedhost->host_id());
+        host->set_display_name(selectedhost->get_display_name());
+
+        if (!selectedhost->parent_hosts.empty())
+          for (const auto& [key, _] : selectedhost->parent_hosts)
+            host->add_parent_hosts(key);
+
+        if (!selectedhost->child_hosts.empty())
+          for (const auto& [key, _] : selectedhost->child_hosts)
+            host->add_child_hosts(key);
+
         return 0;
       });
 

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;
@@ -307,6 +307,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 {
 
@@ -318,6 +319,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/configuration/applier/host.cc

@@ -374,10 +374,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();
@@ -436,6 +434,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)
@@ -470,10 +473,8 @@ void applier::host::resolve_object(configuration::host const& 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.

engine/src/host.cc

@@ -593,6 +593,19 @@ std::ostream& operator<<(std::ostream& os, host_map_unsafe const& obj) {
   return os;
 }
 
+std::ostream& operator<<(std::ostream& os, host_map const& obj) {
+  bool first = true;
+  for (const auto& [key, _] : obj) {
+    if (first) {
+      first = false;
+    } else {
+      os << ", ";
+    }
+    os << key;
+  }
+  return os;
+}
+
 /**
  *  Dump host content into the stream.
  *
@@ -1036,8 +1049,7 @@ int is_host_immediate_child_of_host(com::centreon::engine::host* parent_host,
   }
   // Mid-level/bottom hosts.
   else {
-    host_map_unsafe::const_iterator it{
-        child_host->parent_hosts.find(parent_host->name())};
+    auto it{child_host->parent_hosts.find(parent_host->name())};
     return it != child_host->parent_hosts.end();
   }
 
@@ -1856,8 +1868,8 @@ int host::run_async_check(int check_options,
       try {
         // Run command.
         get_check_command_ptr()->run(processed_cmd, *macros,
-                                                   config->host_check_timeout(),
-                                                   check_result_info);
+                                     config->host_check_timeout(),
+                                     check_result_info);
       } catch (com::centreon::exceptions::interruption const& e) {
         retry = true;
       } catch (std::exception const& e) {
@@ -3157,17 +3169,15 @@ int host::process_check_result_3x(enum host::host_state new_state,
       SPDLOG_LOGGER_DEBUG(checks_logger,
                           "Propagating checks to parent host(s)...");
 
-      for (host_map_unsafe::iterator it{parent_hosts.begin()},
-           end{parent_hosts.end()};
-           it != end; it++) {
-        if (!it->second)
+      for (const auto& [key, sptr_host] : parent_hosts) {
+        if (!sptr_host)
           continue;
-        if (it->second->get_current_state() != host::state_up) {
+        if (sptr_host->get_current_state() != host::state_up) {
           engine_logger(dbg_checks, more)
-              << "Check of parent host '" << it->first << "' queued.";
+              << "Check of parent host '" << key << "' queued.";
           SPDLOG_LOGGER_DEBUG(checks_logger,
-                              "Check of parent host '{}' queued.", it->first);
-          check_hostlist.push_back(it->second);
+                              "Check of parent host '{}' queued.", key);
+          check_hostlist.push_back(sptr_host.get());
         }
       }
 
@@ -3280,24 +3290,21 @@ int host::process_check_result_3x(enum host::host_state new_state,
               "** WARNING: Max attempts = 1, so we have to run serial "
               "checks of all parent hosts!");
 
-          for (host_map_unsafe::iterator it{parent_hosts.begin()},
-               end{parent_hosts.end()};
-               it != end; it++) {
-            if (!it->second)
+          for (const auto& [key, sptr_host] : parent_hosts) {
+            if (!sptr_host)
               continue;
 
             has_parent = true;
 
             engine_logger(dbg_checks, more)
-                << "Running serial check parent host '" << it->first << "'...";
-            SPDLOG_LOGGER_DEBUG(checks_logger,
-                                "Running serial check parent host '{}'...",
-                                it->first);
+                << "Running serial check parent host '" << key << "'...";
+            SPDLOG_LOGGER_DEBUG(
+                checks_logger, "Running serial check parent host '{}'...", key);
 
             /* run an immediate check of the parent host */
-            it->second->run_sync_check_3x(&parent_state, check_options,
-                                          use_cached_result,
-                                          check_timestamp_horizon);
+            sptr_host->run_sync_check_3x(&parent_state, check_options,
+                                         use_cached_result,
+                                         check_timestamp_horizon);
 
             /* bail out as soon as we find one parent host that is UP */
             if (parent_state == host::state_up) {
@@ -3392,17 +3399,15 @@ int host::process_check_result_3x(enum host::host_state new_state,
                             "Propagating checks to immediate parent hosts that "
                             "are UP...");
 
-        for (host_map_unsafe::iterator it{parent_hosts.begin()},
-             end{parent_hosts.end()};
-             it != end; it++) {
-          if (it->second == nullptr)
+        for (const auto& [key, sptr_host] : parent_hosts) {
+          if (sptr_host == nullptr)
             continue;
-          if (it->second->get_current_state() == host::state_up) {
-            check_hostlist.push_back(it->second);
+          if (sptr_host->get_current_state() == host::state_up) {
+            check_hostlist.push_back(sptr_host.get());
             engine_logger(dbg_checks, more)
-                << "Check of host '" << it->first << "' queued.";
+                << "Check of host '" << key << "' queued.";
             SPDLOG_LOGGER_DEBUG(checks_logger, "Check of host '{}' queued.",
-                                it->first);
+                                key);
           }
         }
 
@@ -3644,22 +3649,20 @@ enum host::host_state host::determine_host_reachability(
 
   /* check all parent hosts to see if we're DOWN or UNREACHABLE */
   else {
-    for (host_map_unsafe::iterator it{parent_hosts.begin()},
-         end{parent_hosts.end()};
-         it != end; it++) {
-      if (!it->second)
+    for (const auto& [key, sptr_host] : parent_hosts) {
+      if (!sptr_host)
         continue;
 
       /* bail out as soon as we find one parent host that is UP */
-      if (it->second->get_current_state() == host::state_up) {
+      if (sptr_host->get_current_state() == host::state_up) {
         is_host_present = true;
         /* set the current state */
         state = host::state_down;
-        engine_logger(dbg_checks, most) << "At least one parent (" << it->first
-                                        << ") is up, so host is DOWN.";
+        engine_logger(dbg_checks, most)
+            << "At least one parent (" << key << ") is up, so host is DOWN.";
         SPDLOG_LOGGER_DEBUG(checks_logger,
                             "At least one parent ({}) is up, so host is DOWN.",
-                            it->first);
+                            key);
         break;
       }
     }
@@ -3984,22 +3987,20 @@ void host::resolve(int& w, int& e) {
   }
 
   /* check all parent parent host */
-  for (host_map_unsafe::iterator it(parent_hosts.begin()),
-       end(parent_hosts.end());
-       it != end; it++) {
-    host_map::const_iterator it_host{host::hosts.find(it->first)};
+  for (auto& [key, sptr_host] : parent_hosts) {
+    host_map::const_iterator it_host{host::hosts.find(key)};
     if (it_host == host::hosts.end() || !it_host->second) {
-      engine_logger(log_verification_error, basic) << "Error: '" << it->first
+      engine_logger(log_verification_error, basic) << "Error: '" << key
                                                    << "' is not a "
                                                       "valid parent for host '"
                                                    << name() << "'!";
       config_logger->error("Error: '{}' is not a valid parent for host '{}'!",
-                           it->first, name());
+                           key, name());
       errors++;
     } else {
-      it->second = it_host->second.get();
-      it_host->second->add_child_host(
-          this);  // add a reverse (child) link to make searches faster later on
+      sptr_host = it_host->second;
+      it_host->second->add_child_host(this);  // add a reverse (child) link to
+                                              // make searches faster later on
     }
   }
 

engine/src/macros/grab_host.cc

@@ -184,12 +184,10 @@ std::string get_host_total_services(host& hst, nagios_macros* mac) {
 static std::string get_host_parents(host& hst, nagios_macros* mac) {
   (void)mac;
   std::string retval;
-  for (host_map_unsafe::const_iterator it(hst.parent_hosts.begin()),
-       end(hst.parent_hosts.end());
-       it != end; it++) {
+  for (const auto& [key, _] : hst.parent_hosts) {
     if (!retval.empty())
       retval.append(",");
-    retval.append(it->first);
+    retval.append(key);
   }
   return retval;
 }
@@ -205,12 +203,10 @@ static std::string get_host_parents(host& hst, nagios_macros* mac) {
 static std::string get_host_children(host& hst, nagios_macros* mac) {
   (void)mac;
   std::string retval;
-  for (host_map_unsafe::const_iterator it(hst.child_hosts.begin()),
-       end(hst.child_hosts.end());
-       it != end; it++) {
+  for (const auto& [key, _] : hst.child_hosts) {
     if (!retval.empty())
       retval.append(",");
-    retval.append(it->first);
+    retval.append(key);
   }
   return retval;
 }