[lxc-devel] [PATCH] fix console deadlocks

[Serge Hallyn]

Quoting Dwight Engen (dwight.engen at oracle.com):
> On Fri, 20 Sep 2013 15:26:47 -0500
> Serge Hallyn <serge.hallyn at ubuntu.com> wrote:
> 
> > Quoting Dwight Engen (dwight.engen at oracle.com):
> > > On Fri, 20 Sep 2013 14:48:40 -0500
> > > Serge Hallyn <serge.hallyn at ubuntu.com> wrote:
> > > 
> > > > These might be a bit controversial.  The process lock was held
> > > > for some long periods of time for tweaking consoles.   These
> > > > can deadlock with some of lock holds I introduced recently.  I
> > > > would argue that if two threads are fighting over the console,
> > > > you're gonna have trouble anyway, and the process locks here
> > > > weren't saving us from anything.  If we want to do a console
> > > 
> > > Are we sure we can walk/modify the lxc_ttys list lockless? I agree
> > 
> > Yeah we do need to lock those.  But I don't think there's a rush for
> > it.
> > 
> > > threaded console use is "interesting", but I don't think we want
> > > list corruption. Of course we don't want deadlocks either :)
> > 
> > So which do you think would be better - introduce a lxc_ttys lock
> > specifically, or introduce a general 'console' mutex which is taken
> > any time we open a console or edit the lxc_ttys?
> 
> Hmm, good question. I think that lxc_console_allocate() and
> lxc_console_free() are only called from lxc today when handling commands
> so they are not going to be used from a threaded context anyways. But
> the symbols are available so someone could write a threaded program and
> call them and race on the .busy indicator for example. I think we
> minimally need the list stuff locked, but I'm not sure what else. I
> have not really tested multithreaded console use.
> 
> Just out of curiosity, this is deadlocking because the process lock is
> already held when console routines are getting called and the process
> lock mutex isn't recursive?

At the least lxc_console_allocate() was taking process_lock(),
then calling lxc_console_peer_proxy_alloc() which also calls
process_lock.  I could have decided not to take the lock in
lxc_console_peer_proxy_alloc(), but that seems more wrong and
be less maintainable.

-serge