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:
> > 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 :)
> > No, both dtors are run after the while (!done) {} line.
>
> But how do we get past while (!done) line before the desctructor in the
> thread has finished? The destructor sets done. So after while (!done) line
> the destructor is effectively finished. What am I missing?
>The destructor sets done.
Hmmm I thought done was getting set on 56 but that's done_ (with a trailing
underscore). :sigh:
https://reviews.llvm.org/D50549
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
