Re: [PATCH] Fix bunch of fd leaks in http-fetch

From: Nick Hengeveld <nickh@reactrix.com>
Date: Nov 13, 2005, 03:37 UTC
Message-ID: <20051113033713.GC24546@reactrix.com>
In-Reply-To: <20051112195513.GF30496@pasky.or.cz>

On Sat, Nov 12, 2005 at 08:55:13PM +0100, Petr Baudis wrote:

Show 5 quoted lines

> What about the rest of the leaks?
> 
> Specifically, the one around release_request(), and the one caused by
> re-open()ing local in start_request() when re-calling it on existing
> request.

There should be a close before calling start_request() with an alternate - start doesn't currently check for an existing fd before it opens a new one. There should also be closes for the indexfile and packfile fds.

So our patches combined should take care of those cases and report on anything that may have been missed, which doesn't seem like a bad thing. The extra close in fetch_object shouldn't cause any problems because it only happens if there is still a valid fd in the request, which should never happen...

I'm pretty sure that fds are either already closed or were never opened prior to each call to release_request though. release is called in the following circumstances:

1) process_request_queue finds a WAITING request and the sha1 exists
   locally, no need to start() and the fd is never opened
2) fetch_object finds the sha1 exists locally, which means the object
   was already in the repo (same as #1) or prefetch got it loose
   and called finish
3) fetch_object finds an ABORTED request; requests abort when open fails
   or when the request didn't start (in which case the fd was closed
   after setting the request state)
4) fetch_object finds a request that isn't WAITING/ACTIVE/ABORTED
   (ie. COMPLETE);  finish is called when the request state is set
   to COMPLETE except in the aforementioned alternate restart case

-- 
For a successful technology, reality must take precedence over public
relations, for nature cannot be fooled.