From: Nick Hengeveld <nickh@reactrix.com>
To: Petr Baudis <pasky@suse.cz>
Cc: Junio C Hamano <junkio@cox.net>, git@vger.kernel.org
Subject: Re: [PATCH] Fix bunch of fd leaks in http-fetch
Date: Sat, 12 Nov 2005 19:37:13 -0800 [thread overview]
Message-ID: <20051113033713.GC24546@reactrix.com> (raw)
In-Reply-To: <20051112195513.GF30496@pasky.or.cz>
On Sat, Nov 12, 2005 at 08:55:13PM +0100, Petr Baudis wrote:
> 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.
prev parent reply other threads:[~2005-11-14 16:30 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2005-11-11 22:58 file descriptor leak? or expected behavior? Becky Bruce
2005-11-11 23:22 ` Becky Bruce
2005-11-11 23:55 ` [PATCH] Fix bunch of fd leaks in http-fetch Petr Baudis
2005-11-12 5:45 ` Junio C Hamano
2005-11-12 17:38 ` Nick Hengeveld
2005-11-12 19:55 ` Petr Baudis
2005-11-13 3:37 ` Nick Hengeveld [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20051113033713.GC24546@reactrix.com \
--to=nickh@reactrix.com \
--cc=git@vger.kernel.org \
--cc=junkio@cox.net \
--cc=pasky@suse.cz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).