diff --git a/src/cmake/pthread/log4cxx-pthread.cmake b/src/cmake/pthread/log4cxx-pthread.cmake new file mode 100644 index 000000000..d4a527225 --- /dev/null +++ b/src/cmake/pthread/log4cxx-pthread.cmake @@ -0,0 +1,6 @@ +include(FindThreads) + +try_compile(PTHREAD_SETNAME_NP_FOUND "${CMAKE_BINARY_DIR}/pthread-compile-tests" + "${CMAKE_CURRENT_LIST_DIR}/test-pthread.cpp" + LINK_LIBRARIES Threads::Threads ) + diff --git a/src/cmake/pthread/test-pthread.cpp b/src/cmake/pthread/test-pthread.cpp new file mode 100644 index 000000000..73f453cc7 --- /dev/null +++ b/src/cmake/pthread/test-pthread.cpp @@ -0,0 +1,6 @@ +#include + +int main(){ + pthread_t tid; + pthread_setname_np(tid, "name"); +} diff --git a/src/main/cpp/CMakeLists.txt b/src/main/cpp/CMakeLists.txt index ff5375dc7..0f8f40d29 100644 --- a/src/main/cpp/CMakeLists.txt +++ b/src/main/cpp/CMakeLists.txt @@ -147,6 +147,7 @@ target_sources(log4cxx threadlocal.cpp threadpatternconverter.cpp threadspecificdata.cpp + threadutility.cpp throwableinformationpatternconverter.cpp timebasedrollingpolicy.cpp timezone.cpp diff --git a/src/main/cpp/asyncappender.cpp b/src/main/cpp/asyncappender.cpp index 6da23b18d..d357ff0ff 100644 --- a/src/main/cpp/asyncappender.cpp +++ b/src/main/cpp/asyncappender.cpp @@ -30,6 +30,7 @@ #include #include #include +#include using namespace log4cxx; @@ -50,7 +51,7 @@ AsyncAppender::AsyncAppender() locationInfo(false), blocking(true) { - dispatcher = std::thread( &AsyncAppender::dispatch, this ); + dispatcher = ThreadUtility::instance()->createThread( LOG4CXX_STR("AsyncAppender"), &AsyncAppender::dispatch, this ); } AsyncAppender::~AsyncAppender() diff --git a/src/main/cpp/filewatchdog.cpp b/src/main/cpp/filewatchdog.cpp index e3dceb595..f9d0d7f03 100644 --- a/src/main/cpp/filewatchdog.cpp +++ b/src/main/cpp/filewatchdog.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include using namespace log4cxx; @@ -92,7 +93,7 @@ void FileWatchdog::start() { checkAndConfigure(); - thread = std::thread( &FileWatchdog::run, this ); + thread = ThreadUtility::instance()->createThread( LOG4CXX_STR("FileWatchdog"), &FileWatchdog::run, this ); } bool FileWatchdog::is_interrupted() diff --git a/src/main/cpp/socketappenderskeleton.cpp b/src/main/cpp/socketappenderskeleton.cpp index 122de9d64..e3e137506 100644 --- a/src/main/cpp/socketappenderskeleton.cpp +++ b/src/main/cpp/socketappenderskeleton.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include using namespace log4cxx; @@ -162,7 +163,7 @@ void SocketAppenderSkeleton::fireConnector() { LogLog::debug(LOG4CXX_STR("Connector thread not alive: starting monitor.")); - thread = std::thread( &SocketAppenderSkeleton::monitor, this ); + thread = ThreadUtility::instance()->createThread( LOG4CXX_STR("SocketAppend"), &SocketAppenderSkeleton::monitor, this ); } } diff --git a/src/main/cpp/sockethubappender.cpp b/src/main/cpp/sockethubappender.cpp index a88eb42a3..7d83a23a4 100644 --- a/src/main/cpp/sockethubappender.cpp +++ b/src/main/cpp/sockethubappender.cpp @@ -30,6 +30,7 @@ #include #include #include +#include #include using namespace log4cxx; @@ -177,7 +178,7 @@ void SocketHubAppender::append(const spi::LoggingEventPtr& event, Pool& p) void SocketHubAppender::startServer() { - thread = std::thread( &SocketHubAppender::monitor, this ); + thread = ThreadUtility::instance()->createThread( LOG4CXX_STR("SocketHub"), &SocketHubAppender::monitor, this ); } void SocketHubAppender::monitor() diff --git a/src/main/cpp/telnetappender.cpp b/src/main/cpp/telnetappender.cpp index 418aac009..90872423f 100644 --- a/src/main/cpp/telnetappender.cpp +++ b/src/main/cpp/telnetappender.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include using namespace log4cxx; @@ -61,7 +62,7 @@ void TelnetAppender::activateOptions(Pool& /* p */) serverSocket->setSoTimeout(1000); } - sh = std::thread( &TelnetAppender::acceptConnections, this ); + sh = ThreadUtility::instance()->createThread( LOG4CXX_STR("TelnetAppender"), &TelnetAppender::acceptConnections, this ); } void TelnetAppender::setOption(const LogString& option, diff --git a/src/main/cpp/threadutility.cpp b/src/main/cpp/threadutility.cpp new file mode 100644 index 000000000..a6c7ca2d5 --- /dev/null +++ b/src/main/cpp/threadutility.cpp @@ -0,0 +1,127 @@ +#include "log4cxx/helpers/threadutility.h" +#include "log4cxx/private/log4cxx_private.h" +#include "log4cxx/helpers/loglog.h" + +#include +#include + +#if LOG4CXX_HAS_SETTHREADDESCRIPTION +#include +#include +#endif + +using log4cxx::helpers::ThreadUtility; + +struct ThreadUtility::priv_data{ + priv_data(){ + start_pre = nullptr; + started = nullptr; + start_post = nullptr; + } + + log4cxx::helpers::ThreadStartPre start_pre; + log4cxx::helpers::ThreadStarted started; + log4cxx::helpers::ThreadStartPost start_post; +}; + +#if LOG4CXX_HAS_PTHREAD_SIGMASK +static thread_local sigset_t old_mask; +static thread_local bool sigmask_valid; +#endif + +ThreadUtility::ThreadUtility() : + m_priv( new priv_data() ) +{} + +ThreadUtility::~ThreadUtility(){} + +std::shared_ptr ThreadUtility::instance(){ + static std::shared_ptr instance( new ThreadUtility() ); + return instance; +} + +void ThreadUtility::configure( ThreadConfigurationType type ){ + std::shared_ptr utility = instance(); + + if( type == ThreadConfigurationType::NoConfiguration ){ + utility->configureFuncs( nullptr, nullptr, nullptr ); + }else if( type == ThreadConfigurationType::NameThreadOnly ){ + utility->configureFuncs( nullptr, + std::bind( &ThreadUtility::threadStartedNameThread, utility, + std::placeholders::_1, + std::placeholders::_2, + std::placeholders::_3 ), + nullptr ); + }else if( type == ThreadConfigurationType::BlockSignalsOnly ){ + utility->configureFuncs( std::bind( &ThreadUtility::preThreadBlockSignals, utility ), + nullptr, + std::bind( &ThreadUtility::postThreadUnblockSignals, utility ) ); + }else if( type == ThreadConfigurationType::BlockSignalsAndNameThread ){ + utility->configureFuncs( std::bind( &ThreadUtility::preThreadBlockSignals, utility ), + std::bind( &ThreadUtility::threadStartedNameThread, utility, + std::placeholders::_1, + std::placeholders::_2, + std::placeholders::_3 ), + std::bind( &ThreadUtility::postThreadUnblockSignals, utility ) ); + } +} + +void ThreadUtility::configureFuncs( ThreadStartPre pre_start, + ThreadStarted started, + ThreadStartPost post_start ){ + m_priv->start_pre = pre_start; + m_priv->started = started; + m_priv->start_post = post_start; +} + +void ThreadUtility::preThreadBlockSignals(){ +#if LOG4CXX_HAS_PTHREAD_SIGMASK + sigset_t set; + sigfillset(&set); + if( pthread_sigmask(SIG_SETMASK, &set, &old_mask) < 0 ){ + LOGLOG_ERROR( LOG4CXX_STR("Unable to set thread sigmask") ); + sigmask_valid = false; + }else{ + sigmask_valid = true; + } +#endif /* LOG4CXX_HAS_PTHREAD_SIGMASK */ +} + +void ThreadUtility::threadStartedNameThread(LogString threadName, + std::thread::id /*threadId*/, + std::thread::native_handle_type nativeHandle){ +#if LOG4CXX_HAS_PTHREAD_SETNAME + if( pthread_setname_np( static_cast( nativeHandle ), threadName.c_str() ) < 0 ){ + LOGLOG_ERROR( LOG4CXX_STR("unable to set thread name") ); + } +#elif LOG4CXX_HAS_SETTHREADDESCRIPTION + HRESULT hr = SetThreadDescription(static_cast(nativeHandle), threadName.c_str()); + if(FAILED(hr)){ + LOGLOG_ERROR( LOG4CXX_STR("unable to set thread name") ); + } +#endif +} + +void ThreadUtility::postThreadUnblockSignals(){ +#if LOG4CXX_HAS_PTHREAD_SIGMASK + // Only restore the signal mask if we were able to set it in the first place. + if( sigmask_valid ){ + if( pthread_sigmask(SIG_SETMASK, &old_mask, nullptr) < 0 ){ + LOGLOG_ERROR( LOG4CXX_STR("Unable to set thread sigmask") ); + } + } +#endif /* LOG4CXX_HAS_PTHREAD_SIGMASK */ +} + + +log4cxx::helpers::ThreadStartPre ThreadUtility::preStartFunction(){ + return m_priv->start_pre; +} + +log4cxx::helpers::ThreadStarted ThreadUtility::threadStartedFunction(){ + return m_priv->started; +} + +log4cxx::helpers::ThreadStartPost ThreadUtility::postStartFunction(){ + return m_priv->start_post; +} diff --git a/src/main/include/CMakeLists.txt b/src/main/include/CMakeLists.txt index 584941c97..ce3b4e867 100644 --- a/src/main/include/CMakeLists.txt +++ b/src/main/include/CMakeLists.txt @@ -85,8 +85,26 @@ CHECK_FUNCTION_EXISTS(wcstombs HAS_WCSTOMBS) CHECK_FUNCTION_EXISTS(fwide HAS_FWIDE) CHECK_LIBRARY_EXISTS(esmtp smtp_create_session "" HAS_LIBESMTP) CHECK_FUNCTION_EXISTS(syslog HAS_SYSLOG) +if(UNIX) + set(CMAKE_REQUIRED_LIBRARIES "pthread") + CHECK_SYMBOL_EXISTS(pthread_sigmask "signal.h" HAS_PTHREAD_SIGMASK) + + # Check for the (linux) pthread_setname_np. + # OSX and BSD are special apparently. OSX only lets you name + # the current thread, while BSD calls it pthread_set_name_np. + # Since this is really not a core feature and the end-user can configure + # it anyway, we're only going to worry about linux. + include(${LOG4CXX_SOURCE_DIR}/src/cmake/pthread/log4cxx-pthread.cmake) + if(${PTHREAD_SETNAME_NP_FOUND}) + set(HAS_PTHREAD_SETNAME 1) + endif() +endif(UNIX) -foreach(varName HAS_STD_LOCALE HAS_ODBC HAS_MBSRTOWCS HAS_WCSTOMBS HAS_FWIDE HAS_LIBESMTP HAS_SYSLOG) +if(WIN32) + CHECK_SYMBOL_EXISTS(SetThreadDescription "windows.h;processthreadsapi.h" HAS_SETTHREADDESCRIPTION) +endif(WIN32) + +foreach(varName HAS_STD_LOCALE HAS_ODBC HAS_MBSRTOWCS HAS_WCSTOMBS HAS_FWIDE HAS_LIBESMTP HAS_SYSLOG HAS_PTHREAD_SIGMASK HAS_PTHREAD_SETNAME HAS_SETTHREADDESCRIPTION) if(${varName} EQUAL 0) continue() elseif(${varName} EQUAL 1) diff --git a/src/main/include/log4cxx/helpers/threadutility.h b/src/main/include/log4cxx/helpers/threadutility.h new file mode 100644 index 000000000..84dd25fc6 --- /dev/null +++ b/src/main/include/log4cxx/helpers/threadutility.h @@ -0,0 +1,132 @@ +#ifndef _LOG4CXX_THREADUTILITY_H +#define _LOG4CXX_THREADUTILITY_H + +#include +#include +#include + +#include "log4cxx/logstring.h" + +namespace log4cxx { +namespace helpers { + +/** + * A function that will be called before a thread is started. This can + * be used to (for example) block all of the signals in the thread, so + * that when the thread is created it will have a correct signal mask. + */ +typedef std::function ThreadStartPre; + +/** + * Called when a new thread has started. This can be used to set + * parameters for the thread in a platform-specific manner. + * + * @param threadName The name of the thread + * @param threadId The ID of the thread as reported by std::thread::get_id + * @param nativeHandle The native handle of the thread, as reported by + * std::thread::native_handle + */ +typedef std::function ThreadStarted; + +/** + * Called after a thread has started. This can be used to (for example) + * unblock the signals in the thread. + */ +typedef std::function ThreadStartPost; + +enum class ThreadConfigurationType { + NoConfiguration, + BlockSignalsOnly, + NameThreadOnly, + BlockSignalsAndNameThread, +}; + +class ThreadUtility; +LOG4CXX_PTR_DEF(ThreadUtility); + +class LOG4CXX_EXPORT ThreadUtility { +private: + ThreadUtility(); + + log4cxx::helpers::ThreadStartPre preStartFunction(); + log4cxx::helpers::ThreadStarted threadStartedFunction(); + log4cxx::helpers::ThreadStartPost postStartFunction(); + + struct priv_data; + std::unique_ptr m_priv; + +public: + ~ThreadUtility(); + + static std::shared_ptr instance(); + + /** + * Utility method for configuring the ThreadUtility in a standard + * configuration. + */ + static void configure( ThreadConfigurationType type ); + + /** + * Configure the thread functions that log4cxx will use. + * Note that setting any of these parameters to nullptr is valid, + * and simply results in the callback not being called. + */ + void configureFuncs( ThreadStartPre pre_start, + ThreadStarted started, + ThreadStartPost post_start ); + + /** + * A pre-start thread function that blocks signals to the new thread + * (if the system has pthreads). If the system does not have pthreads, + * does nothing. + */ + void preThreadBlockSignals(); + + /** + * A thread_started function that names the thread using the appropriate + * system call. + */ + void threadStartedNameThread(LogString threadName, + std::thread::id thread_id, + std::thread::native_handle_type native_handle); + + /** + * A post-start thread function that unblocks signals that preThreadBlockSignals + * blocked before starting the thread. If the system does not have pthreads, + * does nothing. + */ + void postThreadUnblockSignals(); + + /** + * Start a thread + */ + template + std::thread createThread(LogString name, + Function&& f, + Args&&... args){ + log4cxx::helpers::ThreadStartPre pre_start = preStartFunction(); + log4cxx::helpers::ThreadStarted thread_start = threadStartedFunction(); + log4cxx::helpers::ThreadStartPost post_start = postStartFunction(); + + if( pre_start ){ + pre_start(); + } + std::thread t( f, args... ); + if( thread_start ){ + thread_start( name, + t.get_id(), + t.native_handle() ); + } + if( post_start ){ + post_start(); + } + return t; + } +}; + +} /* namespace helpers */ +} /* namespace log4cxx */ + +#endif diff --git a/src/main/include/log4cxx/private/log4cxx_private.h.in b/src/main/include/log4cxx/private/log4cxx_private.h.in index 280f21f0e..bf965c937 100644 --- a/src/main/include/log4cxx/private/log4cxx_private.h.in +++ b/src/main/include/log4cxx/private/log4cxx_private.h.in @@ -53,6 +53,10 @@ #define LOG4CXX_WIN32_THREAD_FMTSPEC "0x%.8x" #define LOG4CXX_APR_THREAD_FMTSPEC "0x%pt" +#define LOG4CXX_HAS_PTHREAD_SIGMASK @HAS_PTHREAD_SIGMASK@ +#define LOG4CXX_HAS_PTHREAD_SETNAME @HAS_PTHREAD_SETNAME@ +#define LOG4CXX_HAS_SETTHREADDESCRIPTION @HAS_SETTHREADDESCRIPTION@ + #ifdef __BORLANDC__ /* * embarcadero/RAD Studio compilers don't support thread_local: diff --git a/src/site/markdown/1-usage.md b/src/site/markdown/1-usage.md index f8c6fe732..379c45b60 100644 --- a/src/site/markdown/1-usage.md +++ b/src/site/markdown/1-usage.md @@ -24,6 +24,7 @@ Usage {#usage-overview} See the following pages for usage information: * @subpage usage +* @subpage threading * @subpage extending-log4cxx * @subpage faq * @subpage configuration-samples diff --git a/src/site/markdown/threading.md b/src/site/markdown/threading.md new file mode 100644 index 000000000..41faaa5d9 --- /dev/null +++ b/src/site/markdown/threading.md @@ -0,0 +1,94 @@ +Threading {#threading} +=== + + +# Threading Notes with Log4cxx + +Log4cxx is designed to be thread-safe under under normal usage. This +means that logging itself is always thread-safe, however there are +certain circumstances that can cause threading issues with Log4cxx. + +## Unexpected Exit + +In multithreaded applications, it is possible to call `exit()` from any +thread in the application. When this happens, other threads in the +application may continue to run and attempt to log information. As of +version 12 of Log4cxx, this should not cause the library to crash. + +We recommend that a graceful exit be performed whenever possible, and that +all threads be terminated properly before returning from `main()`. + +See [LOGCXX-322][3] for more information. + +## Signal Handling with Log4cxx + +Under certain configurations, Log4cxx may create new threads in order to do +tasks(e.g. network comms, other async operations). On Linux/POSIX systems, +this can lead to undesirable signal delivery, as signals can be delivered to +any thread in the process. This can be most clearly seen if your application +uses the [sigwait(3)][4] system call, as the thread that calls sigwait may +not be the thread that actually gets the signal. + +There are three main ways to handle signals coming to your process. All +of these ways of handling signals are supported by Log4cxx in order to +provide flexibility to users of the library. These three ways are: + +1. Write to a file descriptor in a signal handler, notifying your main event +loop of a signal. If you use Qt, [their documentation][2] has information on +this method of handling a signal. +2. (Linux-only) Use [signalfd(2)][1] to create a file descriptor that notifies +you of a signal. This is a special case of (1). +3. Block signals in newly created threads, ensuring that signals can only be +sent to threads of your choosing. + +If you need to use option #3(for example, because you are using sigwait), +Log4cxx provides a mechanism for defining methods to be called at two main +points in the lifecycle of a thread: + +1. Just before the thread starts +2. Just after the thread starts + +These two points are intended to let client code determine how best to start +threads. Log4cxx provides a basic implementation of these for POSIX in order +to block signals to the new threads that it creates. + +Once a new thread is created, there is also a callback function that lets +client code do operations on the thread directly. A sample method in Log4cxx +has a callback to name the thread in order to make debugging nicer. + +In order to use these callback functions, use the [ThreadUtility](@ref log4cxx.helpers.ThreadUtility) +class. You can configure the ThreadUtility class in several different ways by using the +[ThreadUtility::configure](@ref log4cxx.helpers.ThreadUtility.configure) +method with several pre-defined configurations. +In the event that you need special signal handling, you can implement your own +functions, and use the ThreadUtility::configureFuncs method in order to +customize exactly what happens. + +**NOTE:** It is very important that if you use the `ThreadUtility::preThreadBlockSignals` +method, it must be paired with the equivalent `ThreadUtility::postThreadUnblockSignals` +call, as there is an internal mutex that is locked and unlocked in order to ensure that +only one thread can be started at a time. Failure to do this may lead to deadlock. +The ThreadUtility::configure method handles this automatically. + +[1]: https://man7.org/linux/man-pages/man2/signalfd.2.html +[2]: https://doc.qt.io/qt-5/unix-signals.html +[3]: https://issues.apache.org/jira/browse/LOGCXX-322 +[4]: https://man7.org/linux/man-pages/man3/sigwait.3.html diff --git a/src/test/cpp/helpers/CMakeLists.txt b/src/test/cpp/helpers/CMakeLists.txt index c40681cd5..fda8194bd 100644 --- a/src/test/cpp/helpers/CMakeLists.txt +++ b/src/test/cpp/helpers/CMakeLists.txt @@ -18,6 +18,7 @@ set(HELPER_TESTS syslogwritertest timezonetestcase transcodertestcase + threadutilitytestcase ) foreach(fileName IN LISTS HELPER_TESTS) add_executable(${fileName} "${fileName}.cpp") diff --git a/src/test/cpp/helpers/threadutilitytestcase.cpp b/src/test/cpp/helpers/threadutilitytestcase.cpp new file mode 100644 index 000000000..05b7cfd08 --- /dev/null +++ b/src/test/cpp/helpers/threadutilitytestcase.cpp @@ -0,0 +1,87 @@ +/* + * 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 + +using namespace log4cxx; +using namespace log4cxx::helpers; + + +LOGUNIT_CLASS(ThreadUtilityTest) +{ + LOGUNIT_TEST_SUITE(ThreadUtilityTest); + LOGUNIT_TEST(testNullFunctions); + LOGUNIT_TEST(testCustomFunctions); + LOGUNIT_TEST(testDefaultFunctions); + LOGUNIT_TEST_SUITE_END(); + +public: + void testNullFunctions(){ + ThreadUtilityPtr thrUtil = ThreadUtility::instance(); + + thrUtil->configureFuncs( nullptr, nullptr, nullptr ); + + std::thread t = thrUtil->createThread( LOG4CXX_STR("FooName"), [](){} ); + + t.join(); + } + + void testCustomFunctions(){ + ThreadUtilityPtr thrUtil = ThreadUtility::instance(); + int num_pre = 0; + int num_started = 0; + int num_post = 0; + + thrUtil->configureFuncs( + [&num_pre](){ + num_pre++; + }, + [&num_started]( LogString, + std::thread::id, + std::thread::native_handle_type ){ + num_started++; + }, + [&num_post](){ + num_post++; + } + ); + + std::thread t = thrUtil->createThread( LOG4CXX_STR("FooName"), [](){} ); + + t.join(); + + LOGUNIT_ASSERT_EQUAL( num_pre, 1 ); + LOGUNIT_ASSERT_EQUAL( num_started, 1 ); + LOGUNIT_ASSERT_EQUAL( num_post, 1 ); + } + + void testDefaultFunctions(){ + ThreadUtility::configure( ThreadConfigurationType::BlockSignalsAndNameThread ); + + ThreadUtilityPtr thrUtil = ThreadUtility::instance(); + + std::thread t = thrUtil->createThread( LOG4CXX_STR("FooName"), [](){} ); + + t.join(); + } + +}; + + +LOGUNIT_TEST_SUITE_REGISTRATION(ThreadUtilityTest); +