Web lists-archives.com

Re: [ANNOUNCEMENT] TEST: Cygwin 3.0.0-0.3




On 2/5/19 10:44 AM, Corinna Vinschen wrote:
> 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?
>>
>> Sorry, was indeed unclear: Yes, of course!
>> Will start testing on Server 2012 while setting up a 2019 VM.
>>
>> 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 different:
>> 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.

Heh, thanks - was my original intent back in 2018.

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

Seems to make sense indeed, and the suspended processes I do see sometimes
may well be related to that.

/haubi/

> 
> 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 = EAGAIN;
>  #ifdef DEBUGGING0
>        error ("child remember failed");
> @@ -508,8 +507,12 @@ cleanup:
>      __malloc_unlock ();
>  
>    /* 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
> 

Attachment: signature.asc
Description: OpenPGP digital signature