Re: [PATCH v6 00/16] daemon-win32

On Wed, Nov 3, 2010 at 10:11 PM, Pat Thoyts
[off-list ref] wrote:

Erik Faye-Lund [off-list ref] writes:

quoted

Here's hopefully the last iteration of this series. The previous version
only got a single complain about a typo in the subject of patch 14/15, so
it seems like most controversies have been settled.

I pulled this win32-daemon branch into my msysgit build tree and built
it. I get the following warnings:

   CC daemon.o
daemon.c: In function 'service_loop':
daemon.c:674: warning: dereferencing pointer 'ss.124' does break strict-aliasing rules
daemon.c:676: warning: dereferencing pointer 'ss.124' does break strict-aliasing rules
daemon.c:681: warning: dereferencing pointer 'ss.124' does break strict-aliasing rules
daemon.c:919: note: initialized from here
daemon.c:679: warning: dereferencing pointer 'sin_addr' does break strict-aliasing rules
daemon.c:675: note: initialized from here
daemon.c:691: warning: dereferencing pointer 'sin6_addr' does break strict-aliasing rules
daemon.c:682: note: initialized from here

Yeah, I'm aware of these. I thought those warnings were already
present in the Linux build, but checking again I see that that's not
the case. Need to investigate.

Otherwise it builds clean. The daemon running on Windows7 seems to be
working fine for both ipv4 and ipv6 connections (I tried both).

However, monitoring the resource usage in procexp it looks like there is
a handle leak. Each 'git ls-remote' over ipv6 is gaining 16 handles that
do not appear to be released. They're all process handles for dead
processes it looks like, so possibly there is a missing waitpid() or
something similar for the 'git daemon -serve' subprocess. Doing this
over ipv4 leaks 2 handles per request.

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)) {
+			CloseHandle(h);
 			return 0;
+		}
 		options &= ~WNOHANG;
 	}