[dm-devel] [PATCH V3] multipathd: release uxsocket and resource when cancel thread

[Benjamin Marzinski]

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?
-Ben

> 
> The patch is looks good now.
> 
> Reviewed-by: Martin Wilck <mwilck at suse.com>
> 
> -- 
> Dr. Martin Wilck <mwilck at suse.com>, 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 at redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel