Skip to content

Commit

Permalink
[ORC] Destroy defunct MaterializationUnits outside the session lock.
Browse files Browse the repository at this point in the history
MaterializationUnits may contain arbitrary resources that need cleanup. We want
to do this outside the JIT's session lock.

This should fix a lock-order-inversion warning in clang-repl (for details see
llvm/llvm-project#124215).
  • Loading branch information
lhames committed Jan 24, 2025
1 parent 9fecb4f commit a001cc0
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 15 deletions.
9 changes: 7 additions & 2 deletions llvm/include/llvm/ExecutionEngine/Orc/Core.h
Original file line number Diff line number Diff line change
Expand Up @@ -1204,8 +1204,13 @@ class JITDylib : public ThreadSafeRefCountedBase<JITDylib>,

JITDylib(ExecutionSession &ES, std::string Name);

std::pair<AsynchronousSymbolQuerySet, std::shared_ptr<SymbolDependenceMap>>
IL_removeTracker(ResourceTracker &RT);
struct RemoveTrackerResult {
AsynchronousSymbolQuerySet QueriesToFail;
std::shared_ptr<SymbolDependenceMap> FailedSymbols;
std::vector<std::unique_ptr<MaterializationUnit>> DefunctMUs;
};

RemoveTrackerResult IL_removeTracker(ResourceTracker &RT);

void transferTracker(ResourceTracker &DstRT, ResourceTracker &SrcRT);

Expand Down
34 changes: 21 additions & 13 deletions llvm/lib/ExecutionEngine/Orc/Core.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1251,9 +1251,7 @@ JITDylib::JITDylib(ExecutionSession &ES, std::string Name)
LinkOrder.push_back({this, JITDylibLookupFlags::MatchAllSymbols});
}

std::pair<JITDylib::AsynchronousSymbolQuerySet,
std::shared_ptr<SymbolDependenceMap>>
JITDylib::IL_removeTracker(ResourceTracker &RT) {
JITDylib::RemoveTrackerResult JITDylib::IL_removeTracker(ResourceTracker &RT) {
// Note: Should be called under the session lock.
assert(State != Closed && "JD is defunct");

Expand Down Expand Up @@ -1292,7 +1290,10 @@ JITDylib::IL_removeTracker(ResourceTracker &RT) {
SymbolsToFail.push_back(Sym);
}

auto Result = ES.IL_failSymbols(*this, std::move(SymbolsToFail));
auto [QueriesToFail, FailedSymbols] =
ES.IL_failSymbols(*this, std::move(SymbolsToFail));

std::vector<std::unique_ptr<MaterializationUnit>> DefunctMUs;

// Removed symbols should be taken out of the table altogether.
for (auto &Sym : SymbolsToRemove) {
Expand All @@ -1302,7 +1303,12 @@ JITDylib::IL_removeTracker(ResourceTracker &RT) {
// Remove Materializer if present.
if (I->second.hasMaterializerAttached()) {
// FIXME: Should this discard the symbols?
UnmaterializedInfos.erase(Sym);
auto J = UnmaterializedInfos.find(Sym);
assert(J != UnmaterializedInfos.end() &&
"Symbol table indicates MU present, but no UMI record");
if (J->second->MU)
DefunctMUs.push_back(std::move(J->second->MU));
UnmaterializedInfos.erase(J);
} else {
assert(!UnmaterializedInfos.count(Sym) &&
"Symbol has materializer attached");
Expand All @@ -1313,7 +1319,8 @@ JITDylib::IL_removeTracker(ResourceTracker &RT) {

shrinkMaterializationInfoMemory();

return Result;
return {std::move(QueriesToFail), std::move(FailedSymbols),
std::move(DefunctMUs)};
}

void JITDylib::transferTracker(ResourceTracker &DstRT, ResourceTracker &SrcRT) {
Expand Down Expand Up @@ -2180,26 +2187,27 @@ Error ExecutionSession::removeResourceTracker(ResourceTracker &RT) {
});
std::vector<ResourceManager *> CurrentResourceManagers;

JITDylib::AsynchronousSymbolQuerySet QueriesToFail;
std::shared_ptr<SymbolDependenceMap> FailedSymbols;
JITDylib::RemoveTrackerResult R;

runSessionLocked([&] {
CurrentResourceManagers = ResourceManagers;
RT.makeDefunct();
std::tie(QueriesToFail, FailedSymbols) =
RT.getJITDylib().IL_removeTracker(RT);
R = RT.getJITDylib().IL_removeTracker(RT);
});

// Release any defunct MaterializationUnits.
R.DefunctMUs.clear();

Error Err = Error::success();

auto &JD = RT.getJITDylib();
for (auto *L : reverse(CurrentResourceManagers))
Err = joinErrors(std::move(Err),
L->handleRemoveResources(JD, RT.getKeyUnsafe()));

for (auto &Q : QueriesToFail)
Q->handleFailed(
make_error<FailedToMaterialize>(getSymbolStringPool(), FailedSymbols));
for (auto &Q : R.QueriesToFail)
Q->handleFailed(make_error<FailedToMaterialize>(getSymbolStringPool(),
R.FailedSymbols));

return Err;
}
Expand Down

0 comments on commit a001cc0

Please sign in to comment.