Re: PTY dies when master in parent process is closed.

[Corinna Vinschen <corinna-cygwin at cygwin dot com>]

On Mar 12 20:03, Takashi Yano wrote:
> On Wed, 11 Mar 2015 17:48:29 +0100
> Corinna Vinschen <corinna-cygwin@cygwin.com> wrote:
> 
> > I'm inclined to apply this patch.  I think using PeekNamedPipe this way
> > is pretty nice, albeit it's not atomic.  It might be a good idea to add
> > an acquire/release_output_mutex bracket, even if that doesn't help for
> > cases in which a process holding a master fd crashes hard.
> > 
> > Since you suggested it, do you have another idea?
> 
> Nothing in particular.
> 
> > Off the top of my head what we could do:
> > 
> > - Either just set input_available_event always, unconditionally.  But
> >   that probably requires to change fhandler_pty_slave::read as well.
> 
> The modification of fhandler_pty_slave::read() will be
> probably not so small. To use PeekNamedPipe() or NtQueryObject()
> is better, I suppose.
>  
> > - Or, use NtQueryObject to fetch the handle count of from_master (see
> >   flock.cc function get_obj_handle_count()).  If the handle count is
> >   one, we're the last process holding the handle and then we set
> >   input_available_event.  But I'm not sure if it's really worth it.  The
> >   PeekNamedPipe usage looks rather ok to me.
> 
> When NtQueryObject() is used, aim is clearer than using
> PeekNamedPipe(). However, either is OK. Please choose one
> which you like.

On second thought, using NtQueryObject introduces a potential new
race condition.  Since for this test to make sense you have to check
the output handle before closing it, the test-and-close action would
have to be made atomically with all other ways to duplicate the handle.
Using PeekNamedPipe after closing the handle is certainly the better
approach.

Patch applied with an additional preceeding comment so we know why
PeekNamedPipe is called here.


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

Attachment: pgpqlz4ulqga8.pgp
Description: PGP signature