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: > > > > 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;` > > >What is gCopy? I don't see anything named gCopy in this file... > > > > The copy is made in the constructor of std::thread. std::thread makes a > > copy of all the input parameters, gives the copy to the started thread, and > > then std::invoke is called there. > > > > >Why does it matter that "the destruction of the object on which op() is > > >being called is not so synchronized."? > > > > Because the two dtors race on `--n_alive;` when `n_alive` is not atomic. > But the first dtor runs before "while (!done) {}" line and the second dtor > runs after "while (!done) {}" line, no? > Or there is third object involved? But then I don't see how joining the > thread would help either. >But the first dtor runs before "while (!done) {}" line No, both dtors are run after the while (!done) {} line. The first dtor runs on line 76 (when the local variable g is destroyed), and the second dtor runs after operator() returns in the constructed thread. The constructed thread is morally doing: ``` void threadproc(G * g) { g->operator(); // setting done happens in here delete g; // dtor of second copy runs here } ``` > I don't see how joining the thread would help either. Joining with the thread would wait for the second dtor -- the one after op() returns -- to complete. Of course joining with the thread isn't doable here given that the point is to test thread::detach :) https://reviews.llvm.org/D50549 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits