[PATCH] libc++ tests: wait_until.pass test sporadically fails (bug 21998)

[Eric Fiselier]

Adding the appropriate reviewers and putting in my two cents.


REPOSITORY
  rL LLVM

================
Comment at: test/std/thread/futures/futures.shared_future/wait_until.pass.cpp:86-88
@@ +85,5 @@
+        std::shared_future<T> f = p.get_future();
+        std::thread(func1, std::move(p)).detach();
+        assert(f.valid());
+        wait_for_thread_state(WorkerThreadState::Started);
+        assert(f.wait_until(Clock::now() + ms(10)) == std::future_status::timeout);
----------------
While this patch may fix the race condition that you and others have seen I think it introduces the possibility that the test could hang.

Isn't in possible (although very unlikely) that `func1` will execute lines 48-53 and set `thread_state = Exiting` before we enter line 88?
It seems we depend on `std::this_thread::sleep_for(ms(500))` to prevent this from happening. Please let me know if I am mistaken.

With our change the race condition now takes 500ms as opposed to 100ms (as mentioned in PR21998). If we are going to depend on the call to `std::this_thread::sleep_for` at all then why not depend on it entirely and increase the amount of time we sleep for? At least that way we prevent these tests from becoming dependent on `std::mutex`, `std::unique_lock` and `std::condition_variable`. 

I'm also a little uncomfortable using explicit locking when testing `<future>`. My concern is that it might hide race conditions that would otherwise be detectable from sporadic test failures or instrumentation like thread sanitizer. However this concern could be entirely unfounded.

http://reviews.llvm.org/D6750

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/