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

[Martin Wilck]

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 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)