Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Received-SPF: pass (nike.apache.org: local policy)
Content-class: urn:content-classes:message
MIME-Version: 1.0
Content-Type: text/plain;
	charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
Subject: RE: File descriptor leak with mpm-event / apr file bucket cleanup
Date: Tue, 18 May 2010 09:57:39 +0200
Message-ID: 
 <99EA83DCDE961346AFA9B5EC33FEC08B03E6CB80@VF-MBX11.internal.vodafone.com>
In-reply-to: <alpine.DEB.1.10.1005180916300.30292@eru.sfritsch.de>
Thread-Topic: File descriptor leak with mpm-event / apr file bucket cleanup
Thread-Index: Acr2WmLflpwTZXMbSr6kyxVxNnnczQABUMuA
References: <201005172311.39558.sf@sfritsch.de> <4BF22B2E.5000304@apache.org>
 <alpine.DEB.1.10.1005180916300.30292@eru.sfritsch.de>
From: =?iso-8859-1?Q?=22Pl=FCm=2C_R=FCdiger=2C_VF-Group=22?=
 <ruediger.pluem@vodafone.com>
To: <dev@httpd.apache.org>
Cc: <dev@apr.apache.org>

=20

> -----Original Message-----
> From: Stefan Fritsch [mailto:sf@sfritsch.de]=20
> Sent: Dienstag, 18. Mai 2010 09:18
> To: dev@httpd.apache.org
> Cc: dev@apr.apache.org
> Subject: Re: File descriptor leak with mpm-event / apr file=20
> bucket cleanup
>=20
> On Tue, 18 May 2010, Ruediger Pluem wrote:
> >> --- buckets/apr_buckets_file.c.dist +0200
> >> +++ buckets/apr_buckets_file.c
> >> @@ -34,8 +34,7 @@
> >>      apr_bucket_file *f =3D data;
> >>
> >>      if (apr_bucket_shared_destroy(f)) {
> >> -        /* no need to close the file here; it will get
> >> -         * done automatically when the pool gets cleaned up */
> >> +        apr_file_close(f->fd);
> >>          apr_bucket_free(f);
> >>      }
> >>  }
> >>
> >>
> >
> > I guess this would be wrong. Think of the following scenario.
> >
> > 1. A brigade with a file bucket.
> > 2. The file bucket is split into two file buckets during=20
> processing in the
> >   filter chain.
> > 3. The first bucket is processed and destroyed. This would=20
> leave the second
> >   file bucket with an closed file descriptor.
> >
> > So if you want to close this fd you IMHO would need to do=20
> some refcounting
> > and only close it if no other filebucket still references it.
>=20
> The filebuckets already do refcounting.=20
> apr_bucket_shared_destroy(f) in=20
> the patch above is only true if the refcount has reached zero.
>=20

Good point. I missed that.

Regards

R=FCdiger