Skip to content

Commit 742532f

Browse files
rupprechtdvbuka
authored andcommitted
[ORC] Fix race when checking isComplete (llvm#165063)
After llvm#164340 there is a tsan race on `OutstandingSymbolsCount` when decrementing it in `notifySymbolMetRequiredState` vs reading it in `isComplete()`. Fix this by having `IL_emit` filter out non-completed queries when it has the lock to do so, and that way we avoid needing to call `isComplete()` later.
1 parent 262de0b commit 742532f

File tree

2 files changed

+14
-5
lines changed
  • llvm

2 files changed

+14
-5
lines changed

llvm/include/llvm/ExecutionEngine/Orc/Core.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1768,7 +1768,7 @@ class ExecutionSession {
17681768
// FIXME: We should be able to derive FailedSymsForQuery from each query once
17691769
// we fix how the detach operation works.
17701770
struct EmitQueries {
1771-
JITDylib::AsynchronousSymbolQuerySet Updated;
1771+
JITDylib::AsynchronousSymbolQuerySet Completed;
17721772
JITDylib::AsynchronousSymbolQuerySet Failed;
17731773
DenseMap<AsynchronousSymbolQuery *, std::shared_ptr<SymbolDependenceMap>>
17741774
FailedSymsForQuery;

llvm/lib/ExecutionEngine/Orc/Core.cpp

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2901,13 +2901,23 @@ ExecutionSession::IL_emit(MaterializationResponsibility &MR,
29012901

29022902
for (auto &SN : ER.Ready)
29032903
IL_collectQueries(
2904-
EQ.Updated, SN->defs(),
2904+
EQ.Completed, SN->defs(),
29052905
[](JITDylib::SymbolTableEntry &E) { E.setState(SymbolState::Ready); },
29062906
[](AsynchronousSymbolQuery &Q, JITDylib &JD,
29072907
NonOwningSymbolStringPtr Name, JITDylib::SymbolTableEntry &E) {
29082908
Q.notifySymbolMetRequiredState(SymbolStringPtr(Name), E.getSymbol());
29092909
});
29102910

2911+
// std::erase_if is not available in C++17, and llvm::erase_if does not work
2912+
// here.
2913+
for (auto it = EQ.Completed.begin(), end = EQ.Completed.end(); it != end;) {
2914+
if ((*it)->isComplete()) {
2915+
++it;
2916+
} else {
2917+
it = EQ.Completed.erase(it);
2918+
}
2919+
}
2920+
29112921
#ifdef EXPENSIVE_CHECKS
29122922
verifySessionState("exiting ExecutionSession::IL_emit");
29132923
#endif
@@ -3043,9 +3053,8 @@ Error ExecutionSession::OL_notifyEmitted(
30433053
}
30443054
}
30453055

3046-
for (auto &UQ : EmitQueries->Updated)
3047-
if (UQ->isComplete())
3048-
UQ->handleComplete(*this);
3056+
for (auto &UQ : EmitQueries->Completed)
3057+
UQ->handleComplete(*this);
30493058

30503059
// If there are any bad dependencies then return an error.
30513060
if (!BadDeps.empty()) {

0 commit comments

Comments
 (0)