X-Recipient: archive-cygwin AT delorie DOT com
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=
DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; 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; s=default;
	 bh=XencZtC+Kh43T/qNu4/Y2Z5sIBA=; b=OAKY9lUsJdHJnVVaG2SarwMnB+HP
	FdvWHT6QVY3kK93dj8gq7fdQvKwITidLd0v32ok8DcQ2LAwwhpGfbhyFBEkgr40Z
	JYqfOEXCpPnauJXAd1jR1H8zbT5hnrXmnKcuNHrG4tEw+yteDPV5pI7OSi7MSnA2
	c1j5tpc7brp9Ulg=
Mailing-List: contact cygwin-help AT cygwin DOT com; run by ezmlm
List-Id: <cygwin.cygwin.com>
List-Subscribe: <mailto:cygwin-subscribe AT cygwin DOT com>
List-Archive: <http://sourceware.org/ml/cygwin/>
List-Post: <mailto:cygwin AT cygwin DOT com>
List-Help: <mailto:cygwin-help AT cygwin DOT com>, <http://sourceware.org/ml/#faqs>
Sender: cygwin-owner AT cygwin DOT com
Mail-Followup-To: cygwin AT cygwin DOT com
Delivered-To: mailing list cygwin AT cygwin DOT com
Authentication-Results: sourceware.org; auth=none
X-Spam-SWARE-Status: No, score=-122.7 required=5.0 tests=BAYES_50,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,GOOD_FROM_CORINNA_CYGWIN,KAM_LAZY_DOMAIN_SECURITY,KAM_NUMSUBJECT,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 spammy=course!, suspended, Haubenwallner, haubenwallner
X-HELO: mout.kundenserver.de
Date: Tue, 5 Feb 2019 10:44:55 +0100
From: Corinna Vinschen <corinna-cygwin AT cygwin DOT com>
To: Michael Haubenwallner <michael DOT haubenwallner AT ssi-schaefer DOT com>
Cc: cygwin AT cygwin DOT com
Subject: Re: [ANNOUNCEMENT] TEST: Cygwin 3.0.0-0.3
Message-ID: <20190205094455.GQ3532@calimero.vinschen.de>
Reply-To: cygwin AT cygwin DOT com
Mail-Followup-To: Michael Haubenwallner <michael DOT haubenwallner AT ssi-schaefer DOT com>,	cygwin AT cygwin DOT com
References: <announce DOT 20190130212227 DOT GP3912 AT calimero DOT vinschen DOT de> <19c5e653-890a-7c52-fde8-80df137223c9 AT ssi-schaefer DOT com> <20190131194813 DOT GT3912 AT calimero DOT vinschen DOT de> <20190203111937 DOT GG3532 AT calimero DOT vinschen DOT de> <65068741-b05e-591f-1577-cab12650bee6 AT ssi-schaefer DOT com> <20190204142538 DOT GN3532 AT calimero DOT vinschen DOT de> <40c1cbd8-be88-86d6-2b66-ded51129f2c3 AT ssi-schaefer DOT 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--