[cfe-dev] [cxxabi] Thread-safe statics causing deadlocks

[Olivier Goffart via cfe-dev]

On Donnerstag, 21. Juli 2016 15:27:07 CEST Craig, Ben via cfe-dev wrote:
> On 7/21/2016 12:54 PM, Richard Smith wrote:
> > On Wed, Jul 20, 2016 at 12:43 PM, Craig, Ben via cfe-dev
> > 
> > <cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>> wrote:
> >     C++14 6.7 Declaration statement, clause 4 has the standardese for
> >     "Magic" / thread-safe statics. Footnote 91 says "The
> >     implementation must not introduce any deadlock around execution of
> >     the initializer."  I believe this is unimplementable.
> > 
> > While your interpretation is not unreasonable, I believe you've
> > misunderstood the meaning and intent of this footnote. Note that it
> > says *the implementation* must not introduce any deadlock -- that is,
> > there must not be any deadlock that is not implied by the program
> > semantics. The normative sentence preceding this footnote says "If
> > control enters the declaration concurrently while the variable is
> > being initialized, the concurrent execution shall wait for completion
> > of the initialization." The potential for deadlock in that rule is not
> > affected by the presence of this footnote, because that's deadlock
> > introduced by the language semantics, not deadlock introduced by the
> > implementation.
> > 
> > To understand the purpose of this footnote, you need to look at how
> > GCC 3.x implemented thread-safe local statics (prior to
> > standardization). They had a single, global, recursive mutex
> > protecting all local static initialization. This results in deadlock
> > *introduced by the implementation* if a static local variable's
> > initializer spawns and joins a thread, and that thread triggers
> > initialization of a different static local variable. It is
> > specifically that implementation strategy which is being called out as
> > non-conforming here.
> 
> Thanks for the clear explanation.  Would it be possible to tack on a few
> more words onto that footnote to clear that up?  My mistake reading of
> that note was basically "No deadlocks allowed", when my reading of the
> text should have been "No deadlocks allowed beyond the ones we just
> mandated".
> 
> Perhaps...
> "The implementation must not introduce any deadlocks around execution of
> the initializer that are not implied by the program semantics".
> Maybe the redundancy is against the C++ standards style, but it would
> make it a bit more obvious to readers of the spec that some deadlocks
> are not only possible, but required.


I have seen deadlocks being reported on OSX.
Consider this example

#include <thread>
#include <string>
#include <mutex>

using namespace std::chrono_literals;
std::mutex mtx;

int main() {
  // This initializes a static under the mutex
  std::thread t1([]{
    std::lock_guard<std::mutex> lock(mtx);
    std::this_thread::sleep_for(100ms);
    static std::string goo = "goo";
  });

  // This thread initializes another static whose ctor locks the same mutex
  std::thread t2([]{
    struct str {
      str() {
        std::this_thread::sleep_for(100ms);
        std::lock_guard<std::mutex> lock(mtx);
      }
    };
    static str s;
  });
  t1.join(); t2.join();
}


Apple's implementation[1] has a bug which would make this code deadlock as the 
same mutex is used and kept locked while initializing both statics. That's a 
classical deadlock with two mutexes locked in different order.
I believe this kind of deadlock is what the standard forbid.

I don't have OSX at hand so I could not verify this is still accurate. Please 
someone correct me if i'm wrong.

[1]
http://opensource.apple.com/source/libcppabi/libcppabi-26/src/cxa_guard.cxx

-- 
Olivier