[flang-commits] [flang] c84616c - [flang] Avoid potential deadlock in CloseAll()

[Peter Klausler via flang-commits]

Author: Peter Klausler
Date: 2021-12-07T09:42:48-08:00
New Revision: c84616c3b368f1ca679a08d2ce131f01a9f14ce9

URL: https://github.com/llvm/llvm-project/commit/c84616c3b368f1ca679a08d2ce131f01a9f14ce9
DIFF: https://github.com/llvm/llvm-project/commit/c84616c3b368f1ca679a08d2ce131f01a9f14ce9.diff

LOG: [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

Added: 
    

Modified: 
    flang/runtime/unit-map.cpp

Removed: 
    


################################################################################
diff  --git a/flang/runtime/unit-map.cpp b/flang/runtime/unit-map.cpp
index 497cef23d47c3..4c931871c0176 100644
--- a/flang/runtime/unit-map.cpp
+++ b/flang/runtime/unit-map.cpp
@@ -52,15 +52,24 @@ void UnitMap::DestroyClosed(ExternalFileUnit &unit) {
 }
 
 void UnitMap::CloseAll(IoErrorHandler &handler) {
-  CriticalSection critical{lock_};
-  for (int j{0}; j < buckets_; ++j) {
-    while (Chain * p{bucket_[j].get()}) {
-      bucket_[j].swap(p->next); // pops p from head of list
-      p->unit.CloseUnit(CloseStatus::Keep, handler);
-      p->unit.~ExternalFileUnit();
-      FreeMemory(p);
+  // Extract units from the map so they can be closed
+  // without holding lock_.
+  OwningPtr<Chain> closeList;
+  {
+    CriticalSection critical{lock_};
+    for (int j{0}; j < buckets_; ++j) {
+      while (Chain * p{bucket_[j].get()}) {
+        bucket_[j].swap(p->next); // pops p from head of bucket list
+        closeList.swap(p->next); // pushes p to closeList
+      }
     }
   }
+  while (Chain * p{closeList.get()}) {
+    closeList.swap(p->next); // pops p from head of closeList
+    p->unit.CloseUnit(CloseStatus::Keep, handler);
+    p->unit.~ExternalFileUnit();
+    FreeMemory(p);
+  }
 }
 
 void UnitMap::FlushAll(IoErrorHandler &handler) {