Skip to content

Commit

Permalink
Protect IncrementalExecutor::m_AtExitFuncs with a spin lock.
Browse files Browse the repository at this point in the history
This prevents:

==4150== Possible data race during write of size 8 at 0xCC136F0 by thread #3
==4150== Locks held: none
==4150==    at 0x146EC214: llvm::SmallVectorTemplateBase<cling::IncrementalExecutor::CXAAtExitElement, false>::grow(unsigned long) (in /afs/cern.ch/cms/sw/ReleaseCandidates/volB/slc6_amd64_gcc491/lcg/root/6.02.00-lnjiaj2/lib/libCling.so)
==4150==    by 0x146E9DE3: cling::IncrementalExecutor::AddAtExitFunc(void (*)(void*), void*, cling::Transaction const*) (in /afs/cern.ch/cms/sw/ReleaseCandidates/volB/slc6_amd64_gcc491/lcg/root/6.02.00-lnjiaj2/lib/libCling.so)
==4150==    by 0x160A5044: ???
==4150==    by 0x160A67D6: ???
==4150==    by 0x160A6783: ???
==4150==    by 0x145E76D3: TCling::ExecuteWithArgsAndReturn(TMethod*, void*, void const**, int, void*) const (in /afs/cern.ch/cms/sw/ReleaseCandidates/volB/slc6_amd64_gcc491/lcg/root/6.02.00-lnjiaj2/lib/libCling.so)
  • Loading branch information
pcanal committed Mar 14, 2015
1 parent a15e8a5 commit 06c8b94
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 8 deletions.
27 changes: 19 additions & 8 deletions interpreter/cling/lib/Interpreter/IncrementalExecutor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
//------------------------------------------------------------------------------

#include "IncrementalExecutor.h"
#include "Threading.h"

#include "cling/Interpreter/Value.h"

Expand Down Expand Up @@ -66,6 +67,9 @@ std::vector<IncrementalExecutor::LazyFunctionCreatorFunc_t>
clang::DiagnosticsEngine& diags)
: m_Diags(diags) {
assert(m && "llvm::Module must not be null!");

// No need to protect this access of m_AtExitFuncs, since nobody
// can use this object yet.
m_AtExitFuncs.reserve(256);

// Rewrire __cxa_atexit to ~Interpreter(), thus also global destruction
Expand Down Expand Up @@ -118,6 +122,8 @@ std::vector<IncrementalExecutor::LazyFunctionCreatorFunc_t>
IncrementalExecutor::~IncrementalExecutor() {}

void IncrementalExecutor::shuttingDown() {
// No need to protect this access, since hopefully there is no concurrent
// shutdown request.
for (size_t I = 0, N = m_AtExitFuncs.size(); I < N; ++I) {
const CXAAtExitElement& AEE = m_AtExitFuncs[N - I - 1];
(*AEE.m_Func)(AEE.m_Arg);
Expand Down Expand Up @@ -166,6 +172,7 @@ void IncrementalExecutor::remapSymbols() {
void IncrementalExecutor::AddAtExitFunc(void (*func) (void*), void* arg,
const cling::Transaction* T) {
// Register a CXAAtExit function
cling::internal::SpinLockGuard slg(m_AtExitFuncsSpinLock);
m_AtExitFuncs.push_back(CXAAtExitElement(func, arg, T));
}

Expand Down Expand Up @@ -365,14 +372,18 @@ void IncrementalExecutor::runAndRemoveStaticDestructors(Transaction* T) {
assert(T && "Must be set");
// Collect all the dtors bound to this transaction.
AtExitFunctions boundToT;
for (AtExitFunctions::iterator I = m_AtExitFuncs.begin();
I != m_AtExitFuncs.end();)
if (I->m_FromT == T) {
boundToT.push_back(*I);
I = m_AtExitFuncs.erase(I);
}
else
++I;

{
cling::internal::SpinLockGuard slg(m_AtExitFuncsSpinLock);
for (AtExitFunctions::iterator I = m_AtExitFuncs.begin();
I != m_AtExitFuncs.end();)
if (I->m_FromT == T) {
boundToT.push_back(*I);
I = m_AtExitFuncs.erase(I);
}
else
++I;
} // end of spin lock lifetime block.

// 'Unload' the cxa_atexit entities.
for (AtExitFunctions::reverse_iterator I = boundToT.rbegin(),
Expand Down
9 changes: 9 additions & 0 deletions interpreter/cling/lib/Interpreter/IncrementalExecutor.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include <set>
#include <map>
#include <memory>
#include <atomic>

namespace clang {
class DiagnosticsEngine;
Expand Down Expand Up @@ -100,6 +101,14 @@ namespace cling {
const Transaction* m_FromT; //FIXME: Should be bound to the llvm symbol.
};

///\brief Atomic used as a spin lock to protect the access to m_AtExitFuncs
///
/// AddAtExitFunc is used at the end of the 'interpreted' user code
/// and before the calling framework has any change of taking back/again
/// its lock protecting the access to cling, so we need to explicit protect
/// again multiple conccurent access.
std::atomic_flag m_AtExitFuncsSpinLock = ATOMIC_FLAG_INIT;

typedef llvm::SmallVector<CXAAtExitElement, 128> AtExitFunctions;
///\brief Static object, which are bound to unloading of certain declaration
/// to be destructed.
Expand Down

0 comments on commit 06c8b94

Please sign in to comment.