[jira] [Commented] (LOGCXX-430) LogManager::getRootLogger is not thread-safe

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

[jira] [Commented] (LOGCXX-430) LogManager::getRootLogger is not thread-safe

Log4cxx - Dev mailing list

    [ https://issues.apache.org/jira/browse/LOGCXX-430?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15811941#comment-15811941 ]

Andrey commented on LOGCXX-430:
-------------------------------

It seems I have related crash.

{noformat}
  log4cxx.dll!std::_Tree<std::_Tmap_traits<std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> >,log4cxx::helpers::ObjectPtrT<log4cxx::Logger>,std::less<std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> > >,std::allocator<std::pair<std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> > const ,log4cxx::helpers::ObjectPtrT<log4cxx::Logger> > >,0> >::_Insert_nohint<std::pair<std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> > const ,log4cxx::helpers::ObjectPtrT<log4cxx::Logger> >,std::_Nil>(bool _Leftish=false, std::pair<std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> > const ,log4cxx::helpers::ObjectPtrT<log4cxx::Logger> > && _Val={...}, std::_Nil _Newnode={...}) Line 1785 C++
  log4cxx.dll!log4cxx::Hierarchy::getLogger(const std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> > & name={...}, const log4cxx::helpers::ObjectPtrT<log4cxx::spi::LoggerFactory> & factory={...}) Line 224 C++
  log4cxx.dll!log4cxx::Hierarchy::getLogger(const std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> > & name={...}) Line 206 C++
> log4cxx.dll!log4cxx::LogManager::getLoggerLS(const std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> > & name={...}) Line 106 C++
  log4cxx.dll!log4cxx::LogManager::getLogger(const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & name={...}) Line 121 C++
  log4cxx.dll!log4cxx::Logger::getLogger(const char * const name=0x18622240) Line 496 C++
{noformat}

Hierarchy object seems to be destroyed already. That is possible in the case if {{LogManager::getLoggerRepository}} is not thread-safe.

> LogManager::getRootLogger is not thread-safe
> --------------------------------------------
>
>                 Key: LOGCXX-430
>                 URL: https://issues.apache.org/jira/browse/LOGCXX-430
>             Project: Log4cxx
>          Issue Type: Bug
>          Components: Core
>    Affects Versions: 0.10.0
>            Reporter: Thorsten Schöning
>            Assignee: Thorsten Schöning
>         Attachments: LOGCXX-430-with-mutex-1.patch, LOGCXX-430-with-mutex-2.patch, LOGCXX-430.patch
>
>
> 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:
> {CODE}
> RepositorySelectorPtr& LogManager::getRepositorySelector() {
>    //
>    //     call to initialize APR and trigger "start" of logging clock
>    //
>    APRInitializer::initialize();
>    static spi::RepositorySelectorPtr selector;
>    return selector;
> }
> {CODE}
> 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.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)