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:23 UTC
[logging-log4cxx] branch LOGCXX-431 created (now 091a9ce)
This is an automated email from the ASF dual-hosted git repository.
rmiddleton pushed a change to branch LOGCXX-431
in repository https://gitbox.apache.org/repos/asf/logging-log4cxx.git.
at 091a9ce LOGCXX-431 Update thread creation
This branch includes the following new commits:
new 091a9ce LOGCXX-431 Update thread creation
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[logging-log4cxx] 01/01: LOGCXX-431 Update thread creation
Posted by rm...@apache.org.
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