On Sat, Jan 16, 2010 at 9:03 AM, Johannes Sixt <j6t@xxxxxxxx> wrote: > On Freitag, 15. Januar 2010, Erik Faye-Lund wrote: >> On Fri, Jan 15, 2010 at 11:30 PM, Johannes Sixt <j6t@xxxxxxxx> 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html