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

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

Hi Takashi,

On Mar  8 16:37, Takashi Yano wrote:
> On Fri, 6 Mar 2015 20:07:10 +0900
> Takashi Yano <takashi.yano@nifty.ne.jp> wrote:
> 
> > On Thu, 5 Mar 2015 14:58:39 +0100
> > Corinna Vinschen <corinna-cygwin@cygwin.com> wrote:
> > 
> > > I applied a patch.  Please have a look.
> > 
> > I have tested the latest CVS version, and found
> > a new problem.
> > 
> > With new CVS version, slave side can not detect
> > closure of master.
> > 
> > Please use following Test Case 3. Test Case 3 is
> > not terminated by itself with latest CVS.
> > 
> > It seems that the program is stopping at
> > cygwait(input_available_event, time_to_wait)
> > in fhandler_pty_slave::read().
> > 
> > I guess input_available_event should be set when
> > the last valid master fd is closed.
> 
> For this problem, I have made a patch.
> 
> With this patch, it has been confirmed that the problems
> in Test Case 1, 2 and 3 are fixed.
> 
> I am glad if this would be a help.
> 
> However, one matter to be concerned is irregular use of
> PeekNamedPipe(). Maybe alternative means could be better
> for detecting closure of all master fds.

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?

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.

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

Oh, btw.  Please always add the ChangeLog entries as plain text, not as
diff.  ChangeLog diffs usually don't apply without manual intervention.


Thanks,
Corinna

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

Attachment: pgpRPhPWFm6u1.pgp
Description: PGP signature