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 f02e9185 Ensure a change to the shared ThreadUtility isActive flag is
visible (#694)
f02e9185 is described below
commit f02e918533259b4be514ac0ead61fa0feb461d06
Author: Stephen Webb <[email protected]>
AuthorDate: Tue Jun 2 11:10:52 2026 +1000
Ensure a change to the shared ThreadUtility isActive flag is visible (#694)
* Reduce flakiness of tests that exit without calling LogManager::shutdown
* Pass LOG4CXX_DEBUG environment var to child test processes
---
.github/workflows/log4cxx-macos.yml | 5 +++++
src/main/cpp/threadutility.cpp | 28 ++++++++++++++++--------
src/test/cpp/CMakeLists.txt | 5 ++++-
src/test/cpp/fileappendertest.cpp | 5 ++++-
src/test/cpp/helpers/filewatchdogtest.cpp | 11 ++++++++++
src/test/cpp/helpers/threadutilitytestcase.cpp | 6 +++++
src/test/cpp/net/telnetappendertestcase.cpp | 2 +-
src/test/cpp/rolling/multiprocessrollingtest.cpp | 2 +-
8 files changed, 51 insertions(+), 13 deletions(-)
diff --git a/.github/workflows/log4cxx-macos.yml
b/.github/workflows/log4cxx-macos.yml
index 79f426a5..02918093 100644
--- a/.github/workflows/log4cxx-macos.yml
+++ b/.github/workflows/log4cxx-macos.yml
@@ -34,6 +34,7 @@ jobs:
multiprocess: ON
cfstring: OFF
next_abi: OFF
+ exitevents: OFF
- name: osx-15-clang
os: macos-15
cxx: clang++
@@ -42,6 +43,7 @@ jobs:
multiprocess: OFF
cfstring: ON
next_abi: OFF
+ exitevents: OFF
- name: osx-26-clang
os: macos-26
cxx: clang++
@@ -50,6 +52,7 @@ jobs:
multiprocess: OFF
cfstring: ON
next_abi: ON
+ exitevents: ON
- name: osx-g++
os: macos-latest
cxx: g++-14
@@ -58,6 +61,7 @@ jobs:
multiprocess: OFF
cfstring: ON
next_abi: OFF
+ exitevents: OFF
steps:
- uses: actions/checkout@v4
@@ -87,6 +91,7 @@ jobs:
-DLOG4CXX_ENABLE_ODBC=${{ matrix.odbc }} \
-DLOG4CXX_MULTIPROCESS_ROLLING_FILE_APPENDER=${{ matrix.multiprocess
}} \
-DLOG4CXX_CFSTRING=${{ matrix.cfstring }} \
+ -DLOG4CXX_EVENTS_AT_EXIT=${{ matrix.exitevents }} \
-DLOG4CXX_BUILD_NEXT_ABI=${{ matrix.next_abi }} \
-DCMAKE_BUILD_TYPE=Debug \
..
diff --git a/src/main/cpp/threadutility.cpp b/src/main/cpp/threadutility.cpp
index 0ffcc140..cfe07507 100644
--- a/src/main/cpp/threadutility.cpp
+++ b/src/main/cpp/threadutility.cpp
@@ -83,18 +83,20 @@ struct ThreadUtility::priv_data
std::atomic<bool> terminated{ false };
int retryCount{ 2 };
Period maxDelay{ 0 };
- bool threadIsActive{ false };
+ std::atomic<bool> threadIsActive{ false };
+ LoggerPtr log;
void doPeriodicTasks();
void setTerminated()
{
- std::lock_guard<std::mutex> lock(interrupt_mutex);
+ std::lock_guard<std::recursive_mutex> lock(job_mutex);
terminated.store(true);
}
void stopThread()
{
+ LOGLOG_DEBUG(log, "stopThread");
setTerminated();
interrupt.notify_all();
if (thread.joinable())
@@ -267,6 +269,9 @@ ThreadStartPost ThreadUtility::postStartFunction()
*/
void ThreadUtility::addPeriodicTask(const LogString& name,
std::function<void()> f, const Period& delay)
{
+ if (!m_priv->log)
+ m_priv->log = Logger::getLogger("ThreadUtility");
+ LOGLOG_DEBUG(m_priv->log, LOG4CXX_STR("addPeriodicTask: ") << name);
std::lock_guard<std::recursive_mutex> lock(m_priv->job_mutex);
if (m_priv->maxDelay < delay)
m_priv->maxDelay = delay;
@@ -274,14 +279,20 @@ void ThreadUtility::addPeriodicTask(const LogString&
name, std::function<void()>
m_priv->jobs.push_back( priv_data::NamedPeriodicFunction{name, delay,
currentTime + delay, f, 0, false} );
// Restart thread if it has stopped.
- if (!m_priv->threadIsActive && m_priv->thread.joinable())
+ if (!m_priv->threadIsActive.load() && m_priv->thread.joinable())
m_priv->thread.join();
if (!m_priv->thread.joinable())
{
m_priv->terminated.store(false);
- m_priv->threadIsActive = true;
- m_priv->thread = createThread(LOG4CXX_STR("log4cxx"),
std::bind(&priv_data::doPeriodicTasks, m_priv.get()));
+ m_priv->threadIsActive.store(true);
+ m_priv->thread = createThread(LOG4CXX_STR("log4cxx"), [this]()
+ {
+ LOGLOG_DEBUG(m_priv->log, "doPeriodicTasks: "
<< "started");
+ m_priv->doPeriodicTasks();
+ LOGLOG_DEBUG(m_priv->log, "doPeriodicTasks: "
<< "stopped");
+ m_priv->threadIsActive.store(false);
+ });
}
else
m_priv->interrupt.notify_one();
@@ -305,6 +316,7 @@ bool ThreadUtility::hasPeriodicTask(const LogString& name)
*/
void ThreadUtility::removeAllPeriodicTasks()
{
+ LOGLOG_DEBUG(m_priv->log, "removeAllPeriodicTasks");
{
std::lock_guard<std::recursive_mutex> lock(m_priv->job_mutex);
while (!m_priv->jobs.empty())
@@ -325,6 +337,7 @@ void ThreadUtility::removePeriodicTask(const LogString&
name)
);
if (m_priv->jobs.end() != pItem)
{
+ LOGLOG_DEBUG(m_priv->log, LOG4CXX_STR("removePeriodicTask: ")
<< name);
pItem->removed = true;
m_priv->interrupt.notify_one();
}
@@ -399,18 +412,15 @@ void ThreadUtility::priv_data::doPeriodicTasks()
);
if (this->jobs.end() == pItem)
break;
+ LOGLOG_DEBUG(this->log, LOG4CXX_STR("doPeriodicTasks:
erase ") << pItem->name);
this->jobs.erase(pItem);
if (this->jobs.empty())
- {
- this->threadIsActive = false;
return;
- }
}
std::unique_lock<std::mutex> lock(this->interrupt_mutex);
this->interrupt.wait_until(lock, nextOperationTime);
}
- this->threadIsActive = false;
}
} //namespace helpers
diff --git a/src/test/cpp/CMakeLists.txt b/src/test/cpp/CMakeLists.txt
index f20a84f8..28ebd7cf 100644
--- a/src/test/cpp/CMakeLists.txt
+++ b/src/test/cpp/CMakeLists.txt
@@ -124,7 +124,10 @@ else()
set(TEST_COMPILE_DEFINITIONS _DEBUG)
endif()
if(LOG4CXX_TEST_ONLY_BUILD)
- list(APPEND TEST_COMPILE_DEFINITIONS
"ENABLE_FAILING_APPENDER_SIMULATION_TESTING=1")
+ list(APPEND TEST_COMPILE_DEFINITIONS
+ "ENABLE_FAILING_APPENDER_SIMULATION_TESTING=1"
+ "LOGLOG_THRESHOLD=1"
+ )
endif()
get_filename_component(UNIT_TEST_WORKING_DIR ../resources ABSOLUTE)
diff --git a/src/test/cpp/fileappendertest.cpp
b/src/test/cpp/fileappendertest.cpp
index 1c874b46..c91a7009 100644
--- a/src/test/cpp/fileappendertest.cpp
+++ b/src/test/cpp/fileappendertest.cpp
@@ -159,6 +159,7 @@ public:
{
int requiredMsgCount = 100;
auto logger = getLogger(LOG4CXX_STR("100message"));
+ LOGLOG_DEBUG(logger, "writeFinalBufferOutput: " <<
"configuing");
// Set up a new file
LogString dir{ LOG4CXX_STR("output/newdir") };
@@ -174,10 +175,12 @@ public:
logger->setAdditivity(false);
logger->addAppender(writer);
+ LOGLOG_DEBUG(logger, "writeFinalBufferOutput: " << "started");
for ( int x = 0; x < requiredMsgCount; x++ )
{
LOG4CXX_INFO( logger, "This is test message " << x );
}
+ LOGLOG_DEBUG(logger, "writeFinalBufferOutput: " << "stopped");
}
void checkFinalBufferOutput()
@@ -247,7 +250,7 @@ private:
{
LOGUNIT_FAIL("apr_procattr_create");
}
- if (apr_procattr_cmdtype_set(*attr, APR_PROGRAM) != APR_SUCCESS)
+ if (apr_procattr_cmdtype_set(*attr, APR_PROGRAM_ENV) !=
APR_SUCCESS)
{
LOGUNIT_FAIL("apr_procattr_cmdtype_set");
}
diff --git a/src/test/cpp/helpers/filewatchdogtest.cpp
b/src/test/cpp/helpers/filewatchdogtest.cpp
index 67cae709..fea3d3c7 100644
--- a/src/test/cpp/helpers/filewatchdogtest.cpp
+++ b/src/test/cpp/helpers/filewatchdogtest.cpp
@@ -16,6 +16,7 @@
*/
#include <log4cxx/helpers/pool.h>
#include <log4cxx/helpers/filewatchdog.h>
+#include <log4cxx/helpers/loglog.h>
#include "../logunit.h"
#include "apr_time.h"
@@ -32,6 +33,14 @@ LOGUNIT_CLASS(FileWatchdogTest)
LOGUNIT_TEST_SUITE(FileWatchdogTest);
LOGUNIT_TEST(testShutdownDelay);
LOGUNIT_TEST_SUITE_END();
+#ifdef _DEBUG
+ struct Fixture
+ {
+ Fixture() {
+ LogLog::setInternalDebugging(true);
+ }
+ } suiteFixture;
+#endif
private:
class MockWatchdog : public FileWatchdog
@@ -63,6 +72,8 @@ public:
}
apr_time_t delta = apr_time_now() - start;
LOGUNIT_ASSERT(delta < 30000000);
+ // wait 50 ms for periodic task thread to exit
+ apr_sleep(50000);
}
};
diff --git a/src/test/cpp/helpers/threadutilitytestcase.cpp
b/src/test/cpp/helpers/threadutilitytestcase.cpp
index 8c10fecb..d89d2a3b 100644
--- a/src/test/cpp/helpers/threadutilitytestcase.cpp
+++ b/src/test/cpp/helpers/threadutilitytestcase.cpp
@@ -22,6 +22,10 @@
#include <log4cxx/patternlayout.h>
#include <log4cxx/fileappender.h>
#include <log4cxx/logmanager.h>
+#if !defined(LOG4CXX)
+ #define LOG4CXX 1
+#endif
+#include <log4cxx/private/log4cxx_private.h>
#include <atomic>
#include <chrono>
@@ -147,6 +151,8 @@ public:
}
thrUtil->removePeriodicTask(secondTask);
LOGUNIT_ASSERT(0 < secondRuns.load());
+ // wait 30 ms for periodic task thread to exit
+ std::this_thread::sleep_for(std::chrono::milliseconds(30));
}
void testThreadNameLogging()
diff --git a/src/test/cpp/net/telnetappendertestcase.cpp
b/src/test/cpp/net/telnetappendertestcase.cpp
index 270465df..52f70464 100644
--- a/src/test/cpp/net/telnetappendertestcase.cpp
+++ b/src/test/cpp/net/telnetappendertestcase.cpp
@@ -189,7 +189,7 @@ private:
{
LOGUNIT_FAIL("apr_procattr_create");
}
- if (apr_procattr_cmdtype_set(*attr, APR_PROGRAM) != APR_SUCCESS)
+ if (apr_procattr_cmdtype_set(*attr, APR_PROGRAM_ENV) !=
APR_SUCCESS)
{
LOGUNIT_FAIL("apr_procattr_cmdtype_set");
}
diff --git a/src/test/cpp/rolling/multiprocessrollingtest.cpp
b/src/test/cpp/rolling/multiprocessrollingtest.cpp
index 98e754d8..e1afa777 100644
--- a/src/test/cpp/rolling/multiprocessrollingtest.cpp
+++ b/src/test/cpp/rolling/multiprocessrollingtest.cpp
@@ -329,7 +329,7 @@ private:
{
LOGUNIT_FAIL("apr_procattr_create");
}
- if (apr_procattr_cmdtype_set(*attr, APR_PROGRAM) != APR_SUCCESS)
+ if (apr_procattr_cmdtype_set(*attr, APR_PROGRAM_ENV) !=
APR_SUCCESS)
{
LOGUNIT_FAIL("apr_procattr_cmdtype_set");
}
