This is an automated email from the ASF dual-hosted git repository.
swebb2066 pushed a commit to branch next_stable
in repository https://gitbox.apache.org/repos/asf/logging-log4cxx.git
The following commit(s) were added to refs/heads/next_stable by this push:
new 7fedcdb Allow automatic configuration to be used with a
multi-threaded application
7fedcdb is described below
commit 7fedcdbfd82fb87044d1f24324176652e6c0e815
Author: Stephen Webb <swebb2066.com>
AuthorDate: Sat Jan 1 16:01:39 2022 +1100
Allow automatic configuration to be used with a multi-threaded application
Add a unit test to check automatic configuration works
Serialize initialization of the single RepositorySelector object.
---
src/main/cpp/aprinitializer.cpp | 18 +++++
src/main/cpp/hierarchy.cpp | 38 ++++-------
src/main/cpp/logmanager.cpp | 27 +++-----
src/main/include/log4cxx/helpers/aprinitializer.h | 35 ++++++++--
src/main/include/log4cxx/hierarchy.h | 2 -
src/main/include/log4cxx/logmanager.h | 1 -
src/test/cpp/CMakeLists.txt | 12 +++-
src/test/cpp/autoconfiguretestcase.cpp | 78 ++++++++++++++++++++++
.../resources/input/autoConfigureTest.properties | 25 +++++++
9 files changed, 184 insertions(+), 52 deletions(-)
diff --git a/src/main/cpp/aprinitializer.cpp b/src/main/cpp/aprinitializer.cpp
index 0bd91d2..8831412 100644
--- a/src/main/cpp/aprinitializer.cpp
+++ b/src/main/cpp/aprinitializer.cpp
@@ -127,3 +127,21 @@ void APRInitializer::unregisterCleanup(FileWatchdog*
watchdog)
}
}
+void APRInitializer::addObject(size_t key, const ObjectPtr& pObject)
+{
+#if APR_HAS_THREADS
+ std::unique_lock<std::mutex> lock(this->mutex);
+#endif
+ this->objects[key] = pObject;
+}
+
+const ObjectPtr& APRInitializer::findOrAddObject(size_t key,
std::function<ObjectPtr()> creator)
+{
+#if APR_HAS_THREADS
+ std::unique_lock<std::mutex> lock(this->mutex);
+#endif
+ auto pItem = this->objects.find(key);
+ if (this->objects.end() == pItem)
+ pItem = this->objects.emplace(key, creator()).first;
+ return pItem->second;
+}
diff --git a/src/main/cpp/hierarchy.cpp b/src/main/cpp/hierarchy.cpp
index 807f94e..3d79a23 100644
--- a/src/main/cpp/hierarchy.cpp
+++ b/src/main/cpp/hierarchy.cpp
@@ -67,6 +67,7 @@ struct Hierarchy::HierarchyPrivate
log4cxx::helpers::Pool pool;
mutable std::mutex mutex;
bool configured;
+ mutable std::mutex configuredMutex;
spi::LoggerFactoryPtr defaultFactory;
spi::HierarchyEventListenerList listeners;
@@ -292,17 +293,15 @@ LoggerPtr Hierarchy::getRootLogger() const
bool Hierarchy::isDisabled(int level) const
{
- bool currentlyConfigured;
+ if (!m_priv->configured) // auto-configuration required?
{
- std::unique_lock<std::mutex> lock(m_priv->mutex);
- currentlyConfigured = m_priv->configured;
- }
-
- if (!currentlyConfigured)
- {
- std::shared_ptr<Hierarchy> nonconstThis =
std::const_pointer_cast<Hierarchy>(shared_from_this());
- DefaultConfigurator::configure(
- nonconstThis);
+ std::unique_lock<std::mutex> lock(m_priv->configuredMutex);
+ if (!m_priv->configured)
+ {
+ std::shared_ptr<Hierarchy> nonconstThis =
std::const_pointer_cast<Hierarchy>(shared_from_this());
+ DefaultConfigurator::configure(nonconstThis);
+ m_priv->configured = true;
+ }
}
return m_priv->thresholdInt > level;
@@ -432,29 +431,20 @@ void Hierarchy::updateChildren(ProvisionNode& pn,
LoggerPtr logger)
void Hierarchy::setConfigured(bool newValue)
{
- std::unique_lock<std::mutex> lock(m_priv->mutex);
- m_priv->configured = newValue;
+ std::unique_lock<std::mutex> lock(m_priv->configuredMutex,
std::try_to_lock);
+ if (lock.owns_lock()) // Not being auto-configured?
+ m_priv->configured = newValue;
}
bool Hierarchy::isConfigured()
{
+ std::unique_lock<std::mutex> lock(m_priv->configuredMutex); // Blocks
while auto-configuration is active
return m_priv->configured;
}
HierarchyPtr Hierarchy::create()
{
HierarchyPtr ret( new Hierarchy() );
- ret->configureRoot();
+ ret->m_priv->root->setHierarchy(ret);
return ret;
}
-
-void Hierarchy::configureRoot()
-{
- // This should really be done in the constructor, but in order to fix
- // LOGCXX-322 we need to turn the repositroy into a weak_ptr, and we
- // can't use weak_from_this() in the constructor.
- if ( !m_priv->root->getLoggerRepository().lock() )
- {
- m_priv->root->setHierarchy(shared_from_this());
- }
-}
diff --git a/src/main/cpp/logmanager.cpp b/src/main/cpp/logmanager.cpp
index c7b7d8e..e060ea8 100644
--- a/src/main/cpp/logmanager.cpp
+++ b/src/main/cpp/logmanager.cpp
@@ -47,28 +47,19 @@ using namespace log4cxx::helpers;
IMPLEMENT_LOG4CXX_OBJECT(DefaultRepositorySelector)
void* LogManager::guard = 0;
-spi::RepositorySelectorPtr LogManager::repositorySelector;
-
RepositorySelectorPtr LogManager::getRepositorySelector()
{
- //
- // call to initialize APR and trigger "start" of logging clock
- //
- APRInitializer::initialize();
-
- if (!repositorySelector)
- {
- LoggerRepositoryPtr hierarchy = Hierarchy::create();
- RepositorySelectorPtr selector(new
DefaultRepositorySelector(hierarchy));
- repositorySelector = selector;
- }
-
- return repositorySelector;
+ auto result = APRInitializer::getUnique<spi::RepositorySelector>( []()
-> ObjectPtr
+ {
+ LoggerRepositoryPtr hierarchy = Hierarchy::create();
+ return ObjectPtr(new
DefaultRepositorySelector(hierarchy));
+ }
+ );
+ return result;
}
-void LogManager::setRepositorySelector(spi::RepositorySelectorPtr selector,
- void* guard1)
+void LogManager::setRepositorySelector(spi::RepositorySelectorPtr selector,
void* guard1)
{
if ((LogManager::guard != 0) && (LogManager::guard != guard1))
{
@@ -81,7 +72,7 @@ void
LogManager::setRepositorySelector(spi::RepositorySelectorPtr selector,
}
LogManager::guard = guard1;
- LogManager::getRepositorySelector() = selector;
+ APRInitializer::setUnique<spi::RepositorySelector>(selector);
}
diff --git a/src/main/include/log4cxx/helpers/aprinitializer.h
b/src/main/include/log4cxx/helpers/aprinitializer.h
index fccd18e..38ea5c6 100644
--- a/src/main/include/log4cxx/helpers/aprinitializer.h
+++ b/src/main/include/log4cxx/helpers/aprinitializer.h
@@ -22,6 +22,7 @@
#error "aprinitializer.h should only be included by log4cxx
implementation"
#endif
+#include <log4cxx/helpers/object.h>
#include <list>
extern "C" {
@@ -31,6 +32,7 @@ extern "C" {
#include <apr_time.h>
#include <mutex>
+#include <functional>
namespace log4cxx
{
@@ -47,25 +49,46 @@ class APRInitializer
static bool isDestructed;
/**
- * Register a FileWatchdog for deletion prior to
- * APR termination. FileWatchdog must be
+ * Register a FileWatchdog for deletion prior to termination.
+ * FileWatchdog must be
* allocated on heap and not deleted elsewhere.
*/
static void registerCleanup(FileWatchdog* watchdog);
static void unregisterCleanup(FileWatchdog* watchdog);
+ /**
+ * Store a single instance type ObjectPtr for deletion prior
to termination
+ */
+ template <class T> static void setUnique(const
std::shared_ptr<T>& pObject)
+ {
+ getInstance().addObject(typeid(T).hash_code(), pObject);
+ }
+ /**
+ * Fetch or add a single instance type ObjectPtr for deletion
prior to termination
+ */
+ template <class T> static std::shared_ptr<T>
getUnique(std::function<ObjectPtr()> creator)
+ {
+ return
cast<T>(getInstance().findOrAddObject(typeid(T).hash_code(), creator));
+ }
+
- private:
+ private: // Constructors
APRInitializer();
- APRInitializer(const APRInitializer&);
- APRInitializer& operator=(const APRInitializer&);
+ APRInitializer(const APRInitializer&) = delete;
+ APRInitializer& operator=(const APRInitializer&) = delete;
+ private: // Modifiers
+ void addObject(size_t key, const ObjectPtr& pObject);
+ const ObjectPtr& findOrAddObject(size_t key,
std::function<ObjectPtr()> creator);
+ private: // Attributes
apr_pool_t* p;
std::mutex mutex;
std::list<FileWatchdog*> watchdogs;
apr_time_t startTime;
apr_threadkey_t* tlsKey;
+ std::map<size_t, ObjectPtr> objects;
+ private: // Class methods
static APRInitializer& getInstance();
- public:
+ public: // Destructor
~APRInitializer();
};
} // namespace helpers
diff --git a/src/main/include/log4cxx/hierarchy.h
b/src/main/include/log4cxx/hierarchy.h
index 0cf6963..72120bd 100644
--- a/src/main/include/log4cxx/hierarchy.h
+++ b/src/main/include/log4cxx/hierarchy.h
@@ -269,8 +269,6 @@ class LOG4CXX_EXPORT Hierarchy :
Hierarchy& operator=(const Hierarchy&);
void updateChildren(ProvisionNode& pn, LoggerPtr logger);
-
- void configureRoot();
};
} //namespace log4cxx
diff --git a/src/main/include/log4cxx/logmanager.h
b/src/main/include/log4cxx/logmanager.h
index 7262861..d347f9a 100644
--- a/src/main/include/log4cxx/logmanager.h
+++ b/src/main/include/log4cxx/logmanager.h
@@ -50,7 +50,6 @@ class LOG4CXX_EXPORT LogManager
{
private:
static void* guard;
- static spi::RepositorySelectorPtr repositorySelector;
static spi::RepositorySelectorPtr getRepositorySelector();
public:
diff --git a/src/test/cpp/CMakeLists.txt b/src/test/cpp/CMakeLists.txt
index eb981b9..eef0e59 100644
--- a/src/test/cpp/CMakeLists.txt
+++ b/src/test/cpp/CMakeLists.txt
@@ -17,6 +17,7 @@ find_program(GZIP_APP gzip REQUIRED)
# Tests defined in this directory
set(ALL_LOG4CXX_TESTS
+ autoconfiguretestcase
asyncappendertestcase
consoleappendertestcase
decodingtest
@@ -86,13 +87,14 @@ if( WIN32 )
endforeach()
endif( WIN32 )
+get_filename_component(UNIT_TEST_WORKING_DIR ../resources ABSOLUTE)
foreach(testName IN LISTS ALL_LOG4CXX_TESTS)
target_compile_definitions(${testName} PRIVATE
${LOG4CXX_COMPILE_DEFINITIONS} ${APR_COMPILE_DEFINITIONS}
${APR_UTIL_COMPILE_DEFINITIONS} )
target_include_directories(${testName} PRIVATE ${CMAKE_CURRENT_LIST_DIR}
$<TARGET_PROPERTY:log4cxx,INCLUDE_DIRECTORIES>)
target_link_libraries(${testName} PRIVATE testingFramework
testingUtilities log4cxx ${APR_LIBRARIES} ${APR_SYSTEM_LIBS} Threads::Threads)
add_test(NAME ${testName}
COMMAND ${testName} -v
- WORKING_DIRECTORY ${CMAKE_CURRENT_LIST_DIR}/../resources
+ WORKING_DIRECTORY ${UNIT_TEST_WORKING_DIR}
)
set_tests_properties( ${testName} PROPERTIES TIMEOUT 120 )
@@ -101,6 +103,10 @@ foreach(testName IN LISTS ALL_LOG4CXX_TESTS)
set_tests_properties(socketservertestcase PROPERTIES
ENVIRONMENT
"SOCKET_SERVER_PARAMETER_FILE=${START_SOCKET_SERVER_PARAMETER_FILE};PATH=${ESCAPED_PATH}"
)
+ elseif(${testName} STREQUAL autoconfiguretestcase)
+ set_tests_properties(autoconfiguretestcase PROPERTIES
+ ENVIRONMENT
"LOG4CXX_CONFIGURATION=${UNIT_TEST_WORKING_DIR}/input/autoConfigureTest.properties;PATH=${ESCAPED_PATH}"
+ )
else()
set_tests_properties(${testName} PROPERTIES
ENVIRONMENT
"TOTO=wonderful;key1=value1;key2=value2;PATH=${ESCAPED_PATH}"
@@ -111,6 +117,10 @@ foreach(testName IN LISTS ALL_LOG4CXX_TESTS)
set_tests_properties(socketservertestcase PROPERTIES
ENVIRONMENT
"SOCKET_SERVER_PARAMETER_FILE=${START_SOCKET_SERVER_PARAMETER_FILE}"
)
+ elseif(${testName} STREQUAL autoconfiguretestcase)
+ set_tests_properties(autoconfiguretestcase PROPERTIES
+ ENVIRONMENT
"LOG4CXX_CONFIGURATION=${UNIT_TEST_WORKING_DIR}/input/autoConfigureTest.properties"
+ )
else()
set_tests_properties(${testName} PROPERTIES
ENVIRONMENT "TOTO=wonderful;key1=value1;key2=value2"
diff --git a/src/test/cpp/autoconfiguretestcase.cpp
b/src/test/cpp/autoconfiguretestcase.cpp
new file mode 100644
index 0000000..4953e2e
--- /dev/null
+++ b/src/test/cpp/autoconfiguretestcase.cpp
@@ -0,0 +1,78 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include "logunit.h"
+#include <log4cxx/logger.h>
+#include <log4cxx/logmanager.h>
+#include <log4cxx/helpers/loglog.h>
+#include <log4cxx/file.h>
+#include <thread>
+
+#define LOGUNIT_TEST_THREADS(testName, threadCount) \
+ class testName ## ThreadTestRegistration { \
+ public: \
+ testName ## ThreadTestRegistration() { \
+ ThisFixture::getSuite()->addTest(#testName,
&testName ## ThreadTestRegistration :: run); \
+ } \
+ static void run(abts_case* tc, void*) { \
+ std::vector<std::thread> threads; \
+ for (auto i = threadCount; 0 < i; --i) \
+ threads.emplace_back( [tc]() { \
+
LogUnit::runTest<ThisFixture>(tc, &ThisFixture::testName); \
+ } ); \
+ while (!threads.empty()) { \
+ threads.back().join(); \
+ threads.pop_back(); \
+ } \
+ } \
+ } register ## testName ## ThreadTest
+
+using namespace log4cxx;
+
+LOGUNIT_CLASS(AutoConfigureTestCase)
+{
+ LOGUNIT_TEST_SUITE(AutoConfigureTestCase);
+ LOGUNIT_TEST_THREADS(test1, 4);
+ LOGUNIT_TEST(test2);
+ LOGUNIT_TEST_SUITE_END();
+#ifdef _DEBUG
+ struct Fixture
+ {
+ Fixture() {
+ helpers::LogLog::setInternalDebugging(true);
+ }
+ } suiteFixture;
+#endif
+public:
+ void test1()
+ {
+ auto debugLogger =
LogManager::getLogger(LOG4CXX_STR("AutoConfig.test1"));
+ LOGUNIT_ASSERT(debugLogger);
+ LOGUNIT_ASSERT(!debugLogger->isDebugEnabled());
+ auto rep = LogManager::getLoggerRepository();
+ LOGUNIT_ASSERT(rep);
+ LOGUNIT_ASSERT(rep->isConfigured());
+ }
+
+ void test2()
+ {
+ auto debugLogger =
Logger::getLogger(LOG4CXX_STR("AutoConfig.test2"));
+ LOGUNIT_ASSERT(debugLogger);
+ LOGUNIT_ASSERT(debugLogger->isDebugEnabled());
+ }
+};
+
+LOGUNIT_TEST_SUITE_REGISTRATION(AutoConfigureTestCase);
diff --git a/src/test/resources/input/autoConfigureTest.properties
b/src/test/resources/input/autoConfigureTest.properties
new file mode 100644
index 0000000..1187d13
--- /dev/null
+++ b/src/test/resources/input/autoConfigureTest.properties
@@ -0,0 +1,25 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+log4j.rootCategory=INFO, A1
+
+log4j.appender.A1=org.apache.log4j.FileAppender
+log4j.appender.A1.File=output/autoConfigureTest.log
+log4j.appender.A1.Append=false
+log4j.appender.A1.layout=org.apache.log4j.PatternLayout
+log4j.appender.A1.layout.ConversionPattern=%m%n
+
+#log4j.logger.AutoConfig.test1=DEBUG
+log4j.logger.AutoConfig.test2=DEBUG
