[flang-commits] [PATCH] D115184: [flang] Avoid potential deadlock in CloseAll()

[Peter Klausler via Phabricator via flang-commits]

klausler created this revision.
klausler added a reviewer: vdonaldson.
klausler added a project: Flang.
Herald added a subscriber: jdoerfert.
klausler requested review of this revision.

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.


https://reviews.llvm.org/D115184

Files:
  flang/runtime/unit-map.cpp


Index: flang/runtime/unit-map.cpp
===================================================================
--- flang/runtime/unit-map.cpp
+++ flang/runtime/unit-map.cpp
@@ -52,15 +52,24 @@
 }
 
 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) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D115184.392170.patch
Type: text/x-patch
Size: 1195 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/flang-commits/attachments/20211206/f93c86c7/attachment.bin>