Re: [PATCH v6 00/16] daemon-win32
From: Pat Thoyts <hidden>
Date: 2016-06-15 22:49:58
Subsystem:
the rest · Maintainer:
Linus Torvalds
Possibly related (same subject, not in this thread)
- 2016-06-15 · [PATCH v6 00/16] daemon-win32 · Erik Faye-Lund <hidden>
- 2016-06-15 · Re: [PATCH v6 00/16] daemon-win32 · Pat Thoyts <hidden>
- 2016-06-15 · Re: [PATCH v6 00/16] daemon-win32 · Erik Faye-Lund <hidden>
- 2016-06-15 · Re: [PATCH v6 00/16] daemon-win32 · Erik Faye-Lund <hidden>
- 2016-06-15 · Re: [PATCH v6 00/16] daemon-win32 · Erik Faye-Lund <hidden>
Erik Faye-Lund [off-list ref] writes:
On Wed, Nov 3, 2010 at 11:18 PM, Erik Faye-Lund [off-list ref] wrote:quoted
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 <redacted> 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 <redacted> --- 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