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

[Vitaly Buka via Phabricator via llvm-commits]

vitalybuka marked 2 inline comments as done.
vitalybuka added inline comments.


================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_file.cc:169
+  return true;
+}
+
----------------
eugenis wrote:
> I don't like this code duplication at all.
Let me resolve this a separate patch.


================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_linux.cc:954
   }
-  return true;
+}
+
----------------
eugenis wrote:
> Would it be better to resize the buffer and start from scratch until we get the entire directory in one go? Otherwise we can still miss a thread that was added before the current directory cursor, I guess - or is that impossible?
If it's not asan (thread registry is blocked there) threads may continue create new threads. So better to stop whatever is known.


================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc:222
+      case ThreadLister::Incomplete:
         added_threads = true;
+        break;
----------------
eugenis wrote:
> Is this because you want to suspend threads even when you've got an incomplete response? If would be cleaner to just loop while(Incomplete) ListThreads() and then handle the other two return codes.
It would be cleaner, but not sure about probability of issues.
Also if you just reread immediatly, it will likely return same list because the dying thread is still there. Suspending threads we give some time to kernel to remove it from list.


Repository:
  rL LLVM

https://reviews.llvm.org/D46517