You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by rm...@apache.org on 2021/08/20 01:11:24 UTC
[logging-log4cxx] 01/01: LOGCXX-431 Update thread creation
This is an automated email from the ASF dual-hosted git repository.
rmiddleton pushed a commit to branch LOGCXX-431
in repository https://gitbox.apache.org/repos/asf/logging-log4cxx.git
commit 091a9ce1e9961fa8d81473e1a8b72167a9a3f912
Author: Robert Middleton <ro...@rm5248.com>
AuthorDate: Thu Aug 19 21:08:10 2021 -0400
LOGCXX-431 Update thread creation
In order to make it possible to block signals when a thread is
created, a new API to let users configure what to do when a
thread is started has been created.
Documentation on how Log4cxx handles threads, and information
about good ways to handle signals has been added as well.
---
src/main/cpp/CMakeLists.txt | 1 +
src/main/cpp/asyncappender.cpp | 3 +-
src/main/cpp/filewatchdog.cpp | 3 +-
src/main/cpp/socketappenderskeleton.cpp | 3 +-
src/main/cpp/sockethubappender.cpp | 3 +-
src/main/cpp/telnetappender.cpp | 3 +-
src/main/cpp/threadutility.cpp | 83 +++++++++++++++
src/main/include/CMakeLists.txt | 12 ++-
src/main/include/log4cxx/helpers/threadutility.h | 114 +++++++++++++++++++++
.../include/log4cxx/private/log4cxx_private.h.in | 4 +
src/site/markdown/1-usage.md | 1 +
src/site/markdown/threading.md | 82 +++++++++++++++
12 files changed, 306 insertions(+), 6 deletions(-)
diff --git a/src/main/cpp/CMakeLists.txt b/src/main/cpp/CMakeLists.txt
index ff5375d..0f8f40d 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 6da23b1..2a4c27f 100644
--- a/src/main/cpp/asyncappender.cpp
+++ b/src/main/cpp/asyncappender.cpp
@@ -30,6 +30,7 @@
#include <log4cxx/helpers/stringhelper.h>
#include <apr_atomic.h>
#include <log4cxx/helpers/optionconverter.h>
+#include <log4cxx/helpers/threadutility.h>
using namespace log4cxx;
@@ -50,7 +51,7 @@ AsyncAppender::AsyncAppender()
locationInfo(false),
blocking(true)
{
- dispatcher = std::thread( &AsyncAppender::dispatch, this );
+ dispatcher = ThreadUtility::createThread( "AyncAppend", &AsyncAppender::dispatch, this );
}
AsyncAppender::~AsyncAppender()
diff --git a/src/main/cpp/filewatchdog.cpp b/src/main/cpp/filewatchdog.cpp
index e3dceb5..fb49a09 100644
--- a/src/main/cpp/filewatchdog.cpp
+++ b/src/main/cpp/filewatchdog.cpp
@@ -23,6 +23,7 @@
#include <apr_atomic.h>
#include <log4cxx/helpers/transcoder.h>
#include <log4cxx/helpers/exception.h>
+#include <log4cxx/helpers/threadutility.h>
#include <functional>
using namespace log4cxx;
@@ -92,7 +93,7 @@ void FileWatchdog::start()
{
checkAndConfigure();
- thread = std::thread( &FileWatchdog::run, this );
+ thread = ThreadUtility::createThread( "Filewatchdog", &FileWatchdog::run, this );
}
bool FileWatchdog::is_interrupted()
diff --git a/src/main/cpp/socketappenderskeleton.cpp b/src/main/cpp/socketappenderskeleton.cpp
index 122de9d..66bf920 100644
--- a/src/main/cpp/socketappenderskeleton.cpp
+++ b/src/main/cpp/socketappenderskeleton.cpp
@@ -26,6 +26,7 @@
#include <log4cxx/spi/loggingevent.h>
#include <log4cxx/helpers/transcoder.h>
#include <log4cxx/helpers/bytearrayoutputstream.h>
+#include <log4cxx/helpers/threadutility.h>
#include <functional>
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::createThread( "SocketAppend", &SocketAppenderSkeleton::monitor, this );
}
}
diff --git a/src/main/cpp/sockethubappender.cpp b/src/main/cpp/sockethubappender.cpp
index a88eb42..dc6eb7c 100644
--- a/src/main/cpp/sockethubappender.cpp
+++ b/src/main/cpp/sockethubappender.cpp
@@ -30,6 +30,7 @@
#include <log4cxx/helpers/objectoutputstream.h>
#include <log4cxx/helpers/socketoutputstream.h>
#include <log4cxx/helpers/exception.h>
+#include <log4cxx/helpers/threadutility.h>
#include <mutex>
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::createThread( "SocketHub", &SocketHubAppender::monitor, this );
}
void SocketHubAppender::monitor()
diff --git a/src/main/cpp/telnetappender.cpp b/src/main/cpp/telnetappender.cpp
index 418aac0..222b982 100644
--- a/src/main/cpp/telnetappender.cpp
+++ b/src/main/cpp/telnetappender.cpp
@@ -24,6 +24,7 @@
#include <apr_strings.h>
#include <log4cxx/helpers/charsetencoder.h>
#include <log4cxx/helpers/bytebuffer.h>
+#include <log4cxx/helpers/threadutility.h>
#include <mutex>
using namespace log4cxx;
@@ -61,7 +62,7 @@ void TelnetAppender::activateOptions(Pool& /* p */)
serverSocket->setSoTimeout(1000);
}
- sh = std::thread( &TelnetAppender::acceptConnections, this );
+ sh = ThreadUtility::createThread( "TelnetAppend", &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 0000000..bdd301b
--- /dev/null
+++ b/src/main/cpp/threadutility.cpp
@@ -0,0 +1,83 @@
+#include "log4cxx/helpers/threadutility.h"
+#include "log4cxx/private/log4cxx_private.h"
+#include "log4cxx/helpers/loglog.h"
+
+#include <signal.h>
+
+#if LOG4CXX_HAS_SETTHREADDESCRIPTION
+#include <windows.h>
+#include <processthreadsapi.h>
+#endif
+
+using log4cxx::ThreadUtility;
+
+log4cxx::pre_thread_start ThreadUtility::pre_start = ThreadUtility::preThreadDoNothing;
+log4cxx::thread_started ThreadUtility::thread_start = ThreadUtility::threadStartedDoNothing;
+log4cxx::post_thread_start ThreadUtility::post_start = ThreadUtility::postThreadDoNothing;
+
+ThreadUtility::ThreadUtility(){}
+
+void ThreadUtility::configureThreadFunctions( pre_thread_start pre_start1,
+ thread_started started,
+ post_thread_start post_start1 ){
+ if( pre_start1 == nullptr ){
+ pre_start = ThreadUtility::preThreadDoNothing;
+ }else{
+ pre_start = pre_start1;
+ }
+
+ if( started == nullptr ){
+ thread_start = ThreadUtility::threadStartedDoNothing;
+ }else{
+ thread_start = started;
+ }
+
+ if( post_start1 == nullptr ){
+ post_start = ThreadUtility::postThreadDoNothing;
+ }else{
+ post_start = pre_start1;
+ }
+}
+
+void ThreadUtility::preThreadDoNothing(){}
+
+void ThreadUtility::preThreadBlockSignals(){
+#if LOG4CXX_HAS_PTHREAD_SIGMASK
+ sigset_t set;
+ sigfillset(&set);
+ if( pthread_sigmask(SIG_SETMASK, &set, nullptr) < 0 ){
+ LOGLOG_ERROR( "Unable to set thread sigmask" );
+ }
+#endif /* LOG4CXX_HAS_PTHREAD_SIGMASK */
+}
+
+void ThreadUtility::threadStartedDoNothing(LogString,
+ std::thread::id,
+ std::thread::native_handle_type){}
+
+void ThreadUtility::threadStartedNameThread(LogString threadName,
+ std::thread::id /*thread_id*/,
+ std::thread::native_handle_type native_handle){
+#if LOG4CXX_HAS_PTHREAD_SETNAME
+ if( pthread_setname_np( static_cast<pthread_t>( native_handle ), threadName.c_str() ) < 0 ){
+ LOGLOG_ERROR( "unable to set thread name" );
+ }
+#elif LOG4CXX_HAS_SETTHREADDESCRIPTION
+ HRESULT hr = SetThreadDescription(static_cast<HANDLE>(native_handle), threadName.c_str());
+ if(FAILED(hr)){
+ LOGLOG_ERROR( "unable to set thread name" );
+ }
+#endif
+}
+
+void ThreadUtility::postThreadDoNothing(){}
+
+void ThreadUtility::postThreadUnblockSignals(){
+#if LOG4CXX_HAS_PTHREAD_SIGMASK
+ sigset_t set;
+ sigemptyset(&set);
+ if( pthread_sigmask(SIG_SETMASK, &set, nullptr) < 0 ){
+ LOGLOG_ERROR( "Unable to set thread sigmask" );
+ }
+#endif /* LOG4CXX_HAS_PTHREAD_SIGMASK */
+}
diff --git a/src/main/include/CMakeLists.txt b/src/main/include/CMakeLists.txt
index 584941c..575f68b 100644
--- a/src/main/include/CMakeLists.txt
+++ b/src/main/include/CMakeLists.txt
@@ -85,8 +85,18 @@ 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)
+ set(CMAKE_REQUIRED_LIBRARIES "pthread")
+ CHECK_SYMBOL_EXISTS(pthread_setname_np "pthread.h" HAS_PTHREAD_SETNAME)
+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 0000000..de4ffad
--- /dev/null
+++ b/src/main/include/log4cxx/helpers/threadutility.h
@@ -0,0 +1,114 @@
+#ifndef _LOG4CXX_THREADUTILITY_H
+#define _LOG4CXX_THREADUTILITY_H
+
+#include <thread>
+#include <functional>
+
+#include "log4cxx/logstring.h"
+
+namespace log4cxx {
+
+/**
+ * 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<void()> pre_thread_start;
+
+/**
+ * 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 thread_id The ID of the thread as reported by std::thread::get_id
+ * @param native_handle The native handle of the thread, as reported by
+ * std::thread::native_handle
+ */
+typedef std::function<void( LogString threadName,
+ std::thread::id thread_id,
+ std::thread::native_handle_type native_handle )> thread_started;
+
+/**
+ * Called after a thread has started. This can be used to (for example)
+ * unblock the signals in the thread.
+ */
+typedef std::function<void()> post_thread_start;
+
+class LOG4CXX_EXPORT ThreadUtility {
+public:
+ /**
+ * Configure the thread functions that log4cxx will use.
+ * Note that setting any of these parameters to nullptr will cause the
+ * default function to be used.
+ *
+ */
+ static void configureThreadFunctions( pre_thread_start pre_start,
+ thread_started started,
+ post_thread_start post_start );
+
+ /**
+ * A pre-start thread function that does nothing
+ */
+ static void preThreadDoNothing();
+
+ /**
+ * A pre-start thread function that blocks signals to the new thread
+ * (if the system has pthreads). If the system does not have pthreads,
+ * is equivalent to preThreadDoNothing();
+ */
+ static void preThreadBlockSignals();
+
+ /**
+ * A thread_started function that does nothing when the thread starts.
+ */
+ static void threadStartedDoNothing(LogString threadName,
+ std::thread::id thread_id,
+ std::thread::native_handle_type native_handle);
+
+ /**
+ * A thread_started function that names the thread using the appropriate
+ * system call.
+ */
+ static void threadStartedNameThread(LogString threadName,
+ std::thread::id thread_id,
+ std::thread::native_handle_type native_handle);
+
+ /**
+ * A post-start thread function that does nothing.
+ */
+ static void postThreadDoNothing();
+
+ /**
+ * A post-start thread function that unblocks signals that preThreadBlockSignals
+ * blocked before starting the thread. If the system does not have pthreads,
+ * is equivalent to postThreadDoNothing();
+ */
+ static void postThreadUnblockSignals();
+
+ /**
+ * Start a thread
+ */
+ template<class Function, class... Args>
+ static std::thread createThread(LogString name,
+ Function&& f,
+ Args&&... args){
+ pre_start();
+ std::thread t( f, args... );
+ thread_start( name,
+ t.get_id(),
+ t.native_handle() );
+ post_start();
+ return t;
+ }
+
+private:
+ ThreadUtility();
+
+ static log4cxx::pre_thread_start pre_start;
+ static log4cxx::thread_started thread_start;
+ static log4cxx::post_thread_start post_start;
+};
+
+}
+
+#endif
diff --git a/src/main/include/log4cxx/private/log4cxx_private.h.in b/src/main/include/log4cxx/private/log4cxx_private.h.in
index 280f21f..bf965c9 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 f8c6fe7..379c45b 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 0000000..045f319
--- /dev/null
+++ b/src/site/markdown/threading.md
@@ -0,0 +1,82 @@
+Threading {#threading}
+===
+<!--
+ Note: License header cannot be first, as doxygen does not generate
+ cleanly if it before the '==='
+-->
+<!--
+ 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.
+-->
+# Threading Notes with Log4cxx
+
+Log4cxx is designed to be thread-safe under all circumstances. However,
+this is not always the case.
+
+## 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.
+
+## Threads Created by Log4cxx
+
+Under certain configurations, Log4cxx may create new threads in order to do
+tasks(e.g. network comms, other async operations). On Linux systems, this
+can lead to undesirable signal delivery, as signals can be delivered to
+any thread in the process.
+
+To handle signals properly on Linux, you may wish to utilize the [signalfd][1]
+API to handle signals correctly. Another way of handling signals is to
+create a pipe internal to your application to notify the main loop that there
+is a signal available - see the [Qt documentation][2] for more information.
+
+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 class. You
+can use some sample functions(not no-ops) as follows:
+
+```
+ThreadUtility::configureThreadFunctions( ThreadUtility::preThreadBlockSignals,
+ ThreadUtility::threadStartedNameThread,
+ ThreadUtility::postThreadUnblockSignals );
+```
+
+These sample functions will block all POSIX signals before starting a new thread,
+and then unblock them once the thread has been created. You may provide your
+own functions to handle this if you so choose.
+
+
+[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