forked from OSchip/llvm-project
[ORC] Use finer-grained and session locking in MachOPlatform to avoid deadlock.
In MachOPlatform, obtaining the link-order for a JITDylib requires locking the session, but also needs to be part of a larger atomic operation that collates initializer symbols tracked by the platform. Trying to do this under a separate platform mutex leads to potential locking order issues, e.g. T1 locks session then tries to lock platform to register a new init symbol meanwhile T2 locks platform then tries to lock session to obtain link order. Removing the platform lock and performing all these operations under the session lock eliminates this possibility. At the same time we also need to collate init pointers from the MachOPlatform::InitScraperPlugin, and we don't need or want to lock the session for that. The new InitSeqMutex has been added to guard these init pointers, and the session mutex is never obtained while the InitSeqMutex is held.
This commit is contained in:
parent
a7b8393ffe
commit
eb918d8daf
|
@ -143,12 +143,15 @@ private:
|
||||||
MachOJITDylibInitializers::SectionExtent ObjCSelRefs,
|
MachOJITDylibInitializers::SectionExtent ObjCSelRefs,
|
||||||
MachOJITDylibInitializers::SectionExtent ObjCClassList);
|
MachOJITDylibInitializers::SectionExtent ObjCClassList);
|
||||||
|
|
||||||
std::mutex PlatformMutex;
|
|
||||||
ExecutionSession &ES;
|
ExecutionSession &ES;
|
||||||
ObjectLinkingLayer &ObjLinkingLayer;
|
ObjectLinkingLayer &ObjLinkingLayer;
|
||||||
std::unique_ptr<MemoryBuffer> StandardSymbolsObject;
|
std::unique_ptr<MemoryBuffer> StandardSymbolsObject;
|
||||||
|
|
||||||
DenseMap<JITDylib *, SymbolLookupSet> RegisteredInitSymbols;
|
DenseMap<JITDylib *, SymbolLookupSet> RegisteredInitSymbols;
|
||||||
|
|
||||||
|
// InitSeqs gets its own mutex to avoid locking the whole session when
|
||||||
|
// aggregating data from the jitlink.
|
||||||
|
std::mutex InitSeqsMutex;
|
||||||
DenseMap<JITDylib *, MachOJITDylibInitializers> InitSeqs;
|
DenseMap<JITDylib *, MachOJITDylibInitializers> InitSeqs;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|
|
@ -163,7 +163,6 @@ Error MachOPlatform::notifyAdding(JITDylib &JD, const MaterializationUnit &MU) {
|
||||||
if (!InitSym)
|
if (!InitSym)
|
||||||
return Error::success();
|
return Error::success();
|
||||||
|
|
||||||
std::lock_guard<std::mutex> Lock(PlatformMutex);
|
|
||||||
RegisteredInitSymbols[&JD].add(InitSym);
|
RegisteredInitSymbols[&JD].add(InitSym);
|
||||||
LLVM_DEBUG({
|
LLVM_DEBUG({
|
||||||
dbgs() << "MachOPlatform: Registered init symbol " << *InitSym << " for MU "
|
dbgs() << "MachOPlatform: Registered init symbol " << *InitSym << " for MU "
|
||||||
|
@ -187,11 +186,10 @@ MachOPlatform::getInitializerSequence(JITDylib &JD) {
|
||||||
std::vector<JITDylib *> DFSLinkOrder;
|
std::vector<JITDylib *> DFSLinkOrder;
|
||||||
|
|
||||||
while (true) {
|
while (true) {
|
||||||
// Lock the platform while we search for any initializer symbols to
|
|
||||||
// look up.
|
|
||||||
DenseMap<JITDylib *, SymbolLookupSet> NewInitSymbols;
|
DenseMap<JITDylib *, SymbolLookupSet> NewInitSymbols;
|
||||||
{
|
|
||||||
std::lock_guard<std::mutex> Lock(PlatformMutex);
|
ES.runSessionLocked([&]() {
|
||||||
DFSLinkOrder = getDFSLinkOrder(JD);
|
DFSLinkOrder = getDFSLinkOrder(JD);
|
||||||
|
|
||||||
for (auto *InitJD : DFSLinkOrder) {
|
for (auto *InitJD : DFSLinkOrder) {
|
||||||
|
@ -201,7 +199,7 @@ MachOPlatform::getInitializerSequence(JITDylib &JD) {
|
||||||
RegisteredInitSymbols.erase(RISItr);
|
RegisteredInitSymbols.erase(RISItr);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
});
|
||||||
|
|
||||||
if (NewInitSymbols.empty())
|
if (NewInitSymbols.empty())
|
||||||
break;
|
break;
|
||||||
|
@ -228,7 +226,7 @@ MachOPlatform::getInitializerSequence(JITDylib &JD) {
|
||||||
// Lock again to collect the initializers.
|
// Lock again to collect the initializers.
|
||||||
InitializerSequence FullInitSeq;
|
InitializerSequence FullInitSeq;
|
||||||
{
|
{
|
||||||
std::lock_guard<std::mutex> Lock(PlatformMutex);
|
std::lock_guard<std::mutex> Lock(InitSeqsMutex);
|
||||||
for (auto *InitJD : reverse(DFSLinkOrder)) {
|
for (auto *InitJD : reverse(DFSLinkOrder)) {
|
||||||
LLVM_DEBUG({
|
LLVM_DEBUG({
|
||||||
dbgs() << "MachOPlatform: Appending inits for \"" << InitJD->getName()
|
dbgs() << "MachOPlatform: Appending inits for \"" << InitJD->getName()
|
||||||
|
@ -251,7 +249,7 @@ MachOPlatform::getDeinitializerSequence(JITDylib &JD) {
|
||||||
|
|
||||||
DeinitializerSequence FullDeinitSeq;
|
DeinitializerSequence FullDeinitSeq;
|
||||||
{
|
{
|
||||||
std::lock_guard<std::mutex> Lock(PlatformMutex);
|
std::lock_guard<std::mutex> Lock(InitSeqsMutex);
|
||||||
for (auto *DeinitJD : DFSLinkOrder) {
|
for (auto *DeinitJD : DFSLinkOrder) {
|
||||||
FullDeinitSeq.emplace_back(DeinitJD, MachOJITDylibDeinitializers());
|
FullDeinitSeq.emplace_back(DeinitJD, MachOJITDylibDeinitializers());
|
||||||
}
|
}
|
||||||
|
@ -285,7 +283,7 @@ void MachOPlatform::registerInitInfo(
|
||||||
MachOJITDylibInitializers::SectionExtent ModInits,
|
MachOJITDylibInitializers::SectionExtent ModInits,
|
||||||
MachOJITDylibInitializers::SectionExtent ObjCSelRefs,
|
MachOJITDylibInitializers::SectionExtent ObjCSelRefs,
|
||||||
MachOJITDylibInitializers::SectionExtent ObjCClassList) {
|
MachOJITDylibInitializers::SectionExtent ObjCClassList) {
|
||||||
std::lock_guard<std::mutex> Lock(PlatformMutex);
|
std::lock_guard<std::mutex> Lock(InitSeqsMutex);
|
||||||
|
|
||||||
auto &InitSeq = InitSeqs[&JD];
|
auto &InitSeq = InitSeqs[&JD];
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue