On Tue, 2018-01-16 at 16:39 -0600, Benjamin Marzinski wrote: > On Tue, Jan 16, 2018 at 02:19:20PM +0100, Martin Wilck wrote: > > On Tue, 2018-01-16 at 11:48 +0000, Wuchongyun wrote: > > > Hi Martin, > > > Sorry to forget that, actually I found that dead_client() will > > > not be > > > interrupt by thread cancle, because after all dead_client() > > > calling > > > point be done then handle_signals() have chance to be called by > > > uxsock_listen() which will call exit_daemon() and send > > > cancel threads signal to all child process include uxlsnr. > > > > Fair enough. > > > > > But your comments is good can make code more safer. Below is the > > > new > > > patch, please have a look, thanks. > > > > I think it's really safer whis way, should anyone see the need to > > cancel the listener thread from another point in the code. > > I'm confused why this is safe. After uxsock_listen() calls > exit_daemon() > from handle_signals(), it doesn't exit. It loops around and polls > again, > and could in theory find a client that has died. In fact if the > client > is killing multipathd via > > # multipathd shutdown > > instead of a signal, won't it be very likely that it will find a dead > client when it loops right after calling exit_daemon() in > cli_shutdown()? This could hit the deadlock that you noticed, where > uxsock_cleanup() can't run because dead_client() already holding the > mutex. > > Or am I missing something here? With "safer", I was only referring to the fact that Wuchongyun replaced pthread_mutex_unlock() with pthread_cleanup_push(cleanup_lock, &client_lock) ... pthread_cleanup_pop(). The original deadlock is avoided either way, AFAICS. 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
