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

From: Erik Faye-Lund <kusmabite@gmail.com>
Date: Nov 3, 2010, 22:18 UTC
Message-ID: <AANLkTin2BCxDoKhDCSgSRG1eVUGVeQh6Bq_=FfAe0M_M@mail.gmail.com>
In-Reply-To: <877hgunmdc.fsf@fox.patthoyts.tk>

On Wed, Nov 3, 2010 at 10:11 PM, Pat Thoyts <patthoyts@users.sourceforge.net> wrote:

Show 20 quoted lines

> Erik Faye-Lund <kusmabite@gmail.com> writes:
>
>>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.

Show 10 quoted lines

> 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;
 	}