On Fri, Feb 09, 2018 at 09:30:56PM +0100, Martin Wilck wrote: > On Thu, 2018-02-08 at 17:56 -0600, Benjamin Marzinski wrote: > > ct->running is now an atomic variable. When the thread is started > > it is set to 1. When the checker wants to kill a thread, it > > atomically > > sets the value to 0 and reads the previous value. If it was 1, > > the checker cancels the thread. If it was 0, the nothing needs to be > > done. After the checker has dealt with the thread, it sets ct- > > >thread > > to NULL. > > > > When the thread is done, it atomicalllys sets the value of ct- > > >running > > to 0 and reads the previous value. If it was 1, the thread just > > exits. > > If it was 0, then the checker is trying to cancel the thread, and so > > the thread calls pause(), which is a cancellation point. > > > > I'm missing one thing here. My poor brain is aching. > > cleanup_func() can be entered in two ways: a) if the thread has been > cancelled and passed a cancellation point already, or b) if it exits > normally and calls pthread_cleanup_pop(). > In case b), waiting for the cancellation request by calling pause() > makes sense to me. But in case a), the thread has already seen the > cancellation request - wouldn't calling pause() cause it to sleep > forever? Urgh. You're right. If it is in the cleanup helper because it already has been cancelled, then the pause isn't going get cancelled. So much for my quick rewrite. That leaves three options. 1. have either the thread or the checker detach the thread (depending on which one exits first) 2. make the checker always cancel and detach the thread. This simplifies the code, but there will zombie threads hanging around between calls to the checker. 3. just move the condlog I really don't care which one we pick anymore. -Ben > > Martin > > -- > Dr. Martin Wilck <mwilck@xxxxxxxx>, Tel. +49 (0)911 74053 2107 > SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton > HRB 21284 (AG Nürnberg) -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel
