[PATCH] D27179: LibFuzzer - Implement timers for Windows and improve synchronization.

[Marcos Pividori via Phabricator via llvm-commits]

mpividori added inline comments.


================
Comment at: lib/Fuzzer/FuzzerInternal.h:146-147
   std::atomic<size_t> CurrentUnitSize;
+  bool RunningCB;
+  std::mutex RunningCBMtx;
   uint8_t BaseSha1[kSHA1NumBytes];  // Checksum of the base unit.
----------------
zturner wrote:
> It seems like we could use `std::atomic_bool` here.  Does that not work for some reason?  The only thing I see that is questionable is that in `FuzzerCallback::CrashCallback` we check the value, and then run multiple lines of code before returning.
> 
> But on the other hand, in `Fuzzer::ExecuteCallback`, we always set it to true before executing any callback, and false after.  So maybe there is no race condition?
> 
> `atomic_bool` is more lightweight than grabbing a full blown mutex, so if we can be sure it's safe, then it seems better.
@zturner, thanks for your comments.
I can't use `std::atomic_bool` because not only do I need to be sure it is modified atomically, buy also I need to synchronize the main thread with the rest of the threads accessing the Fuzzer's data. Because of that I added the mutex `RunningCBMtx`.
All the information related to the state of the fuzzer is only modified by the main thread, when it is not running the callback function.
So, in order to consistently access to the Fuzzer's data, we should lock the `RunningCBMtx` and make sure `RunningCB` is true (the main thread is running the CB).
By locking the `RunningCBMtx`, we can be sure that the main thread won't change its state in `RunningCB`.
This is used to synchronize the thread which manages the timers, and the one which supervises rss limits, with the main thread.


Repository:
  rL LLVM

https://reviews.llvm.org/D27179