Erik Faye-Lund <kusmabite@xxxxxxxxx> writes:
>On Wed, Nov 3, 2010 at 11:18 PM, Erik Faye-Lund <kusmabite@xxxxxxxxx> wrote:
>> Ah, thanks. For me it's leaking a variable amount of handles per
>> ls-remote, but if I apply the following patch it's down to one. Need
>> to find that one as well...
>>
>> diff --git a/compat/mingw.c b/compat/mingw.c
>> index b780200..47e7d26 100644
>> --- a/compat/mingw.c
>> +++ b/compat/mingw.c
>> @@ -1519,8 +1519,10 @@ pid_t waitpid(pid_t pid, int *status, unsigned options)
>> }
>>
>> if (pid > 0 && options & WNOHANG) {
>> - if (WAIT_OBJECT_0 != WaitForSingleObject((HANDLE)pid, 0))
>> + if (WAIT_OBJECT_0 != WaitForSingleObject((HANDLE)pid, 0)) {
>
>AAAND the last one is right here as well:
>- if (WAIT_OBJECT_0 != WaitForSingleObject((HANDLE)pid, 0))
>+ if (WAIT_OBJECT_0 != WaitForSingleObject(h, 0)) {
>
Applying both of these doesn't fix the handle leak when I test
this. Looking further I believe it is due to the use of a reallocated
array. When you remove a pinfo structure you realloc to the one you
found, potentially freeing items you still require.
Attached is a patch that switches to a linked list for this
instead. Using this I no longer accumulate leaked handles.
----
>From e0c05f8f9ed8729648eea92cf654f357fa884e40 Mon Sep 17 00:00:00 2001
From: Pat Thoyts <patthoyts@xxxxxxxxxxxxxxxxxxxxx>
Date: Thu, 4 Nov 2010 00:23:08 +0000
Subject: [PATCH] win32-daemon: fix handle leaks
The use of an array of pinfo structures and the realloc used when cleaning
up a closed child can free structures that are still in use. Use a linked
list instead.
This stops the leaking of handles in the win32-daemon.
Signed-off-by: Pat Thoyts <patthoyts@xxxxxxxxxxxxxxxxxxxxx>
---
compat/mingw.c | 43 ++++++++++++++++++++++++-------------------
1 files changed, 24 insertions(+), 19 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index b780200..29f4036 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -637,11 +637,12 @@ static int env_compare(const void *a, const void *b)
return strcasecmp(*ea, *eb);
}
-struct {
+struct pinfo_t {
+ struct pinfo_t *next;
pid_t pid;
HANDLE proc;
-} *pinfo;
-static int num_pinfo;
+} pinfo_t;
+struct pinfo_t *pinfo = NULL;
CRITICAL_SECTION pinfo_cs;
static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **env,
@@ -746,10 +747,13 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **env,
* Keep the handle in a list for waitpid.
*/
EnterCriticalSection(&pinfo_cs);
- num_pinfo++;
- pinfo = xrealloc(pinfo, sizeof(*pinfo) * num_pinfo);
- pinfo[num_pinfo - 1].pid = pi.dwProcessId;
- pinfo[num_pinfo - 1].proc = pi.hProcess;
+ {
+ struct pinfo_t *info = xmalloc(sizeof(struct pinfo_t));
+ info->pid = pi.dwProcessId;
+ info->proc = pi.hProcess;
+ info->next = pinfo;
+ pinfo = info;
+ }
LeaveCriticalSection(&pinfo_cs);
return (pid_t)pi.dwProcessId;
@@ -1519,13 +1523,15 @@ pid_t waitpid(pid_t pid, int *status, unsigned options)
}
if (pid > 0 && options & WNOHANG) {
- if (WAIT_OBJECT_0 != WaitForSingleObject((HANDLE)pid, 0))
+ if (WAIT_OBJECT_0 != WaitForSingleObject(h, 0)) {
+ CloseHandle(h);
return 0;
+ }
options &= ~WNOHANG;
}
if (options == 0) {
- int i;
+ struct pinfo_t **ppinfo;
if (WaitForSingleObject(h, INFINITE) != WAIT_OBJECT_0) {
CloseHandle(h);
return 0;
@@ -1536,17 +1542,16 @@ pid_t waitpid(pid_t pid, int *status, unsigned options)
EnterCriticalSection(&pinfo_cs);
- for (i = 0; i < num_pinfo; ++i)
- if (pinfo[i].pid == pid)
+ ppinfo = &pinfo;
+ while (*ppinfo) {
+ struct pinfo_t *info = *ppinfo;
+ if (info->pid == pid) {
+ CloseHandle(info->proc);
+ *ppinfo = info->next;
+ free(info);
break;
-
- if (i < num_pinfo) {
- CloseHandle(pinfo[i].proc);
- memmove(pinfo + i, pinfo + i + 1,
- sizeof(*pinfo) * (num_pinfo - i - 1));
- num_pinfo--;
- pinfo = xrealloc(pinfo,
- sizeof(*pinfo) * num_pinfo);
+ }
+ ppinfo = &info->next;
}
LeaveCriticalSection(&pinfo_cs);
--
1.7.3.1.210.g3526b.dirty
--
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