[PATCH] D46517: [sanitizer] Don't miss threads by ThreadSuspender

[Vitaly Buka via Phabricator via llvm-commits]

vitalybuka created this revision.
vitalybuka added a reviewer: eugenis.
Herald added a subscriber: kubamracek.

For process which continuously creates/destriy threads there is a possibility of
false memory leaks.
Enumerating /proc/<pid>/task/ may return just a small sublet of existing threads.
If threads are being destroyed during enumeration kernel may return with short
read and then continue skipping some random threads.
Usually the next enumeration returns the full list or at least a few additional
threads.

For the current know reproducer either helps:

1. doing 3+ iteration
2. iterating until we get persistent list

For each approach the is no guaranty that this is the full list, so I keep both
to reduce probability of miss.


Repository:
  rL LLVM

https://reviews.llvm.org/D46517

Files:
  compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc


Index: compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc
===================================================================
--- compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc
+++ compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc
@@ -209,27 +209,25 @@
 
 bool ThreadSuspender::SuspendAllThreads() {
   ThreadLister thread_lister(pid_);
-  bool added_threads;
-  bool first_iteration = true;
+  InternalMmapVector<int> prev;
+  prev.reserve(128);
   InternalMmapVector<int> threads;
   threads.reserve(128);
-  do {
-    // Run through the directory entries once.
-    added_threads = false;
+  // We can't get consistent thread list from /proc/, so to avoid missing
+  // threads we will try to list them at least 3 times after we suspended new
+  // one or received different list from the lister.
+  // TODO(vitalybuka): Start with the current content of the thread registry.
+  for (int i = 0; i < 3; ++i) {
+    bool added_threads = false;
     if (!thread_lister.ListThreads(&threads)) {
       ResumeAllThreads();
       return false;
     }
     for (int tid : threads)
-      if (SuspendThread(tid))
-        added_threads = true;
-    if (first_iteration && !added_threads) {
-      // Detach threads and fail.
-      ResumeAllThreads();
-      return false;
-    }
-    first_iteration = false;
-  } while (added_threads);
+      if (SuspendThread(tid)) added_threads = true;
+    if (added_threads || prev != threads) i = -1;
+    prev.swap(threads);
+  }
   return true;
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D46517.145436.patch
Type: text/x-patch
Size: 1560 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180507/16cff3df/attachment.bin>