On Sat, Jan 16, 2010 at 10:12 AM, Erik Faye-Lund
<kusmabite@xxxxxxxxxxxxxx> wrote:
> 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.
>
What I had in mind was something along these lines:
diff --git a/compat/mingw.c b/compat/mingw.c
index e2821b3..71201d0 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -638,6 +638,13 @@ static int env_compare(const void *a, const void *b)
return strcasecmp(*ea, *eb);
}
+struct pid_info {
+ pid_t pid;
+ HANDLE proc;
+};
+static struct pid_info *pinfo;
+static int num_pinfo;
+
static pid_t mingw_spawnve(const char *cmd, const char **argv, char **env,
int prepend_cmd)
{
@@ -729,6 +736,13 @@ static pid_t mingw_spawnve(const char *cmd, const
char **argv, char **env,
return -1;
}
CloseHandle(pi.hThread);
+
+ /* store process handle */
+ num_pinfo++;
+ pinfo = xrealloc(pinfo, sizeof(struct pid_info) * num_pinfo);
+ pinfo[num_pinfo - 1].pid = pi.dwProcessId;
+ pinfo[num_pinfo - 1].proc = pi.hProcess;
+
return (pid_t)pi.dwProcessId;
}
@@ -1536,6 +1550,7 @@ int waitpid(pid_t pid, int *status, unsigned options)
}
if (options == 0) {
+ int i;
if (WaitForSingleObject(h, INFINITE) != WAIT_OBJECT_0) {
CloseHandle(h);
return 0;
@@ -1544,6 +1559,19 @@ int waitpid(pid_t pid, int *status, unsigned options)
if (status)
GetExitCodeProcess(h, (LPDWORD)status);
+ for (i = 0; i < num_pinfo; ++i)
+ if (pinfo[i].pid == pid)
+ break;
+
+ if (i < num_pinfo) {
+ CloseHandle(pinfo[i].proc);
+ memmove(pinfo + i, pinfo + i + 1,
+ sizeof(struct pid_info) * (num_pinfo - i - 1));
+ num_pinfo--;
+ pinfo = xrealloc(pinfo,
+ sizeof(struct pid_info) * num_pinfo);
+ }
+
CloseHandle(h);
return pid;
}
--
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