apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Plüm, Rüdiger, VF-Group" <ruediger.pl...@vodafone.com>
Subject RE: File descriptor leak with mpm-event / apr file bucket cleanup
Date Tue, 18 May 2010 07:57:39 GMT
 

> -----Original Message-----
> From: Stefan Fritsch [mailto:sf@sfritsch.de] 
> 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 
> bucket cleanup
> 
> 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 = 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 
> processing in the
> >   filter chain.
> > 3. The first bucket is processed and destroyed. This would 
> leave the second
> >   file bucket with an closed file descriptor.
> >
> > So if you want to close this fd you IMHO would need to do 
> some refcounting
> > and only close it if no other filebucket still references it.
> 
> The filebuckets already do refcounting. 
> apr_bucket_shared_destroy(f) in 
> the patch above is only true if the refcount has reached zero.
> 

Good point. I missed that.

Regards

Rüdiger

Mime
View raw message