BillyONeal created this revision.
BillyONeal added reviewers: EricWF, mclow.lists.
Herald added a subscriber: jfb.

- Repair thread-unsafe modifications of n_alive in F.pass.cpp

In this example, the ctor of G runs in the main thread in the expression G(), 
and also in the copy ctor of G() in the DECAY_COPY inside std::thread. The main 
thread destroys the G() instance at the semicolon, and the started thread 
destroys the G() after it returns. Thus there is a race between the threads on 
the n_alive variable.

The fix is to join with the background thread before attempting to destroy the 
G in the main thread.

- Repair a race in detach.pass.cpp by adding atomic.

This test thinks that setting done = true as the last line in the thread 
procedure is equivalent to joining with it, but that is not true. There is a 
race between destroying the G instance in main and destroying the *parameter* 
of the thread procedure. This problem is avoided by making n_alive and op_run 
atomics.


https://reviews.llvm.org/D50549

Files:
  
test/std/thread/thread.threads/thread.thread.class/thread.thread.constr/F.pass.cpp
  
test/std/thread/thread.threads/thread.thread.class/thread.thread.member/detach.pass.cpp


Index: 
test/std/thread/thread.threads/thread.thread.class/thread.thread.member/detach.pass.cpp
===================================================================
--- 
test/std/thread/thread.threads/thread.thread.class/thread.thread.member/detach.pass.cpp
+++ 
test/std/thread/thread.threads/thread.thread.class/thread.thread.member/detach.pass.cpp
@@ -29,8 +29,8 @@
     int alive_;
     bool done_;
 public:
-    static int n_alive;
-    static bool op_run;
+    static std::atomic_int n_alive;
+    static std::atomic_bool op_run;
 
     G() : alive_(1), done_(false)
     {
@@ -57,8 +57,8 @@
     }
 };
 
-int G::n_alive = 0;
-bool G::op_run = false;
+std::atomic_int G::n_alive = 0;
+std::atomic_bool G::op_run = false;
 
 void foo() {}
 
Index: 
test/std/thread/thread.threads/thread.thread.class/thread.thread.constr/F.pass.cpp
===================================================================
--- 
test/std/thread/thread.threads/thread.thread.class/thread.thread.constr/F.pass.cpp
+++ 
test/std/thread/thread.threads/thread.thread.class/thread.thread.constr/F.pass.cpp
@@ -157,8 +157,11 @@
     {
         assert(G::n_alive == 0);
         assert(!G::op_run);
-        std::thread t((G()));
-        t.join();
+        {
+            G g;
+            std::thread t(g);
+            t.join();
+        }
         assert(G::n_alive == 0);
         assert(G::op_run);
     }
@@ -185,8 +188,11 @@
     {
         assert(G::n_alive == 0);
         assert(!G::op_run);
-        std::thread t(G(), 5, 5.5);
-        t.join();
+        {
+            G g;
+            std::thread t(g, 5, 5.5);
+            t.join();
+        }
         assert(G::n_alive == 0);
         assert(G::op_run);
     }


Index: test/std/thread/thread.threads/thread.thread.class/thread.thread.member/detach.pass.cpp
===================================================================
--- test/std/thread/thread.threads/thread.thread.class/thread.thread.member/detach.pass.cpp
+++ test/std/thread/thread.threads/thread.thread.class/thread.thread.member/detach.pass.cpp
@@ -29,8 +29,8 @@
     int alive_;
     bool done_;
 public:
-    static int n_alive;
-    static bool op_run;
+    static std::atomic_int n_alive;
+    static std::atomic_bool op_run;
 
     G() : alive_(1), done_(false)
     {
@@ -57,8 +57,8 @@
     }
 };
 
-int G::n_alive = 0;
-bool G::op_run = false;
+std::atomic_int G::n_alive = 0;
+std::atomic_bool G::op_run = false;
 
 void foo() {}
 
Index: test/std/thread/thread.threads/thread.thread.class/thread.thread.constr/F.pass.cpp
===================================================================
--- test/std/thread/thread.threads/thread.thread.class/thread.thread.constr/F.pass.cpp
+++ test/std/thread/thread.threads/thread.thread.class/thread.thread.constr/F.pass.cpp
@@ -157,8 +157,11 @@
     {
         assert(G::n_alive == 0);
         assert(!G::op_run);
-        std::thread t((G()));
-        t.join();
+        {
+            G g;
+            std::thread t(g);
+            t.join();
+        }
         assert(G::n_alive == 0);
         assert(G::op_run);
     }
@@ -185,8 +188,11 @@
     {
         assert(G::n_alive == 0);
         assert(!G::op_run);
-        std::thread t(G(), 5, 5.5);
-        t.join();
+        {
+            G g;
+            std::thread t(g, 5, 5.5);
+            t.join();
+        }
         assert(G::n_alive == 0);
         assert(G::op_run);
     }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to