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

[Evgenii Stepanov via Phabricator via llvm-commits]

eugenis added inline comments.


================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_linux.cc:954
   }
-  return true;
+}
+
----------------
vitalybuka wrote:
> 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.
Yes, if the read stopped on a dead/dying thread.
No, if it stopped because it ran out of buffer space.
Basically, we want the final iteration to succeed in a single system call, without any dead threads, and without any threads that have not been stopped on previous iterations. Otherwise we don't know if we've stopped everything.


================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc:222
+      case ThreadLister::Incomplete:
         added_threads = true;
+        break;
----------------
vitalybuka wrote:
> 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.
Agreed, it can't hurt to stop what we can as soon as we can.


Repository:
  rL LLVM

https://reviews.llvm.org/D46517