Skip to content

Commit 1a53553

Browse files
authored
[ORC] Fix synchronization in CoreAPIsTest. (#144556)
The code previously appeared to have a (benign?) race condition on `WorkThreads.size`, since it was being accessed outside of the mutex lock that protected it on the threads. This is usually okay since 1a1d6e6, but doesn't seem reliable in general, so fix this code to express the intent more accurately. This instead relies on the same general principles as ref-counting, where each existing reference (thread) can add new references (threads) because they already have a reference themselves (until joined).
1 parent 71d6762 commit 1a53553

File tree

1 file changed

+10
-8
lines changed

1 file changed

+10
-8
lines changed

llvm/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1559,16 +1559,11 @@ TEST_F(CoreAPIsStandardTest, TestLookupWithThreadedMaterialization) {
15591559
#if LLVM_ENABLE_THREADS
15601560

15611561
std::mutex WorkThreadsMutex;
1562-
std::vector<std::thread> WorkThreads;
1562+
SmallVector<std::thread, 0> WorkThreads;
15631563
DispatchOverride = [&](std::unique_ptr<Task> T) {
1564-
std::promise<void> WaitP;
15651564
std::lock_guard<std::mutex> Lock(WorkThreadsMutex);
15661565
WorkThreads.push_back(
1567-
std::thread([T = std::move(T), WaitF = WaitP.get_future()]() mutable {
1568-
WaitF.get();
1569-
T->run();
1570-
}));
1571-
WaitP.set_value();
1566+
std::thread([T = std::move(T)]() mutable { T->run(); }));
15721567
};
15731568

15741569
cantFail(JD.define(absoluteSymbols({{Foo, FooSym}})));
@@ -1580,8 +1575,15 @@ TEST_F(CoreAPIsStandardTest, TestLookupWithThreadedMaterialization) {
15801575
EXPECT_EQ(FooLookupResult.getFlags(), FooSym.getFlags())
15811576
<< "lookup returned incorrect flags";
15821577

1583-
for (auto &WT : WorkThreads)
1578+
std::unique_lock Lock(WorkThreadsMutex);
1579+
// This works because every child thread that is allowed to use WorkThreads
1580+
// must either be in WorkThreads or its parent must be in WorkThreads.
1581+
while (!WorkThreads.empty()) {
1582+
auto WT = WorkThreads.pop_back_val();
1583+
Lock.unlock();
15841584
WT.join();
1585+
Lock.lock();
1586+
}
15851587
#endif
15861588
}
15871589

0 commit comments

Comments
 (0)