DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id
	:list-unsubscribe:list-subscribe:list-archive:list-post
	:list-help:sender:date:from:to:cc:subject:message-id:reply-to
	:references:mime-version:content-type:in-reply-to; q=dns; s=
	default; b=U+Q32LsNH0F0p6/D59IF55zgJKEVezcMyzPgyn4+urzNqmWYjM2Z6
	QV3PH393HzZ74aEDAgtCRZxjrf3QgJiTe9XUieBb/+V/HRMpeIA6LcW/NyJ+XXzq
	L+tTNCNc5s8UvAboNd07APrRxoybh0Pwo51yOXhGD6FKJNR4qYRv/U=
Mailing-List: contact cygwin-help@cygwin.com; run by ezmlm
Sender: cygwin-owner@cygwin.com
Mail-Followup-To: cygwin@cygwin.com
Date: Tue, 5 Feb 2019 10:44:55 +0100
From: Corinna Vinschen <corinna-cygwin@cygwin.com>
To: Michael Haubenwallner <michael.haubenwallner@ssi-schaefer.com>
Cc: cygwin@cygwin.com
Subject: Re: [ANNOUNCEMENT] TEST: Cygwin 3.0.0-0.3
Message-ID: <20190205094455.GQ3532@calimero.vinschen.de>
Reply-To: cygwin@cygwin.com
Mail-Followup-To: Michael Haubenwallner <michael.haubenwallner@ssi-schaefer.com>,	cygwin@cygwin.com
References: <announce.20190130212227.GP3912@calimero.vinschen.de> <19c5e653-890a-7c52-fde8-80df137223c9@ssi-schaefer.com> <20190131194813.GT3912@calimero.vinschen.de> <20190203111937.GG3532@calimero.vinschen.de> <65068741-b05e-591f-1577-cab12650bee6@ssi-schaefer.com> <20190204142538.GN3532@calimero.vinschen.de> <40c1cbd8-be88-86d6-2b66-ded51129f2c3@ssi-schaefer.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;	protocol="application/pgp-signature"; boundary="/2994txjAzEdQwm5"
Content-Disposition: inline
In-Reply-To: <40c1cbd8-be88-86d6-2b66-ded51129f2c3@ssi-schaefer.com>
User-Agent: Mutt/1.10.1 (2018-07-13)

--/2994txjAzEdQwm5
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Feb  5 09:42, Michael Haubenwallner wrote:
> On 2/4/19 3:25 PM, Corinna Vinschen wrote:
> > Are you going to test the patched branch?
>=20
> Sorry, was indeed unclear: Yes, of course!
> Will start testing on Server 2012 while setting up a 2019 VM.
>=20
> For now, there's already this one patch I've been using with good success,
> please add it to topic/forkables - the suspended thing is something diffe=
rent:
> https://cygwin.com/ml/cygwin-patches/2018-q2/msg00039.html

The collision problem shouldn't be as bad anymore with 3.0, given the
new PID handling.  However, after spending a bit more time in the fork
code, it looks like not releasing the procinfo in the error case is a
generic problem so I'm inclined to apply it to master.

While at it, there are quite a few spots in the code which end up
jumping to the cleanup code but only one of them calls TerminateProcess.
Wouldn't it make sense to move the TerminateProcess call into the
cleanup code to make sure the child process doesn't stay running
in some limbo state, not doing anything useful but not dying either?

Kind of like this:

diff --git a/winsup/cygwin/fork.cc b/winsup/cygwin/fork.cc
index 6f00364334c3..5f775249a990 100644
--- a/winsup/cygwin/fork.cc
+++ b/winsup/cygwin/fork.cc
@@ -400,7 +400,6 @@ frok::parent (volatile char * volatile stack_here)
      we can't actually record the pid in the internal table. */
   if (!child.remember (false))
     {
-      TerminateProcess (hchild, 1);
       this_errno =3D EAGAIN;
 #ifdef DEBUGGING0
       error ("child remember failed");
@@ -508,8 +507,12 @@ cleanup:
     __malloc_unlock ();
=20
   /* Remember to de-allocate the fd table. */
-  if (hchild && !child.hProcess) /* no child.procinfo */
-    ForceCloseHandle1 (hchild, childhProc);
+  if (hchild)
+    {
+      TerminateProcess (hchild, 1);
+      if (!child.hProcess) /* no child.procinfo */
+	ForceCloseHandle1 (hchild, childhProc);
+    }
   if (forker_finished)
     ForceCloseHandle (forker_finished);
   debug_printf ("returning -1");


Corinna

--=20
Corinna Vinschen
Cygwin Maintainer

--/2994txjAzEdQwm5
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEEoVYPmneWZnwT6kwF9TYGna5ET6AFAlxZWxcACgkQ9TYGna5E
T6BzGRAAiwZJL/CMYv3alpeOxSuAIIAz+LV54bMBK4kIKAIFeux0EPDqw0Tdh/ot
yiDpEoSRVjmfssiTIbbCnMKIRImFDlrUoxZQ+AXGn3ySEItwyjHZvEdUI2JEeAVe
VU0ZeXFqN3ve9aijMLfuxTU8CvDPtFrYyUMqyCb4d2KQBrWDluc3HwGWiVAMFRIp
0YrNktVU62ooziQQQSQqld3CpeL54XTb+e9tvtVWQHH360fmX2MsydWGkR+He2bh
Mv/zGTEEZauf2YAy3OFlRinApduE4wAaUwrgaLpEPoqAl+TTlFn5354WjJqXmT77
OHOjRMHiaeBuN8JBI2V1EhgFNl6J6L6gHj/xhNlPdx7jBMl5FYSUzvTWmEHxhi0z
3NL6QovJFLuP+sWGPDMZ4bhSZvFj80sCiGpnLPx1HtRuPaqETqLxyn9DMjO1B/0c
7VpSR2IDVpw7l5cS+vX8mkgP829M1xXEwhFxfRU41GBkd8MWKf18T0KtaG7CjqGB
h+FLapbyTBZjFcX3WcZdJt3k6dKGbUTZUdRjyuFnXBY2rYgZ6W9WiCTJKYOvELaq
pdQI5hkjXCfH4UGQlcaaTZPKkEGV7klS8+a8OEJ5KGH9wDmUIbQ/6ti3/MBa4mwI
WETwbGfig54NY7wBnUlpGs3miL4KArCKn3ZXF4WQ/8j9TDZwZI4=
=Qf4t
-----END PGP SIGNATURE-----

--/2994txjAzEdQwm5--