Re: [PATCH v2 06/14] mingw: use real pid

[Erik Faye-Lund <kusmabite@googlemail.com>]

On Sat, Jan 16, 2010 at 9:03 AM, Johannes Sixt <j6t@kdbg.org> wrote:
> On Freitag, 15. Januar 2010, Erik Faye-Lund wrote:
>> On Fri, Jan 15, 2010 at 11:30 PM, Johannes Sixt <j6t@kdbg.org> wrote:
>> > On Freitag, 15. Januar 2010, Erik Faye-Lund wrote:
>> >> @@ -729,7 +729,7 @@ static pid_t mingw_spawnve(const char *cmd, const
>> >> char **argv, char **env, return -1;
>> >>       }
>> >>       CloseHandle(pi.hThread);
>> >> -     return (pid_t)pi.hProcess;
>> >> +     return (pid_t)pi.dwProcessId;
>> >>  }
>> >
>> > You are not using the pi.hProcess anymore, so you must close it.
>>
>> No. If I do, the pid becomes invalid after the process is finished,
>> and waitpid won't work. I couldn't find anywhere were we actually were
>> closing the handle, even after it was finished. So I don't think we
>> leak any more than we already did (for non-daemon purposes).
>
> Previously, this handle was closed by _cwait() (it was the "pid"), so we
> didn't leak it.

Oh, I see. My planned route with this (before I looked for where the
handle was closed), was to maintain some sort of list of each started
PID and their handle, and lookup in that list instead of using
OpenProcess. I guess that would solve the problem here, but it feels a
bit nasty. Not as nasty as introducing a leak, though.

>
> I somehow thought that you need the process ID instead of the handle for
> TerminateProcess, but now I see that this is not the case (it takes the
> handle). So I don't see the point of this change anymore. You say the process
> handle "does not work consistently with getpid", but I don't know what you
> mean. Please explain.

getpid() returns the real PID, and is used for prefixing each logged
message by git-daemon. However, the root process reports whenever a
new process is started or has terminated using the PID returned by
mingw_spawnve(), and this handle does not match up with the PID that
getpid() reports. Thus it becomes impossible to tell which reported
error belongs to which client.

-- 
Erik "kusma" Faye-Lund