[LOGCXX-430] LogManager::getRootLogger is not thread-safe

• Type: Bug
• Status: Resolved
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 0.10.0
• Fix Version/s: 0.13.0
• Component/s: Core
• Assignee: Stephen Webb
• Reporter: Thorsten Schöning
• Created: 11/Mar/14 08:56
• Updated: 18/Aug/22 07:24
• Resolved: 18/Aug/22 07:24

Description

Kaspar Fischer reported following on the user mailing list:

> I am running into a situation where calling getRootLogger()
> concurrently from many requests results in a EXC_BAD_ACCESS:

> liblog4cxx.10.dylib`log4cxx::LogManager::getRootLogger():
> 0x101f180a0: pushq %rbp
> 0x101f180a1: movq %rsp, %rbp
> 0x101f180a4: pushq %rbx
> 0x101f180a5: pushq %rax
> 0x101f180a6: movq %rdi, %rbx
> 0x101f180a9: callq 0x101f17de0 ;
> log4cxx::LogManager::getLoggerRepository()
> 0x101f180ae: movq 8(%rax), %rsi
> 0x101f180b2: movq (%rsi), %rax
> 0x101f180b5: movq %rbx, %rdi
> 0x101f180b8: callq *120(%rax) <<<<<< THREAD 1: EXC_BAD_ACCESS (code=EXC_I386_GPFLT)
> 0x101f180bb: movq %rbx, %rax
> 0x101f180be: addq $8, %rsp
> 0x101f180c2: popq %rbx
> 0x101f180c3: popq %rbp
> 0x101f180c4: ret
> 0x101f180c5: nopw %cs%rax,%rax)

> If I replace the logging statement with a statement that writes to
> std::cerr, I do not run into any problems.

> I am using log4cxx 0.10.0 on MacOS 10.9.1.

The call to getRootLogger ultimately results in:

RepositorySelectorPtr& LogManager::getRepositorySelector() {
   //
   //     call to initialize APR and trigger "start" of logging clock
   //
   APRInitializer::initialize();
   static spi::RepositorySelectorPtr selector;
   return selector;
}

It looks like we have the same problem here like in LOGCXX-394, a function static which is not thread-safe. Therefore we shoudl fix this like we did in LOGCXX-394 by removing the function static.

Problem is that for this to work we need to change the return type of the function, which shouldn't be a problem because it's private, but need to change additional functions as well: getLoggerRepository and setRepositorySelector both currently rely on the current behavior of getRepositorySelector.

Additionally what I don't understand is why APRInitializer is called more than once even if RepositorySelector only got created once because of it's static.

Looks ike we need to rework the whole part, maybe make it static on a class level like the guard.

Issue Links

• is related to: LOGCXX-322 Crashes on exit from multithreaded program using log4cxx
• is related to: LOGCXX-394 Levels are not thread safe