Re: [ROOT] Some observations on thread safety

From: Rene Brun (Rene.Brun@cern.ch)
Date: Mon Jun 23 2003 - 10:14:08 MEST


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



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