BillyONeal added inline comments.
================ Comment at: test/std/thread/thread.threads/thread.thread.class/thread.thread.member/detach.pass.cpp:73 assert(!t0.joinable()); while (!done) {} assert(G::op_run); ---------------- dvyukov wrote: > BillyONeal wrote: > > BillyONeal wrote: > > > dvyukov wrote: > > > > I don't immediately see how the race on n_alive/op_run happens. It > > > > seems that the modifications in the thread happen before this line, and > > > > modifications in main happen after this line. How can both of them > > > > modify the variables at the same time? > > > The destruction of g here races with the destruction of the DECAY_COPY'd > > > copy of G used as the parameter of operator(). That is, line 69 creates a > > > copy of g, passes that to the started thread, the started thread calls > > > gCopy(). gCopy() doesn't return until the done flag is set, but the > > > destruction of the object on which op() is being called is not so > > > synchronized. Most of the other thread tests avoid this problem by > > > joining with the thread; joining waits for the destruction of the > > > DECAY_COPY'd parameters, but this does not. > > > > > > (This is one of the reasons detach() should basically never be called > > > anywhere) > > > > > (That is to say, there's nothing to prevent both threads from executing > > G::!G() on the two different copies of g... making op_run atomic is > > probably avoidable but I'm being paranoid given that there was already > > thread unsafety here...) > What is gCopy? I don't see anything named gCopy in this file... > > Do we care about completion of destruction? Why? We wait for done to be set, > and other variables are already updated at that point. Why does it matter > that "the destruction of the object on which op() is being called is not so > synchronized."? Because the destructor does `--n_alive;` https://reviews.llvm.org/D50549 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits