amccarth added a comment.

This patch was reverted a while back because a couple DirectoryWatcher tests 
routinely timed out on build bots even though they work when run in isolation.

I believe the problem is that, on a machine busy doing a build, the startup of 
the watcher thread is delayed (either because the scheduler algorithm or the 
thread pool policy).  Thus the "initial scan" on the test thread can complete 
_before_ the watcher thread has called ReadDirectoryChangesW.  This leaves a 
window of time where the test can change a file that the watcher thread will 
miss.

To test this hypothesis, I took this patch and created one more event called 
`WatcherThreadReady`.  I have the watcher thread set this event after 
successfully calling ReadDirectoryChangesW.  In the constructor, I changed:

  if (WaitForInitialSync)
    InitialScan();

to

  if (WaitForInitialSync) {
    ::WaitForSingleObject(WatcherThreadReady, 10000);
    InitialScan();
  }

This is crude, but it seems to be effective.  The tests pass reliably for me 
when my machine is fully loaded.  I didn't use an INFINITE timeout because it 
seems possibly missing a file change is less bad than hanging forever.  I 
didn't even bother to check the result from the wait because there's nothing 
sane to do besides "best effort" if something goes wrong.  I used a Windows 
event because those are most familiar to me and it's Windows-only code.   But 
it certainly could be done with other type of synchronization object.

There may be more elegant ways to solve this, but something like this directly 
addresses the root cause with fairly minimal changes.

I wonder if the Linux and Mac implementations might suffer from a similar 
window but the bug is rare because of differences in thread scheduler.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88666/new/

https://reviews.llvm.org/D88666

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to