[
https://issues.apache.org/jira/browse/LOGCXX-430?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13967570#comment-13967570
]
Claudio Corsi commented on LOGCXX-430:
--------------------------------------
Thorsten,
I just looked at the patch and this will work but it will generate a memory
leak since you are not creating a new instance every time that this method is
called.
You can get around this issue by creating static method that includes the
original creation of the selector pointer instance. The selector can be
assigned using the static method. Since the selector field is static this will
happen once only throughout the life of the process. You could of keep the
code the same exception change the following:
static spi::RepositorySelectorPtr selector;
to
static spi::RepositorySelectorPtr selector =
LogManager::createRepositorySelector();
The LogManager::createRepositorySelector() is a static method and will be
called only once.
This will make the change thread safe since any static initialization is thread
safe and will only create a single instance instead of the multiple instances.
> 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
> Fix For: 0.11.0
>
> Attachments: 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:
> > 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.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
