fix(various): centengine segfaults when it is terminated. (#528)

* fix(externalcmd): pthread_cancel() is not compatible with new C++11 ABI.
* fix(externalcmd): main externalcmd function had a bad name

REFS: MON-10682

CHANGELOG.md

@@ -4,6 +4,11 @@
 
 ### Bugs
 
+*external commands*
+
+The new C++ standard is not compatible with pthread\_cancel(). This last one has
+been removed from the code.
+
 *gRPC*
 
 The reflection module has been removed because of an issue in the compilation.
@@ -20,7 +25,8 @@ one would be enough.
 
 ### Build
 
-repair the compilation for Raspberry PI
+repair the compilation for Raspberry PI. Link between cbmod and neb module is
+also rectified.
 
 ## 21.04.2
 

deps.sh

@@ -2,5 +2,10 @@
 
 go run deps.go "$*" > /tmp/deps.dot
 dot -Tpng /tmp/deps.dot -o deps.png
-eog deps.png&
-
+if [ -x /usr/bin/eog ] ; then
+  eog deps.png&
+elif [ -x /usr/bin/lximage-qt ] ; then
+  lximage-qt deps.png&
+elif [ -x /usr/bin/lximage ] ; then
+  lximage deps.png&
+fi

enginerpc/enginerpc.cc

@@ -17,21 +17,18 @@
  *
  */
 
-#include <sstream>
-#include "com/centreon/engine/logging/logger.hh"
-#include <grpcpp/server_builder.h>
 #include "com/centreon/engine/enginerpc.hh"
+#include <fmt/format.h>
+#include <grpcpp/server_builder.h>
+#include "com/centreon/engine/logging/logger.hh"
 
 using namespace com::centreon::engine;
 
 enginerpc::enginerpc(const std::string& address, uint16_t port) {
-  engine_impl* service = new engine_impl;
-  std::ostringstream oss;
-  oss << address << ":" << port;
-  std::string server_address{oss.str()};
+  std::string server_address{fmt::format("{}:{}", address, port)};
   grpc::ServerBuilder builder;
   builder.AddListeningPort(server_address, grpc::InsecureServerCredentials());
-  builder.RegisterService(service);
+  builder.RegisterService(&_service);
   _server = builder.BuildAndStart();
 }
 

inc/com/centreon/engine/common.hh

@@ -325,7 +325,4 @@ enum ret_val {
 #define HOST_STATECHANGE 0
 #define SERVICE_STATECHANGE 1
 
-/* Thread stuff. */
-#define TOTAL_WORKER_THREADS 1
-
 #endif /* !CCE_COMMON_HH */

inc/com/centreon/engine/enginerpc.hh

@@ -1,15 +1,17 @@
 #ifndef CCE_ENGINERPC_ENGINERPC_HH
 #define CCE_ENGINERPC_ENGINERPC_HH
 
-#include <string>
-#include <memory>
 #include <grpcpp/server.h>
+#include <memory>
+#include <string>
 #include "com/centreon/engine/namespace.hh"
 #include "engine_impl.hh"
 
 CCE_BEGIN()
 class enginerpc final {
+  engine_impl _service;
   std::unique_ptr<grpc::Server> _server;
+
  public:
   enginerpc(const std::string& address, uint16_t port);
   enginerpc() = delete;

inc/com/centreon/engine/globals.hh

@@ -86,7 +86,6 @@ extern time_t program_start;
 extern time_t event_start;
 
 extern circular_buffer external_command_buffer;
-extern pthread_t worker_threads[];
 
 extern check_stats check_statistics[];
 

modules/external_commands/inc/com/centreon/engine/modules/external_commands/utils.hh

@@ -32,8 +32,6 @@ int close_command_file();  // closes and deletes the external command file
                            // (FIFO)
 int init_command_file_worker_thread(void);
 int shutdown_command_file_worker_thread(void);
-void cleanup_command_file_worker_thread(void* arg);
-void* command_file_worker_thread(void* arg);
 int submit_external_command(char const* cmd, int* buffer_items);
 
 #ifdef __cplusplus

modules/external_commands/src/utils.cc

@@ -1,5 +1,6 @@
 /*
 ** Copyright 2011-2013 Merethis
+** Copyright 2020-2021 Centreon
 **
 ** This file is part of Centreon Engine.
 **
@@ -20,15 +21,14 @@
 #include "com/centreon/engine/modules/external_commands/utils.hh"
 #include <fcntl.h>
 #include <poll.h>
-#include <pthread.h>
 #include <sys/stat.h>
-#include <sys/types.h>
 #include <unistd.h>
+#include <atomic>
 #include <cerrno>
 #include <csignal>
 #include <cstdio>
-#include <cstring>
-#include <sstream>
+#include <memory>
+#include <thread>
 #include "com/centreon/engine/common.hh"
 #include "com/centreon/engine/globals.hh"
 #include "com/centreon/engine/logging/logger.hh"
@@ -43,6 +43,9 @@ static int command_file_fd = -1;
 static int command_file_created = false;
 static FILE* command_file_fp = NULL;
 
+static std::unique_ptr<std::thread> worker;
+static std::atomic_bool should_exit{false};
+
 /* creates external command file as a named pipe (FIFO) and opens it for reading
  * (non-blocked mode) */
 int open_command_file(void) {
@@ -64,7 +67,7 @@ int open_command_file(void) {
         (st.st_mode & S_IFIFO))) {
     /* create the external command file as a named pipe (FIFO) */
     if (mkfifo(config->command_file().c_str(),
-                         S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP) != 0) {
+               S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP) != 0) {
       logger(log_runtime_error, basic)
           << "Error: Could not create external command file '"
           << config->command_file() << "' as named pipe: (" << errno << ") -> "
@@ -158,81 +161,10 @@ int close_command_file(void) {
   return OK;
 }
 
-/* initializes command file worker thread */
-int init_command_file_worker_thread(void) {
-  int result = 0;
-  sigset_t newmask;
-
-  /* initialize circular buffer */
-  external_command_buffer.head = 0;
-  external_command_buffer.tail = 0;
-  external_command_buffer.items = 0;
-  external_command_buffer.high = 0;
-  external_command_buffer.overflow = 0L;
-  external_command_buffer.buffer =
-      new void*[config->external_command_buffer_slots()];
-  if (external_command_buffer.buffer == NULL)
-    return ERROR;
-
-  /* initialize mutex (only on cold startup) */
-  if (sigrestart == false)
-    pthread_mutex_init(&external_command_buffer.buffer_lock, NULL);
-
-  /* new thread should block all signals */
-  sigfillset(&newmask);
-  pthread_sigmask(SIG_BLOCK, &newmask, NULL);
-
-  /* create worker thread */
-  result = pthread_create(&worker_threads[COMMAND_WORKER_THREAD], NULL,
-                          command_file_worker_thread, NULL);
-
-  /* main thread should unblock all signals */
-  pthread_sigmask(SIG_UNBLOCK, &newmask, NULL);
-
-  if (result)
-    return ERROR;
-
-  return OK;
-}
-
-/* shutdown command file worker thread */
-int shutdown_command_file_worker_thread(void) {
-  int result = 0;
-
-  /*
-   * calling pthread_cancel(0) will cause segfaults with some
-   * thread libraries. It's possible that will happen if the
-   * user has a number of config files larger than the max
-   * open file descriptor limit (ulimit -n) and some retarded
-   * eventbroker module leaks filedescriptors, since we'll then
-   * enter the cleanup() routine from main() before we've
-   * spawned any threads.
-   */
-  if (worker_threads[COMMAND_WORKER_THREAD]) {
-    /* tell the worker thread to exit */
-    result = pthread_cancel(worker_threads[COMMAND_WORKER_THREAD]);
-
-    /* wait for the worker thread to exit */
-    if (result == 0)
-      pthread_join(worker_threads[COMMAND_WORKER_THREAD], NULL);
-    /* we're being called from a fork()'ed child process - can't cancel thread,
-     * so just cleanup memory */
-    else {
-      cleanup_command_file_worker_thread(NULL);
-    }
-  }
-
-  return OK;
-}
-
 /* clean up resources used by command file worker thread */
-void cleanup_command_file_worker_thread(void* arg) {
-  int x = 0;
-
-  (void)arg;
-
+static void cleanup_command_file_worker_thread() {
   /* release memory allocated to circular buffer */
-  for (x = external_command_buffer.tail; x != external_command_buffer.head;
+  for (int x = external_command_buffer.tail; x != external_command_buffer.head;
        x = (x + 1) % config->external_command_buffer_slots()) {
     delete[]((char**)external_command_buffer.buffer)[x];
     ((char**)external_command_buffer.buffer)[x] = NULL;
@@ -242,24 +174,14 @@ void cleanup_command_file_worker_thread(void* arg) {
 }
 
 /* worker thread - artificially increases buffer of named pipe */
-void* command_file_worker_thread(void* arg) {
+static void command_file_worker_thread() {
   char input_buffer[MAX_EXTERNAL_COMMAND_LENGTH];
   struct pollfd pfd;
   int pollval;
   struct timeval tv;
   int buffer_items = 0;
 
-  (void)arg;
-
-  /* specify cleanup routine */
-  pthread_cleanup_push(cleanup_command_file_worker_thread, NULL);
-
-  /* set cancellation info */
-  pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, NULL);
-  pthread_setcanceltype(PTHREAD_CANCEL_DEFERRED, NULL);
-
-  while (1) {
-    /* should we shutdown? */
+  while (!should_exit) {
     pthread_testcancel();
 
     /* wait for data to arrive */
@@ -311,9 +233,6 @@ void* command_file_worker_thread(void* arg) {
       continue;
     }
 
-    /* should we shutdown? */
-    pthread_testcancel();
-
     /* get number of items in the buffer */
     pthread_mutex_lock(&external_command_buffer.buffer_lock);
     buffer_items = external_command_buffer.items;
@@ -338,7 +257,8 @@ void* command_file_worker_thread(void* arg) {
       clearerr(command_file_fp);
 
       /* read and process the next command in the file */
-      while (fgets(input_buffer, (int)(sizeof(input_buffer) - 1),
+      while (!should_exit &&
+             fgets(input_buffer, (int)(sizeof(input_buffer) - 1),
                    command_file_fp) != nullptr) {
         // Check if command is thread-safe (for immediate execution).
         if (modules::external_commands::gl_processor.is_thread_safe(
@@ -347,33 +267,61 @@ void* command_file_worker_thread(void* arg) {
         // Submit the external command for processing
         // (retry if buffer is full).
         else {
-          while (submit_external_command(input_buffer, &buffer_items) ==
+          while (!should_exit &&
+                 submit_external_command(input_buffer, &buffer_items) ==
                      ERROR &&
                  buffer_items == config->external_command_buffer_slots()) {
             // Wait a bit.
             tv.tv_sec = 0;
             tv.tv_usec = 250000;
             select(0, nullptr, nullptr, nullptr, &tv);
-
-            // Should we shutdown?
-            pthread_testcancel();
           }
 
           // Bail if the circular buffer is full.
           if (buffer_items == config->external_command_buffer_slots())
             break;
-
-          // Should we shutdown?
-          pthread_testcancel();
         }
       }
     }
   }
 
-  /* removes cleanup handler - this should never be reached */
-  pthread_cleanup_pop(0);
+  cleanup_command_file_worker_thread();
+}
+
+/* initializes command file worker thread */
+int init_command_file_worker_thread(void) {
+  /* initialize circular buffer */
+  external_command_buffer.head = 0;
+  external_command_buffer.tail = 0;
+  external_command_buffer.items = 0;
+  external_command_buffer.high = 0;
+  external_command_buffer.overflow = 0L;
+  external_command_buffer.buffer =
+      new void*[config->external_command_buffer_slots()];
+  if (external_command_buffer.buffer == NULL)
+    return ERROR;
+
+  /* initialize mutex (only on cold startup) */
+  if (!sigrestart)
+    pthread_mutex_init(&external_command_buffer.buffer_lock, NULL);
+
+  /* create worker thread */
+  worker = std::make_unique<std::thread>(&command_file_worker_thread);
 
-  return nullptr;
+  return OK;
+}
+
+/* shutdown command file worker thread */
+int shutdown_command_file_worker_thread(void) {
+  if (!should_exit) {
+    /* tell the worker thread to exit */
+    should_exit = true;
+
+    /* wait for the worker thread to exit */
+    worker->join();
+  }
+
+  return OK;
 }
 
 /* submits an external command for processing */

src/globals.cc

@@ -22,8 +22,6 @@
 
 #include "com/centreon/engine/globals.hh"
 
-#include <map>
-
 #include "com/centreon/engine/logging/logger.hh"
 #include "nagios.h"
 
@@ -70,7 +68,6 @@ int test_scheduling(false);
 int verify_circular_paths(true);
 int verify_config(false);
 nebcallback* neb_callback_list[NEBCALLBACK_NUMITEMS];
-pthread_t worker_threads[TOTAL_WORKER_THREADS];
 sched_info scheduling_info;
 time_t event_start((time_t)-1);
 time_t last_command_check((time_t)-1);