[prev in list] [next in list] [prev in thread] [next in thread] 

List:       log4cxx-dev
Subject:    Re: Crashes on exit() from multithreaded program using log4cxx
From:       Aleksandr Vinokurov <aleksandr.vin () gmail ! com>
Date:       2010-01-22 16:11:57
Message-ID: CCCA6375-9A51-4E16-8050-3529C26C8850 () gmail ! com
[Download RAW message or body]



Hello, fellows,

I have this issue in situation when exitting an app that manually load  
a shared object that links to lilog4cxx.so and then manually unload it  
right before performing an exit() call. Quite similar to what Rhosyn  
says. For me it appears on Itanium arch that runs HP-UX. But I think  
that Itanium ABI's __cxa_atexit() addresses this issue. I have no  
ability to check it for GCC (see \
http://forums11.itrc.hp.com/service/forums/questionanswer.do?admit=109447626+1264172397670+28353475&threadId=1370786) \
 , but native HP-UX C++ compiler make it right.

So I consider using -fuse-cxa-atexit for g++ will help.

Sorry if my approach fails -- I try to post a reply for the email I  
have yanked from the archive to keep the hier.

With best wishes,
Aleksandr Vinokurov.

On Tue, 20 Jan 2009 00:06:23 GMT, Rhosyn wrote:
> Curt Arnold wrote:
> > 
> > On Jan 16, 2009, at 9:34 AM, Rhosyn wrote:
> > 
> > > Dear all,
> > > 
> > > 
> > > The crashes
> > > ===========
> > > 
> > > We would like to share with you some patches to log4cxx  we have  
> found
> > > useful in our environment.
> > > 
> > > Our product  (a Linux platform server product, compiled with g++  
> 4.3.x)
> > > started suffering from crashes during (what should have been)  
> graceful
> > > process exit ("exit(0);")- after we replaced our old logging  
> framework
> > > with a log4cxx backend.
> > > 
> > > Commonly we would see segmentation faults in:
> > > 
> > > * log4cxx::helpers::ObjectPtrBase()
> > > * log4cxx::LogManager::getLoggerLS()
> > > 
> > > 
> > > 
> > > The cause
> > > =========
> > > 
> > > We eventually traced this issue to the pervasive pattern of  
> (mis)usage
> > > of singletons in the log4cxx code.
> > > 
> > > The log4cxx library uses the "Meyers" singleton pattern, first
> > > popularised in Scott Meyers "Effective C++" (Item 47 in the 2nd  
> edition
> > > of that book):
> > > 
> > > Thing & getThingSingleton()
> > > {
> > > static Thing t;
> > > return t;
> > > }
> > > 
> > > 
> > > For many years, the above pattern was considered "best practice"  
> for
> > > using Singletons in C++ - and was generally safe for most popular
> > > compiler implementations and most applications.
> > > 
> > > Unfortunately, this recommendation is not actually guaranteed to be
> > > thread-safe for construction or destruction - something which is  
> alluded
> > > to on Scott Meyers' own "Errata List for Effective C++, Second  
> Edition"
> > > as described here: http://www.aristeia.com/BookErrata/ec++2e-errata.html
> > > 
> > > 
> > > The nub of the problem is that when a process calls "exit(0);" or
> > > similar, one thread will start running, in order, any user- 
> registered
> > > "atexit" functions.
> > > 
> > > Along with these, the compiler will execute the (conceptually  
> similar)
> > > compiler-registered functions which invoke the destructors of any  
> static
> > > file or function scope objects (also in order - the reverse order  
> to
> > > static object construction).
> > > 
> > > Unfortunately, other threads may still be in the process of  
> running -and
> > > logging, perhaps using the static objects - during or after the
> > > execution of their destructors - thus opening the door to a bunch  
> of
> > > potential SEGFAULTs.
> > > 
> > > 
> > > Solutions
> > > =========
> > > 
> > > Andrei Alexandrescu goes into a lot of detail about this C++ design
> > > problem in chapter 6 of "Modern C++ design" and proposes two  
> elegant
> > > solutions -  a "Phoenix Singleton" or  SingletonHolder class.
> > > 
> > > The patches attached are somewhat less elegant than either of
> > > Alexandrescus suggestions (we were pressed for time and needed a  
> quick
> > > fix in order to ship on time).
> > > 
> > > However, the patches supplied are simple, pragmatic - and did  
> appear to
> > > hold up during our testing (testing which was readily producing the
> > > crashes described earlier, before we patched).
> > > 
> > > In summary, the patches change Singleton functions to work thus:
> > > 
> > > Thing & getThingSingleton()
> > > {
> > > static Thing * t = new t;
> > > return *t;
> > > }
> > > 
> > > Of course, the downside of this flavour of fix is that:
> > > 
> > > * the static objects - now allocated on the heap with new() -  
> never get
> > > their destructors run.  AFAIK, no other resources (other than  
> memory)
> > > appear to be leaked (due to this patch).  Fortunately, as we are  
> running
> > > on a modern OS, we are able to rely on the OS to reclaim the  
> process
> > > memory on process exit - thus nullifying this particular issue  
> for our
> > > product (but not neccessarily so in other environments).
> > > 
> > > * The patch doesn't address the startup/initialisation race  (for  
> that
> > > we'd need a multiple-locked singleton initialization pattern  
> everywhere
> > > log4cxx creates a singleton) - we're less worried about that at  
> this
> > > stage as we have yet to notice any issues.
> > > 
> > > apr
> > > ===
> > > 
> > > Finally we couldn't work out how it could ever be safe to  
> deiinitialise
> > > APR if there was even the slightest chance that any extant log4cxx
> > > objects existed (accessible to any thread).  We therefore removed  
> the
> > > apr_terminate() call in APRInitializer::~APRInitializer()
> > > 
> > > 
> > > Future
> > > ======
> > > 
> > > I would like to respecfully suggest that there is a discussion in  
> the
> > > log4cxx community about  the best way of reworking the use of  
> singletons
> > > in the log4cxx library  (multithreaded-safe construction and
> > > destruction)  - and that we look to moving towards a different  
> pattern
> > > of usage.
> > > 
> > > I suspect Alexandrescu's "singleton holder" idea might form a  
> part of a
> > > possible solution - but it's not the only game in town.
> > > 
> > > 
> > > 
> > 
> > Thanks for the analysis.
> > 
> > There is a bit of tension here since those who are running under
> > BoundsChecker, Valgrind, Purify et al will then complain about  
> leaks.
> > Probably the best approach is to try to isolate the singleton  
> pattern
> > into a preprocessor macro and then allow the user to select what
> > singleton pattern they'd like to use.
> > 
> > Please file this as a bug report at http://issues.apache.org/jira
> > 
> 
> 
> Thanks; JIRA issue LOGCXX-322 raised, as requested.
> 
> 
> I totally see your point re: BoundsChecker, Valgrind etc. and also re:
> isolating the behaviour.
> 
> A more sophisticated patch than my first cut is definitely the order  
> of
> the day!


[prev in list] [next in list] [prev in thread] [next in thread]