[PATCH] D50549: [libcxx] [test] Repair thread unsafety in thread tests

[Billy Robert O'Neal III via Phabricator via cfe-commits]

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