Removed usage of pthreads and replaced with std C++ features

eduar-hte · eduar-hte · commit 3718ecafaa69 · 2024-08-07T15:05:48.000-07:00
- Replaced pthread_mutex_t in modsecurity::operators::Pm with std::mutex
- Replaced pthread's thread usage in reading_logs_via_rule_message
  example with std::thread.
  - Simplified and modernized C++ code.
- Removed unnecessary includes of pthread.h
diff --git a/build/win32/CMakeLists.txt b/build/win32/CMakeLists.txt
@@ -110,7 +110,6 @@ include(${CMAKE_CURRENT_LIST_DIR}/ConfigureChecks.cmake)
 configure_file(config.h.cmake ${BASE_DIR}/src/config.h)
 
 find_package(PCRE2 REQUIRED)
-find_package(PThreads4W REQUIRED)
 find_package(Poco REQUIRED)
 find_package(dirent REQUIRED)		# used only by tests (check dirent::dirent refernces)
 
@@ -139,7 +138,7 @@ add_library(libModSecurity SHARED ${libModSecuritySources})
 
 target_compile_definitions(libModSecurity PRIVATE WITH_PCRE2)
 target_include_directories(libModSecurity PRIVATE ${BASE_DIR} ${BASE_DIR}/headers ${BASE_DIR}/others ${MBEDTLS_DIR}/include)
-target_link_libraries(libModSecurity PRIVATE pcre2::pcre2 pthreads4w::pthreads4w libinjection mbedcrypto Poco::Poco Iphlpapi.lib)
+target_link_libraries(libModSecurity PRIVATE pcre2::pcre2 libinjection mbedcrypto Poco::Poco Iphlpapi.lib)
 
 macro(add_package_dependency project compile_definition link_library flag)
   if(${flag})
@@ -255,7 +254,6 @@ setExampleTargetProperties(using_bodies_in_chunks)
 # reading_logs_via_rule_message
 add_executable(reading_logs_via_rule_message ${BASE_DIR}/examples/reading_logs_via_rule_message/simple_request.cc)
 setExampleTargetProperties(reading_logs_via_rule_message)
-target_link_libraries(reading_logs_via_rule_message PRIVATE libModSecurity pthreads4w::pthreads4w)
 
 # reading_logs_with_offset
 add_executable(reading_logs_with_offset ${BASE_DIR}/examples/reading_logs_with_offset/read.cc)
diff --git a/examples/multiprocess_c/Makefile.am b/examples/multiprocess_c/Makefile.am
@@ -15,7 +15,6 @@ multi_LDFLAGS = \
 	-L$(top_builddir)/src/.libs/ \
 	$(GEOIP_LDFLAGS) \
 	-lmodsecurity \
-	-lpthread \
 	-lm \
 	-lstdc++ \
 	$(LUA_LDFLAGS) \
diff --git a/examples/reading_logs_via_rule_message/Makefile.am b/examples/reading_logs_via_rule_message/Makefile.am
@@ -21,7 +21,6 @@ simple_request_LDFLAGS = \
 	-L$(top_builddir)/src/.libs/ \
 	$(GEOIP_LDFLAGS) \
 	-lmodsecurity \
-	-lpthread \
 	-lm \
 	-lstdc++ \
 	$(LMDB_LDFLAGS) \
diff --git a/examples/reading_logs_via_rule_message/reading_logs_via_rule_message.h b/examples/reading_logs_via_rule_message/reading_logs_via_rule_message.h
@@ -13,14 +13,19 @@
  *
  */
 
+#ifndef EXAMPLES_READING_LOGS_VIA_RULE_MESSAGE_READING_LOGS_VIA_RULE_MESSAGE_H_
+#define EXAMPLES_READING_LOGS_VIA_RULE_MESSAGE_READING_LOGS_VIA_RULE_MESSAGE_H_
+
 #include <string>
 #include <memory>
 #include <thread>
+#include <array>
 #include <chrono>
-#include <pthread.h>
+
+#include "modsecurity/rule_message.h"
 
 
-#define NUM_THREADS 100
+constexpr auto NUM_THREADS = 100;
 
 
 char request_header[] =  "" \
@@ -62,59 +67,33 @@ char response_body[] = "" \
 
 char ip[] = "200.249.12.31";
 
-#include "modsecurity/rule_message.h"
-
-#ifndef EXAMPLES_READING_LOGS_VIA_RULE_MESSAGE_READING_LOGS_VIA_RULE_MESSAGE_H_
-#define EXAMPLES_READING_LOGS_VIA_RULE_MESSAGE_READING_LOGS_VIA_RULE_MESSAGE_H_
-
 
-struct data_ms {
-    modsecurity::ModSecurity *modsec;
-    modsecurity::RulesSet *rules;
-};
+static void process_request(modsecurity::ModSecurity *modsec, modsecurity::RulesSet *rules) {
+    for (auto z = 0; z < 10000; z++) {
+        auto modsecTransaction = std::make_unique<modsecurity::Transaction>(modsec, rules, nullptr);
 
-#if defined _MSC_VER
-#pragma warning(push)
-#pragma warning(disable:4716)   // avoid error C4716: 'process_request': must return a value, as MSVC C++ compiler doesn't support [[noreturn]]
-#pragma warning(disable:4715)   // avoid warning c4715: 'process_request' : not all control paths return a value
-#endif
-
-[[noreturn]] static void *process_request(void *data) {
-    struct data_ms *a = (struct data_ms *)data;
-    modsecurity::ModSecurity *modsec = a->modsec;
-    modsecurity::RulesSet *rules = a->rules;
-    int z = 0;
-
-    for (z = 0; z < 10000; z++) {
-        modsecurity::Transaction *modsecTransaction = \
-            new modsecurity::Transaction(modsec, rules, NULL);
         modsecTransaction->processConnection(ip, 12345, "127.0.0.1", 80);
         modsecTransaction->processURI(request_uri, "GET", "1.1");
 
         std::this_thread::sleep_for(std::chrono::microseconds(10));
+
         modsecTransaction->addRequestHeader("Host",
             "net.tutsplus.com");
         modsecTransaction->processRequestHeaders();
         modsecTransaction->processRequestBody();
+
         modsecTransaction->addResponseHeader("HTTP/1.1",
             "200 OK");
         modsecTransaction->processResponseHeaders(200, "HTTP 1.2");
         modsecTransaction->appendResponseBody(
             (const unsigned char*)response_body,
             strlen((const char*)response_body));
         modsecTransaction->processResponseBody();
-        modsecTransaction->processLogging();
 
-        delete modsecTransaction;
+        modsecTransaction->processLogging();
     }
-
-    pthread_exit(nullptr);
 }
 
-#if defined _MSC_VER
-#pragma warning(pop)
-#endif
-
 class ReadingLogsViaRuleMessage {
  public:
     ReadingLogsViaRuleMessage(char *request_header,
@@ -134,11 +113,6 @@ class ReadingLogsViaRuleMessage {
         { }
 
     int process() const {
-        pthread_t threads[NUM_THREADS];
-        int i;
-        struct data_ms dms;
-        void *status;
-
         auto modsec = std::make_unique<modsecurity::ModSecurity>();
         modsec->setConnectorInformation("ModSecurity-test v0.0.1-alpha" \
             " (ModSecurity test)");
@@ -152,18 +126,19 @@ class ReadingLogsViaRuleMessage {
             return -1;
         }
 
-        dms.modsec = modsec.get();
-        dms.rules = rules.get();
+        std::array<std::thread, NUM_THREADS> threads;
 
-        for (i = 0; i < NUM_THREADS; i++) {
-            pthread_create(&threads[i], NULL, process_request,
-		reinterpret_cast<void *>(&dms));
+        for (auto i = 0; i != threads.size(); ++i) {
+            threads[i] = std::thread(
+                [&modsec, &rules]() {
+                    process_request(modsec.get(), rules.get());
+                });
         }
 
         std::this_thread::sleep_for(std::chrono::microseconds(10000));
 
-        for (i=0; i < NUM_THREADS; i++) {
-            pthread_join(threads[i], &status);
+        for (auto i = 0; i != threads.size(); ++i) {
+            threads[i].join();
             std::cout << "Main: completed thread id :" << i << std::endl;
         }
 
diff --git a/examples/reading_logs_with_offset/Makefile.am b/examples/reading_logs_with_offset/Makefile.am
@@ -21,7 +21,6 @@ read_LDFLAGS = \
 	-L$(top_builddir)/src/.libs/ \
 	$(GEOIP_LDFLAGS) \
 	-lmodsecurity \
-	-lpthread \
 	-lm \
 	-lstdc++ \
 	$(LMDB_LDFLAGS) \
diff --git a/examples/using_bodies_in_chunks/Makefile.am b/examples/using_bodies_in_chunks/Makefile.am
@@ -21,12 +21,10 @@ simple_request_LDFLAGS = \
 	-L$(top_builddir)/src/.libs/ \
 	$(GEOIP_LDFLAGS) \
 	-lmodsecurity \
-	-lpthread \
 	-lm \
 	-lstdc++ \
 	$(MAXMIND_LDFLAGS) \
 	$(LMDB_LDFLAGS) \
-	-lpthread \
 	$(LUA_LDFLAGS) \
 	$(SSDEEP_LDFLAGS) \
 	$(YAJL_LDFLAGS)
diff --git a/src/collection/backend/in_memory-per_process.cc b/src/collection/backend/in_memory-per_process.cc
@@ -25,8 +25,6 @@
 #include <memory>
 #endif
 
-#include <pthread.h>
-
 #include "modsecurity/variable_value.h"
 #include "src/utils/regex.h"
 #include "src/utils/string.h"
diff --git a/src/collection/backend/lmdb.cc b/src/collection/backend/lmdb.cc
@@ -27,8 +27,6 @@
 #include <string>
 #include <memory>
 
-#include <pthread.h>
-
 #include "modsecurity/variable_value.h"
 #include "src/utils/regex.h"
 #include "src/variables/variable.h"
diff --git a/src/collection/backend/lmdb.h b/src/collection/backend/lmdb.h
@@ -27,12 +27,10 @@
 
 #ifdef WITH_LMDB
 #include <lmdb.h>
-#include <semaphore.h>
 #endif  // WITH_LMDB
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <fcntl.h>
-#include <pthread.h>
 
 #include "modsecurity/variable_value.h"
 #include "modsecurity/collection/collection.h"
diff --git a/src/operators/pm.cc b/src/operators/pm.cc
@@ -39,9 +39,6 @@ Pm::~Pm() {
 
     free(m_p);
     m_p = NULL;
-#ifdef MODSEC_MUTEX_ON_PM
-    pthread_mutex_destroy(&m_lock);
-#endif
 }
 
 
@@ -89,11 +86,12 @@ bool Pm::evaluate(Transaction *transaction, RuleWithActions *rule,
     pt.ptr = NULL;
     const char *match = NULL;
 #ifdef MODSEC_MUTEX_ON_PM
-    pthread_mutex_lock(&m_lock);
+    {
+    const std::lock_guard lock(m_mutex);
 #endif
     rc = acmp_process_quick(&pt, &match, input.c_str(), input.length());
 #ifdef MODSEC_MUTEX_ON_PM
-    pthread_mutex_unlock(&m_lock);
+    }
 #endif
 
     if (rc >= 0 && transaction) {
@@ -118,9 +116,6 @@ bool Pm::init(const std::string &file, std::string *error) {
     std::istringstream *iss;
     const char *err = NULL;
 
-#ifdef MODSEC_MUTEX_ON_PM
-    pthread_mutex_init(&m_lock, NULL);
-#endif
     char *content = parse_pm_content(m_param.c_str(), m_param.length(), &err);
     if (content == NULL) {
         iss = new std::istringstream(m_param);
diff --git a/src/operators/pm.h b/src/operators/pm.h
@@ -20,6 +20,7 @@
 #include <list>
 #include <memory>
 #include <utility>
+#include <mutex>
 
 #include "src/operators/operator.h"
 #include "src/utils/acmp.h"
@@ -56,7 +57,7 @@ class Pm : public Operator {
 #ifdef MODSEC_MUTEX_ON_PM
 
  private:
-    pthread_mutex_t m_lock;
+    std::mutex m_mutex;
 #endif
 };
 
