You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by sw...@apache.org on 2026/08/26 03:10:28 UTC

(logging-log4cxx) branch master updated: Using default configuration, BufferedIO and a watchdog caused recursive configuration (#739)

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 f11f6fc6 Using default configuration, BufferedIO and a watchdog caused recursive configuration (#739)
f11f6fc6 is described below

commit f11f6fc6b8c98dd57bf3e173e1ba3e4f527ce4ce
Author: Stephen Webb <st...@ieee.org>
AuthorDate: Wed Aug 26 13:10:23 2026 +1000

    Using default configuration, BufferedIO and a watchdog caused recursive configuration (#739)
    
    * Stop the watchdog from loading the configuration file twice
    
    * Prevent internal logger creation from triggering default configuration
---
 src/main/cpp/filewatchdog.cpp                      |  9 +-
 src/main/cpp/hierarchy.cpp                         |  2 +-
 src/main/cpp/loglog.cpp                            |  6 ++
 src/main/cpp/threadutility.cpp                     |  2 +-
 src/main/include/log4cxx/helpers/loglog.h          |  5 ++
 src/test/cpp/CMakeLists.txt                        |  1 +
 src/test/cpp/autoconfiguretestcase.cpp             |  4 +-
 src/test/cpp/defaultconfiguratortest.cpp           | 97 ++++++++++++++++++++++
 .../resources/input/defaultconfiguratortest.xml    | 28 +++++++
 9 files changed, 147 insertions(+), 7 deletions(-)

diff --git a/src/main/cpp/filewatchdog.cpp b/src/main/cpp/filewatchdog.cpp
index efbc1fea..66c7a482 100644
--- a/src/main/cpp/filewatchdog.cpp
+++ b/src/main/cpp/filewatchdog.cpp
@@ -162,8 +162,11 @@ void FileWatchdog::setDelay(long delay1){
 	}
 }
 
-void FileWatchdog::setFile(const File& filename)
+void FileWatchdog::setFile(const File& newValue)
 {
-    m_priv->file = filename;
-	m_priv->lastModif = 0;
+	if (m_priv->file.getPath() != newValue.getPath())
+	{
+		m_priv->file = newValue;
+		m_priv->lastModif = 0;
+	}
 }
diff --git a/src/main/cpp/hierarchy.cpp b/src/main/cpp/hierarchy.cpp
index 60142105..4b7103e2 100644
--- a/src/main/cpp/hierarchy.cpp
+++ b/src/main/cpp/hierarchy.cpp
@@ -307,8 +307,8 @@ void Hierarchy::ensureIsConfigured(std::function<void()> configurator)
 	std::lock_guard<std::recursive_mutex> lock(m_priv->configuredMutex);
 	if (!m_priv->configured && m_priv->alreadyTriedMethod != configurator.target_type().name())
 	{
-		configurator();
 		m_priv->alreadyTriedMethod = configurator.target_type().name();
+		configurator();
 	}
 }
 
diff --git a/src/main/cpp/loglog.cpp b/src/main/cpp/loglog.cpp
index 1f8875fe..3aab3dac 100644
--- a/src/main/cpp/loglog.cpp
+++ b/src/main/cpp/loglog.cpp
@@ -17,6 +17,7 @@
 
 #include <log4cxx/logstring.h>
 #include <log4cxx/helpers/loglog.h>
+#include <log4cxx/logmanager.h>
 #include <log4cxx/helpers/transcoder.h>
 #include <iostream>
 #if !defined(LOG4CXX)
@@ -187,6 +188,11 @@ void LogLog::error(const LogString& msg, const std::exception& e)
 	}
 }
 
+LoggerPtr LogLog::getLogger(const LogString& name)
+{
+	return LogManager::getLoggerRepository()->getLogger(name);
+}
+
 #if !LOG4CXX_LOGCHAR_IS_UTF8
 void LogLog::trace(const LoggerPtr& category, const std::string& msg)
 {
diff --git a/src/main/cpp/threadutility.cpp b/src/main/cpp/threadutility.cpp
index 4ea4c078..230cad8d 100644
--- a/src/main/cpp/threadutility.cpp
+++ b/src/main/cpp/threadutility.cpp
@@ -270,7 +270,7 @@ 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");
+		m_priv->log = LogLog::getLogger(LOG4CXX_STR("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)
diff --git a/src/main/include/log4cxx/helpers/loglog.h b/src/main/include/log4cxx/helpers/loglog.h
index 82fde47d..bd64f792 100644
--- a/src/main/include/log4cxx/helpers/loglog.h
+++ b/src/main/include/log4cxx/helpers/loglog.h
@@ -132,6 +132,11 @@ class LOG4CXX_EXPORT LogLog
 		*/
 		static void warn(const LogString&  msg, const std::exception& ex);
 
+		/**
+		A \c name category for internal logging.
+		*/
+		static LoggerPtr getLogger(const LogString& name);
+
 	private:
 		static void emit_log(const LogString& prefix, const LogString& msg, const LogString& suffix);
 		static void emit_log(const LogString& prefix, const std::exception& ex, const LogString& suffix);
diff --git a/src/test/cpp/CMakeLists.txt b/src/test/cpp/CMakeLists.txt
index 913a5512..e3d82518 100644
--- a/src/test/cpp/CMakeLists.txt
+++ b/src/test/cpp/CMakeLists.txt
@@ -49,6 +49,7 @@ set(ALL_LOG4CXX_TESTS
     asyncappendertestcase
     asyncappenderracestress
     consoleappendertestcase
+    defaultconfiguratortest
     decodingtest
     encodingtest
     fileappendertest
diff --git a/src/test/cpp/autoconfiguretestcase.cpp b/src/test/cpp/autoconfiguretestcase.cpp
index af3fe15e..38336738 100644
--- a/src/test/cpp/autoconfiguretestcase.cpp
+++ b/src/test/cpp/autoconfiguretestcase.cpp
@@ -113,8 +113,8 @@ public:
 		LogManager::shutdown();
 		LOG4CXX_ENCODE_CHAR(configFile, m_configFile);
 		apr_file_remove(configFile.c_str(), m_pool.getAPRPool());
-		// wait 0.2 sec to ensure the file is really gone on Windows
-		apr_sleep(200000);
+		// wait 1 sec to ensure the WatchDog will reload the configuration
+		apr_sleep(1000000);
 	}
 
 	void testSetup()
diff --git a/src/test/cpp/defaultconfiguratortest.cpp b/src/test/cpp/defaultconfiguratortest.cpp
new file mode 100644
index 00000000..6e93ad9a
--- /dev/null
+++ b/src/test/cpp/defaultconfiguratortest.cpp
@@ -0,0 +1,97 @@
+/*
+ * 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/logmanager.h>
+#include <log4cxx/defaultconfigurator.h>
+#include <log4cxx/basicconfigurator.h>
+#include <log4cxx/fileappender.h>
+#include <log4cxx/helpers/filesystempath.h>
+
+using namespace LOG4CXX_NS;
+
+namespace
+{
+	auto GetLogger(const std::string& name) -> LoggerPtr
+	{
+		static struct log4cxx_initializer
+		{
+			log4cxx_initializer()
+			{
+				// Check every 5 seconds for configuration file changes
+				DefaultConfigurator::setConfigurationWatchSeconds(5);
+#if !LOG4CXX_HAS_FILESYSTEM_PATH
+				auto& props = spi::Configurator::properties();
+				props.setProperty(LOG4CXX_STR("PROGRAM_FILE_PATH.STEM"), LOG4CXX_STR("defaultconfiguratortest"));
+				props.setProperty(LOG4CXX_STR("PROGRAM_FILE_PATH.PARENT_PATH"), LOG4CXX_STR("output"));
+#endif
+
+
+				// Look for a configuration file in the current working directory
+				// and the same directory as the program
+				std::vector<LogString> paths
+					{ LOG4CXX_STR("input")
+					, LOG4CXX_STR("${PROGRAM_FILE_PATH.PARENT_PATH}")
+					};
+				std::vector<LogString> names
+					{ LOG4CXX_STR("${PROGRAM_FILE_PATH.STEM}.xml")
+					, LOG4CXX_STR("${PROGRAM_FILE_PATH.STEM}.properties")
+					};
+				auto status       = spi::ConfigurationStatus::NotConfigured;
+				auto selectedPath = LogString();
+				std::tie(status, selectedPath) = DefaultConfigurator::configureFromFile(paths, names);
+				if (status == spi::ConfigurationStatus::NotConfigured)
+					BasicConfigurator::configure(); // Send events to the console
+			}
+			~log4cxx_initializer()
+			{
+				LogManager::shutdown();
+			}
+		} initialiser;
+		return name.empty()
+			? LogManager::getRootLogger()
+			: LogManager::getLogger(name);
+	}
+
+	auto logger = GetLogger("com.test");
+}
+
+LOGUNIT_CLASS(DefaultConfiguratorTest)
+{
+	LOGUNIT_TEST_SUITE(DefaultConfiguratorTest);
+	LOGUNIT_TEST(test1);
+	LOGUNIT_TEST_SUITE_END();
+public:
+
+	void test1()
+	{
+		LOGUNIT_ASSERT(logger);
+		LOGUNIT_ASSERT_EQUAL(LOG4CXX_STR("com.test"), logger->getName());
+		auto comLogger = logger->getParent();
+		LOGUNIT_ASSERT(comLogger);
+		LOGUNIT_ASSERT_EQUAL(LOG4CXX_STR("com"), comLogger->getName());
+		auto rootLogger = comLogger->getParent();
+		LOGUNIT_ASSERT(rootLogger);
+		LOGUNIT_ASSERT_EQUAL(LOG4CXX_STR("root"), rootLogger->getName());
+		auto appender = rootLogger->getAppender(LOG4CXX_STR("A1"));
+		LOGUNIT_ASSERT(appender);
+		auto fileAppender = LOG4CXX_NS::cast<FileAppender>(appender);
+		LOGUNIT_ASSERT(fileAppender);
+		LOGUNIT_ASSERT(fileAppender->getBufferedIO());
+	}
+};
+
+LOGUNIT_TEST_SUITE_REGISTRATION(DefaultConfiguratorTest);
diff --git a/src/test/resources/input/defaultconfiguratortest.xml b/src/test/resources/input/defaultconfiguratortest.xml
new file mode 100644
index 00000000..5c1a4a15
--- /dev/null
+++ b/src/test/resources/input/defaultconfiguratortest.xml
@@ -0,0 +1,28 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<log4j:configuration xmlns:log4j="http://jakarta.apache.org/log4j/" debug="true">
+
+  <appender name="A1" class="org.apache.log4j.RollingFileAppender">
+    <param name="file" value="${PROGRAM_FILE_PATH.PARENT_PATH}/${PROGRAM_FILE_PATH.STEM}.log" />
+    <param name="BufferedIO" value="true" />
+    <layout class="org.apache.log4j.PatternLayout">
+      <param name="ConversionPattern" value="[%d{yyyy-MM-dd HH:mm:ss.SSS}] %c %-5p - %m%n" />
+    </layout>
+    <rollingPolicy class="org.apache.log4j.rolling.FixedWindowRollingPolicy">
+       <param name="fileNamePattern" value="${PROGRAM_FILE_PATH.PARENT_PATH}/${PROGRAM_FILE_PATH.STEM}.%i.log"/>
+       <param name="minIndex" value="0"/>
+    </rollingPolicy>
+    <triggeringPolicy class="org.apache.log4j.rolling.SizeBasedTriggeringPolicy">
+      <param name="maxFileSize" value="4MB" />
+    </triggeringPolicy>
+  </appender>
+
+  <root>
+     <priority value="info" />
+     <appender-ref ref="A1"/>
+  </root>
+
+  <logger name="com" >
+     <priority value="debug"/>
+  </logger>
+
+</log4j:configuration>
\ No newline at end of file