This is an automated email from the ASF dual-hosted git repository.

swebb2066 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/logging-log4cxx.git


The following commit(s) were added to refs/heads/master by this push:
     new 587f4938 Threads created by Log4cxx no longer depend on APR thread 
support (#297)
587f4938 is described below

commit 587f493881941a3a71f70d430adaec4f90b2beaf
Author: Stephen Webb <[email protected]>
AuthorDate: Tue Nov 21 09:56:58 2023 +1100

    Threads created by Log4cxx no longer depend on APR thread support (#297)
    
    * The FileWatchDog thread no longer uses APR threads
    
    * The AsyncAppendr thread no longer uses APR threads
    
    * Synchonization should not depend on APR threads
    
    * Use pthread_self() instead of apr_os_thread_current() in non-WIN32 systems
    
    * Add a test to ensure the Log4cxx created thread runs
---
 src/examples/cpp/delayedloop.cpp                   | 10 +---------
 src/main/cpp/aprinitializer.cpp                    | 11 -----------
 src/main/cpp/class.cpp                             |  2 --
 src/main/cpp/domconfigurator.cpp                   | 22 ---------------------
 src/main/cpp/loggingevent.cpp                      | 23 ++++++++++------------
 src/main/cpp/optionconverter.cpp                   |  6 +-----
 src/main/cpp/propertyconfigurator.cpp              |  5 -----
 src/main/include/CMakeLists.txt                    |  2 ++
 .../include/log4cxx/private/log4cxx_private.h.in   |  1 +
 src/test/cpp/asyncappendertestcase.cpp             |  2 --
 src/test/cpp/helpers/charsetencodertestcase.cpp    |  4 ----
 src/test/cpp/helpers/threadutilitytestcase.cpp     |  7 ++++++-
 src/test/cpp/net/socketappendertestcase.cpp        |  2 --
 src/test/cpp/net/telnetappendertestcase.cpp        |  2 --
 src/test/cpp/net/xmlsocketappendertestcase.cpp     |  3 +--
 15 files changed, 22 insertions(+), 80 deletions(-)

diff --git a/src/examples/cpp/delayedloop.cpp b/src/examples/cpp/delayedloop.cpp
index d1768135..1562d512 100644
--- a/src/examples/cpp/delayedloop.cpp
+++ b/src/examples/cpp/delayedloop.cpp
@@ -68,19 +68,11 @@ public:
                 if(configFile.length() > 4 &&
                      configFile.substr(configFile.length() - 4) == ".xml")
                 {
-#if APR_HAS_THREADS
-               xml::DOMConfigurator::configureAndWatch(configFile, 3000);
-#else
-               xml::DOMConfigurator::configure(configFile);
-#endif
+                        xml::DOMConfigurator::configureAndWatch(configFile, 
3000);
                 }
                 else
                 {
-#if APR_HAS_THREADS
                         PropertyConfigurator::configureAndWatch(configFile, 
3000);
-#else
-                        PropertyConfigurator::configure(configFile);
-#endif
                 }
         }
 
diff --git a/src/main/cpp/aprinitializer.cpp b/src/main/cpp/aprinitializer.cpp
index b27e55af..58c44fad 100644
--- a/src/main/cpp/aprinitializer.cpp
+++ b/src/main/cpp/aprinitializer.cpp
@@ -92,7 +92,6 @@ APRInitializer::APRInitializer() :
 #if APR_HAS_THREADS
        apr_status_t stat = apr_threadkey_private_create(&m_priv->tlsKey, 
tlsDestructImpl, m_priv->p);
        assert(stat == APR_SUCCESS);
-       assert(stat == APR_SUCCESS);
 #endif
     apr_status_t stat2 = apr_dbd_init(m_priv->p);
     assert(stat2 == APR_SUCCESS);
@@ -110,9 +109,7 @@ APRInitializer::~APRInitializer()
 
 void APRInitializer::stopWatchDogs()
 {
-#if APR_HAS_THREADS
        std::unique_lock<std::mutex> lock(m_priv->mutex);
-#endif
 
        while (!m_priv->watchdogs.empty())
        {
@@ -153,18 +150,14 @@ apr_threadkey_t* APRInitializer::getTlsKey()
 void APRInitializer::registerCleanup(FileWatchdog* watchdog)
 {
        APRInitializer& instance(getInstance());
-#if APR_HAS_THREADS
        std::unique_lock<std::mutex> lock(instance.m_priv->mutex);
-#endif
        instance.m_priv->watchdogs.push_back(watchdog);
 }
 
 void APRInitializer::unregisterCleanup(FileWatchdog* watchdog)
 {
        APRInitializer& instance(getInstance());
-#if APR_HAS_THREADS
        std::unique_lock<std::mutex> lock(instance.m_priv->mutex);
-#endif
 
        for (std::list<FileWatchdog*>::iterator iter = 
instance.m_priv->watchdogs.begin();
                iter != instance.m_priv->watchdogs.end();
@@ -180,17 +173,13 @@ void APRInitializer::unregisterCleanup(FileWatchdog* 
watchdog)
 
 void APRInitializer::addObject(size_t key, const ObjectPtr& pObject)
 {
-#if APR_HAS_THREADS
        std::unique_lock<std::mutex> lock(m_priv->mutex);
-#endif
        m_priv->objects[key] = pObject;
 }
 
 const ObjectPtr& APRInitializer::findOrAddObject(size_t key, 
std::function<ObjectPtr()> creator)
 {
-#if APR_HAS_THREADS
        std::unique_lock<std::mutex> lock(m_priv->mutex);
-#endif
        auto pItem = m_priv->objects.find(key);
        if (m_priv->objects.end() == pItem)
                pItem = m_priv->objects.emplace(key, creator()).first;
diff --git a/src/main/cpp/class.cpp b/src/main/cpp/class.cpp
index 18a5c4b2..ced24e34 100644
--- a/src/main/cpp/class.cpp
+++ b/src/main/cpp/class.cpp
@@ -171,9 +171,7 @@ bool Class::registerClass(const Class& newClass)
 
 void Class::registerClasses()
 {
-#if APR_HAS_THREADS
        AsyncAppender::registerClass();
-#endif
        ConsoleAppender::registerClass();
        FileAppender::registerClass();
        LOG4CXX_NS::db::ODBCAppender::registerClass();
diff --git a/src/main/cpp/domconfigurator.cpp b/src/main/cpp/domconfigurator.cpp
index f7bc5067..186100c9 100644
--- a/src/main/cpp/domconfigurator.cpp
+++ b/src/main/cpp/domconfigurator.cpp
@@ -65,7 +65,6 @@ struct DOMConfigurator::DOMConfiguratorPrivate
        spi::LoggerFactoryPtr loggerFactory;
 };
 
-#if APR_HAS_THREADS
 namespace LOG4CXX_NS
 {
 namespace xml
@@ -90,7 +89,6 @@ class XMLWatchdog  : public FileWatchdog
 }
 }
 XMLWatchdog* DOMConfigurator::xdog = NULL;
-#endif
 
 
 IMPLEMENT_LOG4CXX_OBJECT(DOMConfigurator)
@@ -911,8 +909,6 @@ spi::ConfigurationStatus 
DOMConfigurator::configureAndWatch(const CFStringRef& f
 spi::ConfigurationStatus DOMConfigurator::configureAndWatch(const std::string& 
filename, long delay)
 {
        File file(filename);
-#if APR_HAS_THREADS
-
        if ( xdog )
        {
                APRInitializer::unregisterCleanup(xdog);
@@ -927,17 +923,12 @@ spi::ConfigurationStatus 
DOMConfigurator::configureAndWatch(const std::string& f
        xdog->start();
 
        return status;
-#else
-       return DOMConfigurator().doConfigure(file, 
LogManager::getLoggerRepository());
-#endif
 }
 
 #if LOG4CXX_WCHAR_T_API
 spi::ConfigurationStatus DOMConfigurator::configureAndWatch(const 
std::wstring& filename, long delay)
 {
        File file(filename);
-#if APR_HAS_THREADS
-
        if ( xdog )
        {
                APRInitializer::unregisterCleanup(xdog);
@@ -952,9 +943,6 @@ spi::ConfigurationStatus 
DOMConfigurator::configureAndWatch(const std::wstring&
        xdog->start();
 
        return status;
-#else
-       return DOMConfigurator().doConfigure(file, 
LogManager::getLoggerRepository());
-#endif
 }
 #endif
 
@@ -962,8 +950,6 @@ spi::ConfigurationStatus 
DOMConfigurator::configureAndWatch(const std::wstring&
 spi::ConfigurationStatus DOMConfigurator::configureAndWatch(const 
std::basic_string<UniChar>& filename, long delay)
 {
        File file(filename);
-#if APR_HAS_THREADS
-
        if ( xdog )
        {
                APRInitializer::unregisterCleanup(xdog);
@@ -978,9 +964,6 @@ spi::ConfigurationStatus 
DOMConfigurator::configureAndWatch(const std::basic_str
        xdog->start();
 
        return status;
-#else
-       return DOMConfigurator().doConfigure(file, 
LogManager::getLoggerRepository());
-#endif
 }
 #endif
 
@@ -988,8 +971,6 @@ spi::ConfigurationStatus 
DOMConfigurator::configureAndWatch(const std::basic_str
 spi::ConfigurationStatus DOMConfigurator::configureAndWatch(const CFStringRef& 
filename, long delay)
 {
        File file(filename);
-#if APR_HAS_THREADS
-
        if ( xdog )
        {
                APRInitializer::unregisterCleanup(xdog);
@@ -1004,9 +985,6 @@ spi::ConfigurationStatus 
DOMConfigurator::configureAndWatch(const CFStringRef& f
        xdog->start();
 
        return status;
-#else
-       return DOMConfigurator().doConfigure(file, 
LogManager::getLoggerRepository());
-#endif
 }
 #endif
 
diff --git a/src/main/cpp/loggingevent.cpp b/src/main/cpp/loggingevent.cpp
index 0f9e333d..129d196f 100644
--- a/src/main/cpp/loggingevent.cpp
+++ b/src/main/cpp/loggingevent.cpp
@@ -342,9 +342,9 @@ const LogString& LoggingEvent::getCurrentThreadName()
 #if defined(_WIN32)
        using ThreadIdType = DWORD;
        ThreadIdType threadId = GetCurrentThreadId();
-#elif APR_HAS_THREADS
-       using ThreadIdType = apr_os_thread_t;
-       ThreadIdType threadId = apr_os_thread_current();
+#elif LOG4CXX_HAS_PTHREAD_SELF
+       using ThreadIdType = pthread_t;
+       ThreadIdType threadId = pthread_self();
 #else
        using ThreadIdType = int;
        ThreadIdType threadId = 0;
@@ -368,22 +368,19 @@ const LogString& LoggingEvent::getCurrentThreadName()
                return thread_id_string;
        }
 
-#if APR_HAS_THREADS
 #if defined(_WIN32)
        char result[20];
        apr_snprintf(result, sizeof(result), LOG4CXX_WIN32_THREAD_FMTSPEC, 
threadId);
-#else
-       // apr_os_thread_t encoded in HEX takes needs as many characters
+       Transcoder::decode(reinterpret_cast<const char*>(result), 
thread_id_string);
+#elif LOG4CXX_HAS_PTHREAD_SELF
+       // pthread_t encoded in HEX takes needs as many characters
        // as two times the size of the type, plus an additional null byte.
-       char result[sizeof(apr_os_thread_t) * 3 + 10];
+       char result[sizeof(pthread_t) * 3 + 10];
        apr_snprintf(result, sizeof(result), LOG4CXX_APR_THREAD_FMTSPEC, 
(void*) &threadId);
-#endif /* _WIN32 */
-
-       LOG4CXX_NS::helpers::Transcoder::decode(reinterpret_cast<const 
char*>(result), thread_id_string);
-
+       Transcoder::decode(reinterpret_cast<const char*>(result), 
thread_id_string);
 #else
-    thread_id_string = LOG4CXX_STR("0x00000000");
-#endif /* APR_HAS_THREADS */
+       thread_id_string = LOG4CXX_STR("0x00000000");
+#endif
        return thread_id_string;
 }
 
diff --git a/src/main/cpp/optionconverter.cpp b/src/main/cpp/optionconverter.cpp
index 7adda4f6..54290730 100644
--- a/src/main/cpp/optionconverter.cpp
+++ b/src/main/cpp/optionconverter.cpp
@@ -42,9 +42,8 @@
        #define LOG4CXX 1
 #endif
 #include <log4cxx/helpers/aprinitializer.h>
-
-#if APR_HAS_THREADS
 #include <log4cxx/helpers/filewatchdog.h>
+
 namespace LOG4CXX_NS
 {
 
@@ -70,7 +69,6 @@ class ConfiguratorWatchdog  : public helpers::FileWatchdog
 };
 
 }
-#endif
 
 using namespace LOG4CXX_NS;
 using namespace LOG4CXX_NS::helpers;
@@ -445,7 +443,6 @@ void OptionConverter::selectAndConfigure(const File& 
configFileName,
                configurator = std::make_shared<PropertyConfigurator>();
        }
 
-#if APR_HAS_THREADS
        if (0 < delay)
        {
                auto dog = new ConfiguratorWatchdog(configurator, 
configFileName);
@@ -454,6 +451,5 @@ void OptionConverter::selectAndConfigure(const File& 
configFileName,
                dog->start();
        }
        else
-#endif
                configurator->doConfigure(configFileName, hierarchy);
 }
diff --git a/src/main/cpp/propertyconfigurator.cpp 
b/src/main/cpp/propertyconfigurator.cpp
index 7a873a92..2c54b4b8 100644
--- a/src/main/cpp/propertyconfigurator.cpp
+++ b/src/main/cpp/propertyconfigurator.cpp
@@ -49,7 +49,6 @@ using namespace LOG4CXX_NS::helpers;
 using namespace LOG4CXX_NS::config;
 using namespace LOG4CXX_NS::rolling;
 
-#if APR_HAS_THREADS
 #include <log4cxx/helpers/filewatchdog.h>
 namespace LOG4CXX_NS
 {
@@ -75,8 +74,6 @@ class PropertyWatchdog  : public FileWatchdog
 
 PropertyWatchdog* PropertyConfigurator::pdog = NULL;
 
-#endif
-
 IMPLEMENT_LOG4CXX_OBJECT(PropertyConfigurator)
 
 PropertyConfigurator::PropertyConfigurator()
@@ -135,7 +132,6 @@ spi::ConfigurationStatus 
PropertyConfigurator::configure(helpers::Properties& pr
        return PropertyConfigurator().doConfigure(properties, 
LogManager::getLoggerRepository());
 }
 
-#if APR_HAS_THREADS
 spi::ConfigurationStatus PropertyConfigurator::configureAndWatch(const File& 
configFilename)
 {
        return configureAndWatch(configFilename, FileWatchdog::DEFAULT_DELAY);
@@ -161,7 +157,6 @@ spi::ConfigurationStatus 
PropertyConfigurator::configureAndWatch(
 
        return stat;
 }
-#endif
 
 spi::ConfigurationStatus 
PropertyConfigurator::doConfigure(helpers::Properties& properties,
        spi::LoggerRepositoryPtr hierarchy)
diff --git a/src/main/include/CMakeLists.txt b/src/main/include/CMakeLists.txt
index 4e0603db..ff98ca27 100644
--- a/src/main/include/CMakeLists.txt
+++ b/src/main/include/CMakeLists.txt
@@ -136,6 +136,7 @@ CHECK_SYMBOL_EXISTS(syslog "syslog.h" HAS_SYSLOG)
 if(UNIX)
     set(CMAKE_REQUIRED_LIBRARIES "pthread")
     CHECK_SYMBOL_EXISTS(pthread_sigmask "signal.h" HAS_PTHREAD_SIGMASK)
+    CHECK_SYMBOL_EXISTS(pthread_self "pthread.h" HAS_PTHREAD_SELF)
 
     # Check for the (linux) pthread_setname_np.
     # OSX and BSD are special apparently.  OSX only lets you name
@@ -160,6 +161,7 @@ foreach(varName
   HAS_FWIDE
   HAS_LIBESMTP
   HAS_SYSLOG
+  HAS_PTHREAD_SELF
   HAS_PTHREAD_SIGMASK
   HAS_PTHREAD_SETNAME
   HAS_PTHREAD_GETNAME
diff --git a/src/main/include/log4cxx/private/log4cxx_private.h.in 
b/src/main/include/log4cxx/private/log4cxx_private.h.in
index 61e8e7ac..b7bed0be 100644
--- a/src/main/include/log4cxx/private/log4cxx_private.h.in
+++ b/src/main/include/log4cxx/private/log4cxx_private.h.in
@@ -57,6 +57,7 @@
 #define LOG4CXX_WIN32_THREAD_FMTSPEC "0x%.8x"
 #define LOG4CXX_APR_THREAD_FMTSPEC "0x%pt"
 
+#define LOG4CXX_HAS_PTHREAD_SELF @HAS_PTHREAD_SELF@
 #define LOG4CXX_HAS_PTHREAD_SIGMASK @HAS_PTHREAD_SIGMASK@
 #define LOG4CXX_HAS_PTHREAD_SETNAME @HAS_PTHREAD_SETNAME@
 #define LOG4CXX_HAS_PTHREAD_GETNAME @HAS_PTHREAD_GETNAME@
diff --git a/src/test/cpp/asyncappendertestcase.cpp 
b/src/test/cpp/asyncappendertestcase.cpp
index dedbded9..e7086dc7 100644
--- a/src/test/cpp/asyncappendertestcase.cpp
+++ b/src/test/cpp/asyncappendertestcase.cpp
@@ -108,7 +108,6 @@ class BlockableVectorAppender : public VectorAppender
 
 LOG4CXX_PTR_DEF(BlockableVectorAppender);
 
-#if APR_HAS_THREADS
 /**
  * Tests of AsyncAppender.
  */
@@ -324,4 +323,3 @@ class AsyncAppenderTestCase : public 
AppenderSkeletonTestCase
 };
 
 LOGUNIT_TEST_SUITE_REGISTRATION(AsyncAppenderTestCase);
-#endif
diff --git a/src/test/cpp/helpers/charsetencodertestcase.cpp 
b/src/test/cpp/helpers/charsetencodertestcase.cpp
index b4ca980e..65bac6a7 100644
--- a/src/test/cpp/helpers/charsetencodertestcase.cpp
+++ b/src/test/cpp/helpers/charsetencodertestcase.cpp
@@ -38,9 +38,7 @@ LOGUNIT_CLASS(CharsetEncoderTestCase)
        LOGUNIT_TEST(encode3);
        LOGUNIT_TEST(encode4);
        LOGUNIT_TEST(encode5);
-#if APR_HAS_THREADS
        LOGUNIT_TEST(thread1);
-#endif
        LOGUNIT_TEST_SUITE_END();
 
        enum { BUFSIZE = 256 };
@@ -237,7 +235,6 @@ public:
                }
        }
 
-#if APR_HAS_THREADS
        class ThreadPackage
        {
                public:
@@ -392,7 +389,6 @@ public:
                LOGUNIT_ASSERT_EQUAL((apr_uint32_t) THREAD_COUNT * THREAD_REPS, 
package->getPass());
                delete package;
        }
-#endif
 
 };
 
diff --git a/src/test/cpp/helpers/threadutilitytestcase.cpp 
b/src/test/cpp/helpers/threadutilitytestcase.cpp
index effe1eda..1304f896 100644
--- a/src/test/cpp/helpers/threadutilitytestcase.cpp
+++ b/src/test/cpp/helpers/threadutilitytestcase.cpp
@@ -63,6 +63,7 @@ public:
                auto thrUtil = ThreadUtility::instance();
                int num_pre = 0;
                int num_started = 0;
+               int num_run = 0;
                int num_post = 0;
 
                thrUtil->configureFuncs(
@@ -82,12 +83,16 @@ public:
                }
                );
 
-               std::thread t = thrUtil->createThread( LOG4CXX_STR("FooName"), 
[]() {} );
+               std::thread t = thrUtil->createThread( LOG4CXX_STR("FooName"), 
[&num_run]()
+               {
+                       num_run++;
+               } );
 
                t.join();
 
                LOGUNIT_ASSERT_EQUAL( num_pre, 1 );
                LOGUNIT_ASSERT_EQUAL( num_started, 1 );
+               LOGUNIT_ASSERT_EQUAL( num_run, 1 );
                LOGUNIT_ASSERT_EQUAL( num_post, 1 );
        }
 
diff --git a/src/test/cpp/net/socketappendertestcase.cpp 
b/src/test/cpp/net/socketappendertestcase.cpp
index 0a462cb6..2271930f 100644
--- a/src/test/cpp/net/socketappendertestcase.cpp
+++ b/src/test/cpp/net/socketappendertestcase.cpp
@@ -21,7 +21,6 @@
 using namespace log4cxx;
 using namespace log4cxx::helpers;
 
-#if APR_HAS_THREADS
 /**
    Unit tests of log4cxx::SocketAppender
  */
@@ -92,4 +91,3 @@ class SocketAppenderTestCase : public AppenderSkeletonTestCase
 };
 
 LOGUNIT_TEST_SUITE_REGISTRATION(SocketAppenderTestCase);
-#endif
diff --git a/src/test/cpp/net/telnetappendertestcase.cpp 
b/src/test/cpp/net/telnetappendertestcase.cpp
index 7482f959..41c88798 100644
--- a/src/test/cpp/net/telnetappendertestcase.cpp
+++ b/src/test/cpp/net/telnetappendertestcase.cpp
@@ -25,7 +25,6 @@ using namespace log4cxx;
 using namespace log4cxx::helpers;
 using namespace log4cxx::net;
 
-#if APR_HAS_THREADS
 /**
    Unit tests of log4cxx::TelnetAppender
  */
@@ -101,4 +100,3 @@ class TelnetAppenderTestCase : public 
AppenderSkeletonTestCase
 };
 
 LOGUNIT_TEST_SUITE_REGISTRATION(TelnetAppenderTestCase);
-#endif
diff --git a/src/test/cpp/net/xmlsocketappendertestcase.cpp 
b/src/test/cpp/net/xmlsocketappendertestcase.cpp
index 79445bec..cfd715b8 100644
--- a/src/test/cpp/net/xmlsocketappendertestcase.cpp
+++ b/src/test/cpp/net/xmlsocketappendertestcase.cpp
@@ -23,7 +23,6 @@
 using namespace log4cxx;
 using namespace log4cxx::helpers;
 
-#if APR_HAS_THREADS
 /**
    Unit tests of log4cxx::net::XMLSocketAppender
  */
@@ -60,4 +59,4 @@ class XMLSocketAppenderTestCase : public 
AppenderSkeletonTestCase
 };
 
 LOGUNIT_TEST_SUITE_REGISTRATION(XMLSocketAppenderTestCase);
-#endif
+

Reply via email to