Workaround ASAN race condition in future tests by manually joining threads.

[EricWF]

Summary

There is a race condition in ASAN where it can perform the leak detection before all detached threads exit and free their resources. This can lead to failures in the future tests.
This patch fixes the race condition my manually joining all threads that are used. This patch should not affect the functionality of the tests.

[EricWF]

The ASAN bug has been filed as PR21621.
http://llvm.org/bugs/show_bug.cgi?id=21621

[jroelofs]

So 4 things:

• I'm a little out of my comfort zone with this thread stuff... pretty far from my area of expertise
• Perhaps you missed a few, for example: thread/futures/futures.promise/set_value_at_thread_exit_void.pass.cpp (grep for 'detach' in tests).
• Doesn't this reduce the test coverage for the thread.detach() method?
• I *really* *really* don't like the idea of changing tests to push bugs under the rug. It'd be much better to 'XFAIL: asan' these. I have a thought on how to fix asan that I'll add to the PR.

Cheers,

Jon

[danalbert]

I *really* *really* don't like the idea of changing tests to push bugs under the rug. It'd be much better to 'XFAIL: asan' these. I have a thought on how to fix asan that I'll add to the PR.

Agreed.

[jroelofs]

Erm, nevermind, my idea won't work (for the record, it was something to do with -Wl,--wrap,atexit). This still makes me uncomfortable.

[EricWF]

In D6347#5, @jroelofs wrote:

So 4 things:

• I'm a little out of my comfort zone with this thread stuff... pretty far from my area of expertise
• Perhaps you missed a few, for example: thread/futures/futures.promise/set_value_at_thread_exit_void.pass.cpp (grep for 'detach' in tests).

I left those out on purpose since the value should not be set until the thread cleans up and exits. So there should be no race condition.

• Doesn't this reduce the test coverage for the thread.detach() method?

These tests are not intended to test thread.detach(). detach() should be thoroughly tested elsewhere but I'll double check.

• I *really* *really* don't like the idea of changing tests to push bugs under the rug. It'd be much better to 'XFAIL: asan' these. I have a thought on how to fix asan that I'll add to the PR.

Me too. The initial response to the PR was that this might not be fixable and we don't want tests that occasionally fail with ASAN. We can't XFAIL these tests because they pass 90% of the time. We could mark them as UNSUPPORTED but then you lose test coverage with ASAN. Since the above changes should not change the functionality of the tests I think this is the best approach.

[mclow.lists]

In D6347#5, @jroelofs wrote:

• I *really* *really* don't like the idea of changing tests to push bugs under the rug. It'd be much better to 'XFAIL: asan' these. I have a thought on how to fix asan that I'll add to the PR.

I, too, would much prefer a fix to ASAN, and/or a way to XFAIL them.
[ But I did note that this is an "intermittent failure" under ASAN ]

[EricWF]

Lets hold off for a couple of days to see if this can be fixed in ASAN then. Otherwise I don't see another option.

[EricWF]

Apparently this has been fixed in ASAN! Happily abandoning.

test/thread/futures/futures.shared_future/get.pass.cpp

 
 int main()
 {
     {
         typedef int T;
         {
             std::promise<T> p;
             std::shared_future<T> f = p.get_future();
-            std::thread(func1, std::move(p)).detach();
+            std::thread t(func1, std::move(p));
             assert(f.valid());
             assert(f.get() == 3);
             assert(f.valid());
+            t.join();
         }
         {
             std::promise<T> p;
             std::shared_future<T> f = p.get_future();
-            std::thread(func2, std::move(p)).detach();
+            std::thread t(func2, std::move(p));
             try
             {
                 assert(f.valid());
                 assert(f.get() == 3);
                 assert(false);
             }
             catch (int i)
             {
                 assert(i == 3);
             }
             assert(f.valid());
+            t.join();
         }
     }
     {
         typedef int& T;
         {
             std::promise<T> p;
             std::shared_future<T> f = p.get_future();
-            std::thread(func3, std::move(p)).detach();
+            std::thread t(func3, std::move(p));
             assert(f.valid());
             assert(f.get() == 5);
             assert(f.valid());
+            t.join();
         }
         {
             std::promise<T> p;
             std::shared_future<T> f = p.get_future();
-            std::thread(func4, std::move(p)).detach();
+            std::thread t(func4, std::move(p));
             try
             {
                 assert(f.valid());
                 assert(f.get() == 3);
                 assert(false);
             }
             catch (double i)
             {
                 assert(i == 3.5);
             }
             assert(f.valid());
+            t.join();
         }
     }
     {
         typedef void T;
         {
             std::promise<T> p;
             std::shared_future<T> f = p.get_future();
-            std::thread(func5, std::move(p)).detach();
+            std::thread t(func5, std::move(p));
             assert(f.valid());
             f.get();
             assert(f.valid());
+            t.join();
         }
         {
             std::promise<T> p;
             std::shared_future<T> f = p.get_future();
-            std::thread(func6, std::move(p)).detach();
+            std::thread t(func6, std::move(p));
             try
             {
                 assert(f.valid());
                 f.get();
                 assert(false);
             }
             catch (char i)
             {
                 assert(i == 'c');
             }
             assert(f.valid());
+            t.join();
         }
     }
 }

test/thread/futures/futures.shared_future/wait.pass.cpp

 int main()
 {
     typedef std::chrono::high_resolution_clock Clock;
     typedef std::chrono::duration<double, std::milli> ms;
     {
         typedef int T;
         std::promise<T> p;
         std::shared_future<T> f = p.get_future();
-        std::thread(func1, std::move(p)).detach();
+        std::thread t(func1, std::move(p));
         assert(f.valid());
         f.wait();
         assert(f.valid());
         Clock::time_point t0 = Clock::now();
         f.wait();
         Clock::time_point t1 = Clock::now();
         assert(f.valid());
         assert(t1-t0 < ms(5));
+        t.join();
     }
     {
         typedef int& T;
         std::promise<T> p;
         std::shared_future<T> f = p.get_future();
-        std::thread(func3, std::move(p)).detach();
+        std::thread t(func3, std::move(p));
         assert(f.valid());
         f.wait();
         assert(f.valid());
         Clock::time_point t0 = Clock::now();
         f.wait();
         Clock::time_point t1 = Clock::now();
         assert(f.valid());
         assert(t1-t0 < ms(5));
+        t.join();
     }
     {
         typedef void T;
         std::promise<T> p;
         std::shared_future<T> f = p.get_future();
-        std::thread(func5, std::move(p)).detach();
+        std::thread t(func5, std::move(p));
         assert(f.valid());
         f.wait();
         assert(f.valid());
         Clock::time_point t0 = Clock::now();
         f.wait();
         Clock::time_point t1 = Clock::now();
         assert(f.valid());
         assert(t1-t0 < ms(5));
+        t.join();
     }
 }

test/thread/futures/futures.shared_future/wait_for.pass.cpp

 
 int main()
 {
     typedef std::chrono::high_resolution_clock Clock;
     {
         typedef int T;
         std::promise<T> p;
         std::shared_future<T> f = p.get_future();
-        std::thread(func1, std::move(p)).detach();
+        std::thread t(func1, std::move(p));
         assert(f.valid());
         assert(f.wait_for(ms(300)) == std::future_status::timeout);
         assert(f.valid());
         assert(f.wait_for(ms(300)) == std::future_status::ready);
         assert(f.valid());
         Clock::time_point t0 = Clock::now();
         f.wait();
         Clock::time_point t1 = Clock::now();
         assert(f.valid());
         assert(t1-t0 < ms(5));
+        t.join();
     }
     {
         typedef int& T;
         std::promise<T> p;
         std::shared_future<T> f = p.get_future();
-        std::thread(func3, std::move(p)).detach();
+        std::thread t(func3, std::move(p));
         assert(f.valid());
         assert(f.wait_for(ms(300)) == std::future_status::timeout);
         assert(f.valid());
         assert(f.wait_for(ms(300)) == std::future_status::ready);
         assert(f.valid());
         Clock::time_point t0 = Clock::now();
         f.wait();
         Clock::time_point t1 = Clock::now();
         assert(f.valid());
         assert(t1-t0 < ms(5));
+        t.join();
     }
     {
         typedef void T;
         std::promise<T> p;
         std::shared_future<T> f = p.get_future();
-        std::thread(func5, std::move(p)).detach();
+        std::thread t(func5, std::move(p));
         assert(f.valid());
         assert(f.wait_for(ms(300)) == std::future_status::timeout);
         assert(f.valid());
         assert(f.wait_for(ms(300)) == std::future_status::ready);
         assert(f.valid());
         Clock::time_point t0 = Clock::now();
         f.wait();
         Clock::time_point t1 = Clock::now();
         assert(f.valid());
         assert(t1-t0 < ms(5));
+        t.join();
     }
 }

test/thread/futures/futures.shared_future/wait_until.pass.cpp

 
 int main()
 {
     typedef std::chrono::high_resolution_clock Clock;
     {
         typedef int T;
         std::promise<T> p;
         std::shared_future<T> f = p.get_future();
-        std::thread(func1, std::move(p)).detach();
+        std::thread t(func1, std::move(p));
         assert(f.valid());
         assert(f.wait_until(Clock::now() + ms(300)) == std::future_status::timeout);
         assert(f.valid());
         assert(f.wait_until(Clock::now() + ms(300)) == std::future_status::ready);
         assert(f.valid());
         Clock::time_point t0 = Clock::now();
         f.wait();
         Clock::time_point t1 = Clock::now();
         assert(f.valid());
         assert(t1-t0 < ms(5));
+        t.join();
     }
     {
         typedef int& T;
         std::promise<T> p;
         std::shared_future<T> f = p.get_future();
-        std::thread(func3, std::move(p)).detach();
+        std::thread t(func3, std::move(p));
         assert(f.valid());
         assert(f.wait_until(Clock::now() + ms(300)) == std::future_status::timeout);
         assert(f.valid());
         assert(f.wait_until(Clock::now() + ms(300)) == std::future_status::ready);
         assert(f.valid());
         Clock::time_point t0 = Clock::now();
         f.wait();
         Clock::time_point t1 = Clock::now();
         assert(f.valid());
         assert(t1-t0 < ms(5));
+        t.join();
     }
     {
         typedef void T;
         std::promise<T> p;
         std::shared_future<T> f = p.get_future();
-        std::thread(func5, std::move(p)).detach();
+        std::thread t(func5, std::move(p));
         assert(f.valid());
         assert(f.wait_until(Clock::now() + ms(300)) == std::future_status::timeout);
         assert(f.valid());
         assert(f.wait_until(Clock::now() + ms(300)) == std::future_status::ready);
         assert(f.valid());
         Clock::time_point t0 = Clock::now();
         f.wait();
         Clock::time_point t1 = Clock::now();
         assert(f.valid());
         assert(t1-t0 < ms(5));
+        t.join();
     }
 }

test/thread/futures/futures.tas/futures.task.members/dtor.pass.cpp

void func2(std::packaged_task<double(int, char)> p)
     p(3, 'a');
 }
 
 int main()
 {
     {
         std::packaged_task<double(int, char)> p(A(5));
         std::future<double> f = p.get_future();
-        std::thread(func, std::move(p)).detach();
+        std::thread t(func, std::move(p));
         try
         {
             double i = f.get();
             assert(false);
         }
         catch (const std::future_error& e)
         {
             assert(e.code() == make_error_code(std::future_errc::broken_promise));
         }
+        t.join();
     }
     {
         std::packaged_task<double(int, char)> p(A(5));
         std::future<double> f = p.get_future();
-        std::thread(func2, std::move(p)).detach();
+        std::thread t(func2, std::move(p));
         assert(f.get() == 105.0);
+        t.join();
     }
 }

test/thread/futures/futures.tas/futures.task.members/make_ready_at_thread_exit.pass.cpp

void func3(std::packaged_task<double(int, char)> p)
     }
 }
 
 int main()
 {
     {
         std::packaged_task<double(int, char)> p(A(5));
         std::future<double> f = p.get_future();
-        std::thread(func0, std::move(p)).detach();
+        std::thread t(func0, std::move(p));
         assert(f.get() == 105.0);
+        t.join();
     }
     {
         std::packaged_task<double(int, char)> p(A(5));
         std::future<double> f = p.get_future();
-        std::thread(func1, std::move(p)).detach();
+        std::thread t(func1, std::move(p));
         try
         {
             f.get();
             assert(false);
         }
         catch (const A& e)
         {
             assert(e(3, 'a') == 106);
         }
+        t.join();
     }
     {
         std::packaged_task<double(int, char)> p(A(5));
         std::future<double> f = p.get_future();
-        std::thread(func2, std::move(p)).detach();
+        std::thread t(func2, std::move(p));
         assert(f.get() == 105.0);
+        t.join();
     }
     {
         std::packaged_task<double(int, char)> p;
         std::thread t(func3, std::move(p));
         t.join();
     }
 }

test/thread/futures/futures.tas/futures.task.members/operator.pass.cpp

void func3(std::packaged_task<double(int, char)> p)
     }
 }
 
 int main()
 {
     {
         std::packaged_task<double(int, char)> p(A(5));
         std::future<double> f = p.get_future();
-        std::thread(func0, std::move(p)).detach();
+        std::thread t(func0, std::move(p));
         assert(f.get() == 105.0);
+        t.join();
     }
     {
         std::packaged_task<double(int, char)> p(A(5));
         std::future<double> f = p.get_future();
-        std::thread(func1, std::move(p)).detach();
+        std::thread t(func1, std::move(p));
         try
         {
             f.get();
             assert(false);
         }
         catch (const A& e)
         {
             assert(e(3, 'a') == 106);
         }
+        t.join();
     }
     {
         std::packaged_task<double(int, char)> p(A(5));
         std::future<double> f = p.get_future();
         std::thread t(func2, std::move(p));
         assert(f.get() == 105.0);
         t.join();
     }
     {
         std::packaged_task<double(int, char)> p;
         std::thread t(func3, std::move(p));
         t.join();
     }
 }

test/thread/futures/futures.unique_future/get.pass.cpp

 
 int main()
 {
     {
         typedef int T;
         {
             std::promise<T> p;
             std::future<T> f = p.get_future();
-            std::thread(func1, std::move(p)).detach();
+            std::thread t(func1, std::move(p));
             assert(f.valid());
             assert(f.get() == 3);
             assert(!f.valid());
+            t.join();
         }
         {
             std::promise<T> p;
             std::future<T> f = p.get_future();
-            std::thread(func2, std::move(p)).detach();
+            std::thread t(func2, std::move(p));
             try
             {
                 assert(f.valid());
                 assert(f.get() == 3);
                 assert(false);
             }
             catch (int i)
             {
                 assert(i == 3);
             }
             assert(!f.valid());
+            t.join();
         }
     }
     {
         typedef int& T;
         {
             std::promise<T> p;
             std::future<T> f = p.get_future();
-            std::thread(func3, std::move(p)).detach();
+            std::thread t(func3, std::move(p));
             assert(f.valid());
             assert(f.get() == 5);
             assert(!f.valid());
+            t.join();
         }
         {
             std::promise<T> p;
             std::future<T> f = p.get_future();
-            std::thread(func4, std::move(p)).detach();
+            std::thread t(func4, std::move(p));
             try
             {
                 assert(f.valid());
                 assert(f.get() == 3);
                 assert(false);
             }
             catch (double i)
             {
                 assert(i == 3.5);
             }
             assert(!f.valid());
+            t.join();
         }
     }
     {
         typedef void T;
         {
             std::promise<T> p;
             std::future<T> f = p.get_future();
-            std::thread(func5, std::move(p)).detach();
+            std::thread t(func5, std::move(p));
             assert(f.valid());
             f.get();
             assert(!f.valid());
+            t.join();
         }
         {
             std::promise<T> p;
             std::future<T> f = p.get_future();
-            std::thread(func6, std::move(p)).detach();
+            std::thread t(func6, std::move(p));
             try
             {
                 assert(f.valid());
                 f.get();
                 assert(false);
             }
             catch (char i)
             {
                 assert(i == 'c');
             }
             assert(!f.valid());
+            t.join();
         }
     }
 }

test/thread/futures/futures.unique_future/wait.pass.cpp

 int main()
 {
     typedef std::chrono::high_resolution_clock Clock;
     typedef std::chrono::duration<double, std::milli> ms;
     {
         typedef int T;
         std::promise<T> p;
         std::future<T> f = p.get_future();
-        std::thread(func1, std::move(p)).detach();
+        std::thread t(func1, std::move(p));
         assert(f.valid());
         f.wait();
         assert(f.valid());
         Clock::time_point t0 = Clock::now();
         f.wait();
         Clock::time_point t1 = Clock::now();
         assert(f.valid());
         assert(t1-t0 < ms(5));
+        t.join();
     }
     {
         typedef int& T;
         std::promise<T> p;
         std::future<T> f = p.get_future();
-        std::thread(func3, std::move(p)).detach();
+        std::thread t(func3, std::move(p));
         assert(f.valid());
         f.wait();
         assert(f.valid());
         Clock::time_point t0 = Clock::now();
         f.wait();
         Clock::time_point t1 = Clock::now();
         assert(f.valid());
         assert(t1-t0 < ms(5));
+        t.join();
     }
     {
         typedef void T;
         std::promise<T> p;
         std::future<T> f = p.get_future();
-        std::thread(func5, std::move(p)).detach();
+        std::thread t(func5, std::move(p));
         assert(f.valid());
         f.wait();
         assert(f.valid());
         Clock::time_point t0 = Clock::now();
         f.wait();
         Clock::time_point t1 = Clock::now();
         assert(f.valid());
         assert(t1-t0 < ms(5));
+        t.join();
     }
 }

test/thread/futures/futures.unique_future/wait_for.pass.cpp

 
 int main()
 {
     typedef std::chrono::high_resolution_clock Clock;
     {
         typedef int T;
         std::promise<T> p;
         std::future<T> f = p.get_future();
-        std::thread(func1, std::move(p)).detach();
+        std::thread t(func1, std::move(p));
         assert(f.valid());
         assert(f.wait_for(ms(300)) == std::future_status::timeout);
         assert(f.valid());
         assert(f.wait_for(ms(300)) == std::future_status::ready);
         assert(f.valid());
         Clock::time_point t0 = Clock::now();
         f.wait();
         Clock::time_point t1 = Clock::now();
         assert(f.valid());
         assert(t1-t0 < ms(50));
+        t.join();
     }
     {
         typedef int& T;
         std::promise<T> p;
         std::future<T> f = p.get_future();
-        std::thread(func3, std::move(p)).detach();
+        std::thread t(func3, std::move(p));
         assert(f.valid());
         assert(f.wait_for(ms(300)) == std::future_status::timeout);
         assert(f.valid());
         assert(f.wait_for(ms(300)) == std::future_status::ready);
         assert(f.valid());
         Clock::time_point t0 = Clock::now();
         f.wait();
         Clock::time_point t1 = Clock::now();
         assert(f.valid());
         assert(t1-t0 < ms(50));
+        t.join();
     }
     {
         typedef void T;
         std::promise<T> p;
         std::future<T> f = p.get_future();
-        std::thread(func5, std::move(p)).detach();
+        std::thread t(func5, std::move(p));
         assert(f.valid());
         assert(f.wait_for(ms(300)) == std::future_status::timeout);
         assert(f.valid());
         assert(f.wait_for(ms(300)) == std::future_status::ready);
         assert(f.valid());
         Clock::time_point t0 = Clock::now();
         f.wait();
         Clock::time_point t1 = Clock::now();
         assert(f.valid());
         assert(t1-t0 < ms(50));
+        t.join();
     }
 }

test/thread/futures/futures.unique_future/wait_until.pass.cpp

 
 int main()
 {
     typedef std::chrono::high_resolution_clock Clock;
     {
         typedef int T;
         std::promise<T> p;
         std::future<T> f = p.get_future();
-        std::thread(func1, std::move(p)).detach();
+        std::thread t(func1, std::move(p));
         assert(f.valid());
         assert(f.wait_until(Clock::now() + ms(300)) == std::future_status::timeout);
         assert(f.valid());
         assert(f.wait_until(Clock::now() + ms(300)) == std::future_status::ready);
         assert(f.valid());
         Clock::time_point t0 = Clock::now();
         f.wait();
         Clock::time_point t1 = Clock::now();
         assert(f.valid());
         assert(t1-t0 < ms(5));
+        t.join();
     }
     {
         typedef int& T;
         std::promise<T> p;
         std::future<T> f = p.get_future();
-        std::thread(func3, std::move(p)).detach();
+        std::thread t(func3, std::move(p));
         assert(f.valid());
         assert(f.wait_until(Clock::now() + ms(300)) == std::future_status::timeout);
         assert(f.valid());
         assert(f.wait_until(Clock::now() + ms(300)) == std::future_status::ready);
         assert(f.valid());
         Clock::time_point t0 = Clock::now();
         f.wait();
         Clock::time_point t1 = Clock::now();
         assert(f.valid());
         assert(t1-t0 < ms(5));
+        t.join();
     }
     {
         typedef void T;
         std::promise<T> p;
         std::future<T> f = p.get_future();
-        std::thread(func5, std::move(p)).detach();
+        std::thread t(func5, std::move(p));
         assert(f.valid());
         assert(f.wait_until(Clock::now() + ms(300)) == std::future_status::timeout);
         assert(f.valid());
         assert(f.wait_until(Clock::now() + ms(300)) == std::future_status::ready);
         assert(f.valid());
         Clock::time_point t0 = Clock::now();
         f.wait();
         Clock::time_point t1 = Clock::now();
         assert(f.valid());
         assert(t1-t0 < ms(5));
+        t.join();
     }
 }