You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by "swebb2066 (via GitHub)" <gi...@apache.org> on 2025/07/15 07:27:14 UTC
[PR] Prevent potential deadlock on shutdown when using AsyncAppender [logging-log4cxx]
swebb2066 opened a new pull request, #511:
URL: https://github.com/apache/logging-log4cxx/pull/511
When the application does not call LogManager::shutdown before program exit, the single LogLog instance is destroyed before AsynAppender terminates. Undefined behaviour (use-after-free) arises as the std::unique_ptr destructor does not zero the pointer, but does release the memory.
The following is a example of a program deadlock that may arise:
```
(gdb) thread apply all bt
Thread 2 (Thread 0x7bc83cdff640 (LWP 13173) "MyApp3"):
#0 futex_wait (private=0, expected=2, futex_word=0x650ab00af4e8) at ../sysdeps/nptl/futex-internal.h:146
#1 __GI___lll_lock_wait (futex=futex@entry=0x650ab00af4e8, private=0) at ./nptl/lowlevellock.c:49
#2 0x00007bc83ce98002 in lll_mutex_lock_optimized (mutex=0x650ab00af4e8) at ./nptl/pthread_mutex_lock.c:48
#3 ___pthread_mutex_lock (mutex=0x650ab00af4e8) at ./nptl/pthread_mutex_lock.c:93
#4 0x00007bc83d77296c in log4cxx::helpers::LogLog::debug(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) () from /home/stephen/build/logging-log4cxx/src/main/cpp/liblog4cxx.so.15
#5 0x00007bc83d710d84 in log4cxx::AsyncAppender::dispatch() () from /home/stephen/build/logging-log4cxx/src/main/cpp/liblog4cxx.so.15
#6 0x00007bc83d2dc253 in ?? () from /lib/x86_64-linux-gnu/libstdc++.so.6
#7 0x00007bc83ce94ac3 in start_thread (arg=<optimised out>) at ./nptl/pthread_create.c:442
#8 0x00007bc83cf26850 in clone3 () at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81
Thread 1 (Thread 0x7bc83d89bb40 (LWP 13172) "MyApp3"):
#0 __futex_abstimed_wait_common64 (private=128, cancel=true, abstime=0x0, op=265, expected=13173, futex_word=0x7bc83cdff910) at ./nptl/futex-internal.c:57
#1 __futex_abstimed_wait_common (cancel=true, private=128, abstime=0x0, clockid=0, expected=13173, futex_word=0x7bc83cdff910) at ./nptl/futex-internal.c:87
#2 __GI___futex_abstimed_wait_cancelable64 (futex_word=futex_word@entry=0x7bc83cdff910, expected=13173, clockid=clockid@entry=0, abstime=abstime@entry=0x0, private=private@entry=128) at ./nptl/futex-internal.c:139
#3 0x00007bc83ce96624 in __pthread_clockjoin_ex (threadid=136099944986176, thread_return=0x0, clockid=0, abstime=0x0, block=<optimised out>) at ./nptl/pthread_join_common.c:105
#4 0x00007bc83d2dc2c7 in std::thread::join() () from /lib/x86_64-linux-gnu/libstdc++.so.6
#5 0x00007bc83d71210c in log4cxx::AsyncAppender::close() () from /home/stephen/build/logging-log4cxx/src/main/cpp/liblog4cxx.so.15
#6 0x00007bc83d707c9a in log4cxx::helpers::AppenderAttachableImpl::removeAllAppenders() () from /home/stephen/build/logging-log4cxx/src/main/cpp/liblog4cxx.so.15
#7 0x00007bc83d76c4d9 in log4cxx::Logger::removeAllAppenders() () from /home/stephen/build/logging-log4cxx/src/main/cpp/liblog4cxx.so.15
#8 0x00007bc83d748964 in log4cxx::Hierarchy::~Hierarchy() () from /home/stephen/build/logging-log4cxx/src/main/cpp/liblog4cxx.so.15
#9 0x00007bc83d74d8ca in std::_Sp_counted_ptr<log4cxx::Hierarchy*, (__gnu_cxx::_Lock_policy)2>::_M_dispose() () from /home/stephen/build/logging-log4cxx/src/main/cpp/liblog4cxx.so.15
#10 0x00007bc83d72ef75 in log4cxx::spi::DefaultRepositorySelector::~DefaultRepositorySelector() () from /home/stephen/build/logging-log4cxx/src/main/cpp/liblog4cxx.so.15
#11 0x00007bc83d70dc25 in log4cxx::helpers::APRInitializer::~APRInitializer() () from /home/stephen/build/logging-log4cxx/src/main/cpp/liblog4cxx.so.15
#12 0x00007bc83ce45495 in __run_exit_handlers (status=0, listp=0x7bc83d01a838 <__exit_funcs>, run_list_atexit=run_list_atexit@entry=true, run_dtors=run_dtors@entry=true) at ./stdlib/exit.c:113
#13 0x00007bc83ce45610 in __GI_exit (status=<optimised out>) at ./stdlib/exit.c:143
#14 0x00007bc83ce29d97 in __libc_start_call_main (main=main@entry=0x650aa05d2250 <main>, argc=argc@entry=1, argv=argv@entry=0x7ffdf8f6a8b8) at ../sysdeps/nptl/libc_start_call_main.h:74
#15 0x00007bc83ce29e40 in __libc_start_main_impl (main=0x650aa05d2250 <main>, argc=1, argv=0x7ffdf8f6a8b8, init=<optimised out>, fini=<optimised out>, rtld_fini=<optimised out>, stack_end=0x7ffdf8f6a8a8) at ../csu/libc-start.c:392
#16 0x0000650aa05d2415 in _start ()
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prevent potential deadlock on shutdown when using AsyncAppender [logging-log4cxx]
Posted by "swebb2066 (via GitHub)" <gi...@apache.org>.
swebb2066 commented on code in PR #511:
URL: https://github.com/apache/logging-log4cxx/pull/511#discussion_r2209155149
##########
src/examples/cpp/MyApp1.cpp:
##########
@@ -20,5 +20,6 @@ int main(int argc, char **argv) {
LOG4CXX_INFO(logger, "Entering application.");
foo();
LOG4CXX_INFO(logger, "Exiting application.");
+ log4cxx::LogManager::shutdown();
Review Comment:
> How about adding a comment here?
Clinking the hyperlinked `LogManager::shutdown()` provides the details.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prevent potential deadlock on shutdown when using AsyncAppender [logging-log4cxx]
Posted by "swebb2066 (via GitHub)" <gi...@apache.org>.
swebb2066 commented on code in PR #511:
URL: https://github.com/apache/logging-log4cxx/pull/511#discussion_r2209107114
##########
src/examples/cpp/MyApp1.cpp:
##########
@@ -1,11 +1,11 @@
-#include <log4cxx/logger.h>
+#include <log4cxx/logmanager.h>
#include <log4cxx/basicconfigurator.h>
-static auto logger = log4cxx::Logger::getLogger("MyApp");
Review Comment:
To use `LogManager::shutdown` I need to include `logmanager.h`, so I thought it would be clearer to then use `LogManager::getLogger`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prevent potential deadlock on shutdown when using AsyncAppender [logging-log4cxx]
Posted by "swebb2066 (via GitHub)" <gi...@apache.org>.
swebb2066 commented on code in PR #511:
URL: https://github.com/apache/logging-log4cxx/pull/511#discussion_r2209155149
##########
src/examples/cpp/MyApp1.cpp:
##########
@@ -20,5 +20,6 @@ int main(int argc, char **argv) {
LOG4CXX_INFO(logger, "Entering application.");
foo();
LOG4CXX_INFO(logger, "Exiting application.");
+ log4cxx::LogManager::shutdown();
Review Comment:
> How about adding a comment here?
Clicking the hyperlinked `LogManager::shutdown()` provides the details.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prevent potential deadlock on shutdown when using AsyncAppender [logging-log4cxx]
Posted by "rm5248 (via GitHub)" <gi...@apache.org>.
rm5248 commented on code in PR #511:
URL: https://github.com/apache/logging-log4cxx/pull/511#discussion_r2209095595
##########
src/examples/cpp/MyApp1.cpp:
##########
@@ -1,11 +1,11 @@
-#include <log4cxx/logger.h>
+#include <log4cxx/logmanager.h>
#include <log4cxx/basicconfigurator.h>
-static auto logger = log4cxx::Logger::getLogger("MyApp");
Review Comment:
is there a particular reason that we should use `LogManager::getLogger` instead of `Logger::getLogger`? IIRC they're effectively the same.
##########
src/examples/cpp/MyApp1.cpp:
##########
@@ -20,5 +20,6 @@ int main(int argc, char **argv) {
LOG4CXX_INFO(logger, "Entering application.");
foo();
LOG4CXX_INFO(logger, "Exiting application.");
+ log4cxx::LogManager::shutdown();
Review Comment:
How about adding a comment here? Something along the lines of:
```
While not needed in many cases, calling LogManager::shutdown() guarantees that
all of the resources used by log4cxx are cleaned up and disposed of properly.
```
Most simple applications don't need it, but it's probably good practice. Did we also change `LogManager::shutdown()` to work properly if there are multiple threads logging on exit?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prevent potential deadlock on shutdown when using AsyncAppender [logging-log4cxx]
Posted by "swebb2066 (via GitHub)" <gi...@apache.org>.
swebb2066 merged PR #511:
URL: https://github.com/apache/logging-log4cxx/pull/511
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prevent potential deadlock on shutdown when using AsyncAppender [logging-log4cxx]
Posted by "swebb2066 (via GitHub)" <gi...@apache.org>.
swebb2066 commented on code in PR #511:
URL: https://github.com/apache/logging-log4cxx/pull/511#discussion_r2209109613
##########
src/examples/cpp/MyApp1.cpp:
##########
@@ -20,5 +20,6 @@ int main(int argc, char **argv) {
LOG4CXX_INFO(logger, "Entering application.");
foo();
LOG4CXX_INFO(logger, "Exiting application.");
+ log4cxx::LogManager::shutdown();
Review Comment:
> Most simple applications don't need it
A simple application will morph into a more complex application (as in #510), so it is best to promote safe coding practices at the start.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org