Skip to content

Commit c84616c

Browse files
committed
[flang] Avoid potential deadlock in CloseAll()
When closing all open units, don't hold the unit map lock over the actual close operations; if one of those aborts, CloseAll() may be called and then deadlock. Differential Review: https://reviews.llvm.org/D115184
1 parent 622d689 commit c84616c

File tree

1 file changed

+16
-7
lines changed

1 file changed

+16
-7
lines changed

flang/runtime/unit-map.cpp

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -52,15 +52,24 @@ void UnitMap::DestroyClosed(ExternalFileUnit &unit) {
5252
}
5353

5454
void UnitMap::CloseAll(IoErrorHandler &handler) {
55-
CriticalSection critical{lock_};
56-
for (int j{0}; j < buckets_; ++j) {
57-
while (Chain * p{bucket_[j].get()}) {
58-
bucket_[j].swap(p->next); // pops p from head of list
59-
p->unit.CloseUnit(CloseStatus::Keep, handler);
60-
p->unit.~ExternalFileUnit();
61-
FreeMemory(p);
55+
// Extract units from the map so they can be closed
56+
// without holding lock_.
57+
OwningPtr<Chain> closeList;
58+
{
59+
CriticalSection critical{lock_};
60+
for (int j{0}; j < buckets_; ++j) {
61+
while (Chain * p{bucket_[j].get()}) {
62+
bucket_[j].swap(p->next); // pops p from head of bucket list
63+
closeList.swap(p->next); // pushes p to closeList
64+
}
6265
}
6366
}
67+
while (Chain * p{closeList.get()}) {
68+
closeList.swap(p->next); // pops p from head of closeList
69+
p->unit.CloseUnit(CloseStatus::Keep, handler);
70+
p->unit.~ExternalFileUnit();
71+
FreeMemory(p);
72+
}
6473
}
6574

6675
void UnitMap::FlushAll(IoErrorHandler &handler) {

0 commit comments

Comments
 (0)