Static locals within log4cxx cause SEGFAULTs

Good day,

I believe there's a problem with the use of static local variables w/non-trivial constructors within
log4cxx code (bug LOG4CXX-159, http://issues.apache.org/jira/browse/LOGCXX-159 ). Log4cxx
contains things like this (from ndc.cpp):

const LogString& NDC::getNull() {
    static LogString nullStr(LOG4CXX_STR("null"));
    return nullStr;
}

I think there are two major problems with this (and similar) code:

1) I think it is not thread safe. Local statics are initialized the first time the enclosing function is
called (C++'98 ISO standard, section 6.7). If two threads call it at about the same time from different
threads, they may both try to initialize the static local at once, effectively executing the object's
constructor on the same object simultaneously. [I vaguely remember reading something about Visual C++
generating code that ensures this doesn't happen, using mutexes or something... even if it's true, other
compilers most likely wouldn't do that].

2) The order of construction and (especially) destruction of such local statics is ... "interesting",
esp. w.r.t. static objects defined in other object files. LOG4CXX-159 contains a couple of example
programs that demonstrate this, but the bottom line is, such local static variables are sometimes
destructed before static objects in user code are, which in my case caused SEGFAULTs.

I understand static locals were introduced in favor of globals to make sure they are initialized as
necessary... however there are still instances where global statics are used (Level::ALL, etc); are
there any plans to do anything about them?

Also, as suggested by Curt in that JIRA discussion, arranging a call to something like

Re: Static locals within log4cxx cause SEGFAULTs

Davlet Panech wrote:
> Good day,
> 
> I believe there's a problem with the use of static local variables
> w/non-trivial constructors within log4cxx code (bug LOG4CXX-159,
> http://issues.apache.org/jira/browse/LOGCXX-159 ). Log4cxx contains
> things like this (from ndc.cpp):
> 
> const LogString& NDC::getNull() { static LogString
> nullStr(LOG4CXX_STR("null")); return nullStr; }
> 
> I think there are two major problems with this (and similar) code:

[two major problems omitted]

I'd like to just add a minor point against these statics: when you use a
simple and cheap leak detector tool like the debug heap of visual
studio, all these statics show up as leaks and make it difficult to
detect real leaks in the forest of warnings.

To solve this problem it would probably enough to have some 'uninit'
function.

Regards, Peter
--

-- 
    _   _        Peter Steiner <peter.steiner <at> hugwi.ch>
  / /_/ /        Hug-Witschi AG <http://www.hugwi.ch/>
 /  _  /         Electronic Engineering
/_/ /_/  _   _   Auriedstrasse 10
   / / / / / /   CH-3178 Boesingen

Re: Static locals within log4cxx cause SEGFAULTs

Davlet Panech <dp1978x_lists <at> yahoo.ca> writes:
> 
> Good day,
> 
> I believe there's a problem with the use of static local variables
> w/non-trivial constructors within log4cxx code
> [...]

I have a patch (attached to JIRA bug LOGCXX-159, 
http://issues.apache.org/jira/browse/LOGCXX-159 ) that solves some of these 
problems: it calls most, but not all, functions w/static locals from a 
constructor of a static object ("helpers::StaticInitializer") that is forced 
into each (user) translation unit via #include <log4cxx/log4cxx.h>. 
This "initialization" is protected with a reference count and happens only on 
the first call.

This solves my immediate problems, but some issues still remain:

(1) Most classes implement reflection (??? I think), via #define 
IMPLEMENT_LOG4CXX_OBJECT, that macro contains function(s) with static locals. 
The patch I submitted doesn't do anything about those -- one would have to 
explicitely "initialize" each and every class.

(2) There are still a few instances of global statics (Level:ALL() etc), that 
I think are prone to the order of initialization problem.

(3) Each instance of a static local should really be isolated into a simple 
function that doesn't do much beyond returning a reference to that static. 
Otherwise initialization is unnecessarily expensieve since it has to execute 
unreleated code in order for control to pass through a static local 

Is this forum/library alive?

Hi All,
I have posted a message a month ago - no response....
I just want to know, is there any other forum where there is more traffic,
or if this library is dead 
Any comment will be appreciated...
Thanks,
Ron.
--

-- 
View this message in context: http://www.nabble.com/Is-this-forum-library-alive--tf2726935.html#a7605090
Sent from the Log4cxx - Dev mailing list archive at Nabble.com.

RE: Is this forum/library alive?

I think it's dead... I know it's buggy!

I moved to log4cpp months ago and it works much better.

-----Original Message-----
From: Ron Hashimshony [mailto:ronhash <at> gmail.com] 
Sent: 29 novembre 2006 13:36
To: log4cxx-dev <at> logging.apache.org
Subject: Is this forum/library alive?

Hi All,
I have posted a message a month ago - no response....
I just want to know, is there any other forum where there is more traffic,
or if this library is dead 
Any comment will be appreciated...
Thanks,
Ron.
--

-- 
View this message in context:
http://www.nabble.com/Is-this-forum-library-alive--tf2726935.html#a7605090
Sent from the Log4cxx - Dev mailing list archive at Nabble.com.

Re: Is this forum/library alive?

RE: Is this forum/library alive?

Re: Is this forum/library alive?

Re: Leak in apr_pools when using RollingFileAppender

Replying to my own mail in log4cxx-user (which should have gone to
log4cxx-dev):

My original mail didn't get all details right:

It's not a leak and it's not in apr_pools. But it happens when using a
RollingFileAppender.

It's not a leak because the memory is properly given back when the
application terminates (but only then).

It's the mutexes of some helper object used when the RollingFileAppender
does a rollover. These mutexes are allocated out of the "root pool" of
log4cxx instead out of a short lived pool. See
http://issues.apache.org/jira/browse/LOGCXX-161 for more details.

I propose a patch to fix this (see log4cxx-161-mutex.patch of LOGCXX-161).

There are two more patches attached to the JIRA issue. They are not
related to RollingFileAppender and optimise log4cxx (I hope). As for the
default threshold of 40960 in log4cxx-161-threshold.patch: this is an
arbitrary number that works fine for my application, I haven't done any
tests to back it up.

Regards, Peter

Peter Steiner wrote:
> I'm using the SVN version of log4cxx (with a patched 
> propertyconfigurator, see
> http://issues.apache.org/jira/browse/LOGCXX-102).
> 
> I have observed that our application increases its working set in 8k 
> steps. I have tracked it down to happen whenever the log file rolls
> over:
> 
> log4cxx::rolling::RollingFileAppender::rollover() (Line 153) calls 
> WriterAppender::closeWriter() which allocates a new APR pool.
> 
> I think the error ultimately is in allocator_free() in apr_pools.c
> and was already discussed by Brian Pane in dev <at> httpd.apache.org (see 
> http://mail-archives.apache.org/mod_mbox/httpd-dev/200512.mbox/%3CA99202F3-A131-4656-8A68-7574C9E9062F <at> apache.org%3E
>  for the message). I couldn't find any correction for this in the APR
>  trunk, however.
> 
> In my case I have observed when stepping through the code that after
> the allocation of the pool in WriterAppender::closeWriter(), the next
> call to allocator_free() comes from a FileOutputStream destructor and
> only after that the pool from closeWriter is freed.
> 
> The second call to allocator_free has a current_free_index of 
> (unsigned)-1 and then overwrites the node pointer in allocator->free,
> so that the first of these nodes will never be freed.
> 
> 
> That said I'm not sure if it makes sense that closeWriter allocates
> its own pool. Reading 
> http://svn.apache.org/viewvc/apr/apr/trunk/docs/pool-design.html?view=co
>  I ask myself if closeWriter should not take a pool parameter (like 
> rollover already does) instead of having its own.
> 
> Regards, Peter

--

-- 
    _   _        Peter Steiner <peter.steiner <at> hugwi.ch>
  / /_/ /        Hug-Witschi AG <http://www.hugwi.ch/>
 /  _  /         Electronic Engineering
/_/ /_/  _   _   Auriedstrasse 10
   / / / / / /   CH-3178 Boesingen
  / /_/ /_/ /    Tel  +41 31 740 44 44
 /_ _ _ _ _/     Fax  +41 31 740 44 45