Mail Archives: cygwin/2015/05/28/07:47:48
--vkogqOf2sHV7VnPd
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On May 27 17:17, Corinna Vinschen wrote:
> On May 27 14:23, Corinna Vinschen wrote:
> > Hi Takashi,
> >=20
> > On May 21 20:53, Takashi Yano wrote:
> > > [...]
> > > + if (output_handle_local)
> > > + {
> > > + OBJECT_BASIC_INFORMATION obi;
> > > + NTSTATUS status;
> > > + ULONG hdl_cnt =3D 0;
> > > +
> > > + status =3D NtQueryObject (output_handle_local, ObjectBasicInfo=
rmation,
> > > + &obi, sizeof obi, NULL);
> > > + if (!NT_SUCCESS (status))
> > > + debug_printf ("NtQueryObject: %y", status);
> > > + else
> > > + hdl_cnt =3D obi.HandleCount;
> > > + termios_printf("HandleCount: %d", hdl_cnt);
> > > + if (hdl_cnt =3D=3D 1)
> > > + SetEvent (input_available_event);
> > > + CloseHandle (output_handle_local);
> > > + }
> >=20=20
> > Isn't that racy? Consider two processes doing that at the same time.
> > Both calls to NtQueryObject could come up with hdl_cnt =3D=3D 2 and the
> > problem persists.
> >=20
> > Wouldn't it be safer to call SetEvent(input_available_event) all the
> > time from here?
>=20
> We discussed this already in March and only briefly talked about a
> change like this requiring changes to fhandler_pty_slave::read.
> However, I don't see this. The read code already takes 0 bytes input
> and broken pipe scenarios into account. Do you see something needing
> a change I don't?
Never mind, it doesn't work quite as expected, so changes would be
required.
I created another version of your patch which avoids duplicating the
tested handle and makes the test-and-close-handle operation atomic:
* fhandler_tty.cc (fhandler_pty_common::close): Don't close output_mutex
here. Move into callers.
(fhandler_pty_master::close): Use NtQueryObject instead of
PeekNamedPipe to detect closing the last master handle.
diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index e91b3e3..12f6124 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -552,7 +552,10 @@ fhandler_pty_slave::close ()
get_output_handle_cyg ());
if ((unsigned) myself->ctty =3D=3D FHDEV (DEV_PTYS_MAJOR, get_minor ()))
fhandler_console::free_console (); /* assumes that we are the last pty=
closer */
- return fhandler_pty_common::close ();
+ fhandler_pty_common::close ();
+ if (!ForceCloseHandle (output_mutex))
+ termios_printf ("CloseHandle (output_mutex<%p>), %E", output_mutex);
+ return 0;
}
=20
int
@@ -1259,8 +1262,6 @@ fhandler_pty_common::close ()
termios_printf ("pty%d <%p,%p> closing", get_minor (), get_handle (), ge=
t_output_handle ());
if (!ForceCloseHandle (input_mutex))
termios_printf ("CloseHandle (input_mutex<%p>), %E", input_mutex);
- if (!ForceCloseHandle (output_mutex))
- termios_printf ("CloseHandle (output_mutex<%p>), %E", output_mutex);
if (!ForceCloseHandle1 (get_handle (), from_pty))
termios_printf ("CloseHandle (get_handle ()<%p>), %E", get_handle ());
if (!ForceCloseHandle1 (get_output_handle (), to_pty))
@@ -1281,6 +1282,9 @@ fhandler_pty_master::cleanup ()
int
fhandler_pty_master::close ()
{
+ OBJECT_BASIC_INFORMATION obi;
+ NTSTATUS status;
+
termios_printf ("closing from_master(%p)/to_master(%p)/to_master_cyg(%p)=
since we own them(%u)",
from_master, to_master, to_master_cyg, dwProcessId);
if (cygwin_finished_initializing)
@@ -1309,13 +1313,22 @@ fhandler_pty_master::close ()
}
}
=20
- fhandler_pty_common::close ();
-
/* Check if the last master handle has been closed. If so, set
input_available_event to wake up potentially waiting slaves. */
- if (!PeekNamedPipe (from_master, NULL, 0, NULL, NULL, NULL)
- && GetLastError () =3D=3D ERROR_BROKEN_PIPE)=20
- SetEvent (input_available_event);
+ acquire_output_mutex (INFINITE);
+ status =3D NtQueryObject (get_output_handle (), ObjectBasicInformation,
+ &obi, sizeof obi, NULL);
+ fhandler_pty_common::close ();
+ release_output_mutex ();
+ if (!ForceCloseHandle (output_mutex))
+ termios_printf ("CloseHandle (output_mutex<%p>), %E", output_mutex);
+ if (!NT_SUCCESS (status))
+ debug_printf ("NtQueryObject: %y", status);
+ else if (obi.HandleCount =3D=3D 1)
+ {
+ termios_printf("Closing last master");
+ SetEvent (input_available_event);
+ }
=20
if (!ForceCloseHandle (from_master))
termios_printf ("error closing from_master %p, %E", from_master);
Does that look ok? It fixes the reported problem for me.
Thanks,
Corinna
--=20
Corinna Vinschen Please, send mails regarding Cygwin to
Cygwin Maintainer cygwin AT cygwin DOT com
Red Hat
--vkogqOf2sHV7VnPd
Content-Type: application/pgp-signature
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iQIcBAEBCAAGBQJVZwBQAAoJEPU2Bp2uRE+gp4kQAJaOduM6cV//3y9AyVzb4IMp
1x8UrtE7XHAucmrbw9iYfnYSA6aHp9sX6/cfJzQ2kncr6scxl6NNezWROD4UbigW
Zkv04cINdb27yRHefbwLAbJT/5p2PpKCxN4RTcIMTtLKDmFwmEaOgeVtXGhSzLKr
311vXvjUz/AOcgHEVulTkRt6SPu2iASSdE/M/XQ+pxkGYYWrm/nxBbPCAtPfa4pT
EIDOxHgo6yiMHmrUN+anfN7ug2Z2bVIa2kt+4apXgc5irRqc+0yOVmUPoZ9n/S/H
DGtx4FSGx8w1fhqkY1BlaPgXanJxlX2jP5xPqBNxD8V0ShmUDfLoFQdbLT/RBiZl
hDQTpseyc9n9HmU6vrcpSfgyKnF5afMIWXaqfYf8bmOyTxdwCZvnXP8neYAbojAJ
neLGV+glbUpLChc/ObjDbG9KrkGghLIuU/vAX7unTCUESNEy5iluM3mK2xbDKbbr
KQyTt1OipZl/VZi2AHPXXhIPr2uY4I+qUaR0vvKG/VmvXBlacKkCynafrYDQvfwz
NItpGtJ/baUQFhUJlezW+Ly8P+S+fljDPSRrCq7kkxZdZpy5mdwH48chKp6OWvnz
yDPci/MBWe2KR50vhcz8ELZcmRIj/wFoU3DJK7kOGE7MEPzPhnz4BDqjxw1qaAhI
w/xrYvZSYZiBC+lF4QKK
=hNoR
-----END PGP SIGNATURE-----
--vkogqOf2sHV7VnPd--
- Raw text -