This is the mail archive of the
mailing list for the Cygwin project.
Re: Fix leaky pipe
- From: Christopher Faylor <cgf-no-personal-reply-please at cygwin dot com>
- To: cygwin-patches at cygwin dot com
- Date: Mon, 29 May 2006 18:00:33 -0400
- Subject: Re: Fix leaky pipe
- References: <022401c682fc$b51c2d50$a501a8c0@CAM.ARTIMI.COM>
- Reply-to: cygwin-patches at cygwin dot com
On Mon, May 29, 2006 at 09:48:50AM +0100, Dave Korn wrote:
>As discussed elsewhere, here's a patch that solves the race problem
>without leaking handles any more by placing the master-pipe-closing
>logic where it really belongs, in fhandler_tty_common::close where the
>send-an-EOF decision is made. I figured 4 extra bytes in the vtable
>isn't too bad, it's not like you expect to have millions of terminals
>open at once.
I don't know. The more I think about this problem, the more I think
that the logic which you've exposed which deals with "inuse" in
fhandler_pty_master::close is wrong.
It's been a while since I looked at this code (and I've always hated it
-- even after I sort of rewrote it) but for a pty, I don't see why it
should matter if there are still other things using the open master
handles. I think that each parent pty should have its own copy of the
from_master/to_master handles which are unconditionally closed. I'm
probably missing something, but I think the inuse handle for the master
part of the pty is probably not needed.
So, thanks very much for the patch, Dave, but I think I have to cogitate
on this some more.
I probably will propose another way to handle this that will reorganize
the tty structure and the fhandler_[pt]tty classes. OTOH, I haven't
given this as much thought as you have, so maybe I'll come around to
agreeing that your patch is the best way to go.
Right now, however, this code is sending off little alarms that tell me
that something is wrong. I tend to trust these alarms because they are
right 50% of the time.
I know. It's an amazing ability.
Anyway, thanks again for the patch. I hope to discuss this more
intelligently in the next couple of days.