Re: [ROOT] Some observations on thread safety

From: Fons Rademakers (Fons.Rademakers@cern.ch)
Date: Mon Jun 23 2003 - 12:02:30 MEST


Note however that there are still many more issues with threads and ROOT
collections. For example two threads modifying the same collections is
not protected. It is debatable if the collections should be thread safe
or that the app using the collections should prevent its threads from
updating the same collections without lock.

Cheers, Fons.



On Mon, 2003-06-23 at 10:14, Rene Brun wrote:
> Hi Walter,
> 
> I have added a LOCKGUARD in the TSeqCollection::QSort functions.
> 
> About Form, as you say, this is a bit more complex. One could also
> add a LOCKGUARD in FORM for the current value of bfree, but this would
> add a non-negligible performance penalty for many applications
> not using Threads (Root is compiled by default with the Thread option).
> A clean solution requires more thinking.
> 
> Thanks for sending these comments.
> 
> Rene Brun
> 
> "Walter F.J. Mueller" wrote:
> > 
> > Dear ROOTers,                                   using 3.05.05
> > 
> > there has been many times on this list and otherwise discussions on
> > whether ROOT is thread-safe (or under what precautions it might be)
> > and what additional measures (like locks) have to be added to achieve
> > thread-safety.
> > 
> > More by accident I found two central functions in ROOT which are as
> > far as I an see currently thread-unsafe and consequently render
> > everything using them also thread-unsafe.
> > 
> > Case 1:
> > 
> >    The two TSeqCollection::QSort() methods declare local variables as
> >    'static', the comment states "to save stack space".  This method is
> >    used to implement Sort() in TClonesArray, TObjArray, and
> >    TOrdCollection.
> > 
> >    When the Sort() method is called for these classes in two different
> >    threads the result can to be corrupted.
> > 
> > Case 2:
> > 
> >    The usage of the Form() global method provided by TString is
> >    inherently thread-unsafe. It relies on a small and predictable
> >    usage time of the char* pointer returned by this method, it has
> >    to be smaller than the time it takes to re-fill the circular buffer.
> > 
> >    If Form() is used in two threads this can't be guaranteed anymore.
> >    A context switch right after a Form() call followed by Form()
> >    calls in another thread may render the pointer invalid before it
> >    is used.
> > 
> >    A simple find $ROOTSYS -name "*.cxx" | xargs egrep -l "Form\(" | wc
> >    shows that Form() is used in 56 source files, so quite a few classes
> >    are affected.
> > 
> > Case 1 is trivial to cure, with todays memory sizes this hack surely
> > isn't a need. Case 2 can as far as I see only be cured by changing
> > the interface, e.g. eliminating the circular buffer and returning a
> > string or TString object by value.
> > 
> > Too me it seems that there is quite a way to go before ROOT can be
> > considered thread-safe, and that a thorough code review is probably
> > one of things to be done on this way.
> > 
> >                 With best regards,
> > 
> >                           Walter
-- 
Org:    CERN, European Laboratory for Particle Physics.
Mail:   1211 Geneve 23, Switzerland
E-Mail: Fons.Rademakers@cern.ch              Phone: +41 22 7679248
WWW:    http://root.cern.ch/~rdm/            Fax:   +41 22 7679480



This archive was generated by hypermail 2b29 : Thu Jan 01 2004 - 17:50:12 MET