You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Joe Orton <jo...@redhat.com> on 2003/03/06 16:31:05 UTC
[PATCH] fix fd leaks
Hi, here is a version of the patch in #17206 which removes the current
the fd leaks. Most of these were introduced in this commit
http://marc.theaimsgroup.com/?l=apache-cvs&m=99531770520998&w=2
though the pod leak has been around longer. I haven't checked whether
the mod_file_cache change in that commit should be reverted as well. The
patch is against 2.0 HEAD.
Submitted by: Christian Kratzer, Bjoern A. Zeeb
Index: modules/loggers/mod_log_config.c
===================================================================
RCS file: /store/cvs/root/httpd-2.0/modules/loggers/mod_log_config.c,v
retrieving revision 1.99
diff -u -r1.99 mod_log_config.c
--- modules/loggers/mod_log_config.c 14 Feb 2003 04:17:34 -0000 1.99
+++ modules/loggers/mod_log_config.c 6 Mar 2003 15:38:07 -0000
@@ -1291,7 +1291,6 @@
"could not open transfer log file %s.", fname);
return NULL;
}
- apr_file_inherit_set(fd);
return fd;
}
}
Index: modules/mappers/mod_rewrite.c
===================================================================
RCS file: /store/cvs/root/httpd-2.0/modules/mappers/mod_rewrite.c,v
retrieving revision 1.148
diff -u -r1.148 mod_rewrite.c
--- modules/mappers/mod_rewrite.c 1 Mar 2003 18:35:50 -0000 1.148
+++ modules/mappers/mod_rewrite.c 6 Mar 2003 15:38:08 -0000
@@ -3429,7 +3429,6 @@
"file %s", fname);
exit(1);
}
- apr_file_inherit_set(conf->rewritelogfp);
}
return;
}
Index: server/log.c
===================================================================
RCS file: /store/cvs/root/httpd-2.0/server/log.c,v
retrieving revision 1.130
diff -u -r1.130 log.c
--- server/log.c 10 Feb 2003 16:27:28 -0000 1.130
+++ server/log.c 6 Mar 2003 15:38:08 -0000
@@ -320,8 +320,6 @@
ap_server_argv0, fname);
return DONE;
}
-
- apr_file_inherit_set(s->error_log);
}
return OK;
Index: server/mpm_common.c
===================================================================
RCS file: /store/cvs/root/httpd-2.0/server/mpm_common.c,v
retrieving revision 1.103
diff -u -r1.103 mpm_common.c
--- server/mpm_common.c 3 Feb 2003 17:53:19 -0000 1.103
+++ server/mpm_common.c 6 Mar 2003 15:38:08 -0000
@@ -410,6 +410,10 @@
apr_sockaddr_info_get(&(*pod)->sa, ap_listeners->bind_addr->hostname,
APR_UNSPEC, ap_listeners->bind_addr->port, 0, p);
+ /* close these before exec. */
+ apr_file_unset_inherit((*pod)->pod_in);
+ apr_file_unset_inherit((*pod)->pod_out);
+
return APR_SUCCESS;
}
Index: server/mpm/worker/pod.c
===================================================================
RCS file: /store/cvs/root/httpd-2.0/server/mpm/worker/pod.c,v
retrieving revision 1.7
diff -u -r1.7 pod.c
--- server/mpm/worker/pod.c 3 Feb 2003 17:53:26 -0000 1.7
+++ server/mpm/worker/pod.c 6 Mar 2003 15:38:08 -0000
@@ -76,6 +76,10 @@
*/
(*pod)->p = p;
+ /* close these before exec. */
+ apr_file_unset_inherit((*pod)->pod_in);
+ apr_file_unset_inherit((*pod)->pod_out);
+
return APR_SUCCESS;
}
Re: [PATCH] fix fd leaks
Posted by Joe Orton <jo...@redhat.com>.
On Thu, Mar 06, 2003 at 05:33:39PM +0000, Bjoern A. Zeeb wrote:
> On Thu, 6 Mar 2003, Joe Orton wrote:
>
> Hi,
>
> > Submitted by: Christian Kratzer, Bjoern A. Zeeb
> >
> ...
> > -
> > - apr_file_inherit_set(s->error_log);
> > }
>
>
> so now we are back to that point that needs further discussing.
>
> Should we simply remove apr_file_inherit_set or explicitly call
> apr_file_unset_inherit ? Simply removing works as long as defaults in
> APR remain but there is still p.ex.:
> if (!(flag & APR_FILE_NOCLEANUP)){
> that can again prevent the child_cleanup_fn from being set (seems to
> unused in open at the moment).
>
> It seems to me that the apr_file_inherit_set had been in because up
> to including httpd 2.0.39 a bug in apr prevented _set/_unset from
> working as expected (the 2^24 vs. 2<<24 flag check).
> Thus _set did the correct thing that _unset should have done.
> If the _sets had been in for some specific reason we sould perhaps
> use _unset.
> So there needs to be further tracking on why the _sets had been
> committed to apache code.
AFAICT they were added in the commit I referenced, under the guise of
the APR_INHERIT flag to apr_file_open, before the _inherit_set/unset API
was introduced:
http://marc.theaimsgroup.com/?l=apache-cvs&m=99531770520998&w=2
the comment being simply that it was "obvious" that these fds should be
inherited. Maybe Bill Rowe can elucidate on that, but I suspect it was
just a mistake.
Regards,
joe
Re: [PATCH] fix fd leaks
Posted by "Bjoern A. Zeeb" <bz...@lists.zabbadoz.net>.
On Thu, 6 Mar 2003, Joe Orton wrote:
Hi,
> Submitted by: Christian Kratzer, Bjoern A. Zeeb
>
...
> -
> - apr_file_inherit_set(s->error_log);
> }
so now we are back to that point that needs further discussing.
Should we simply remove apr_file_inherit_set or explicitly call
apr_file_unset_inherit ? Simply removing works as long as defaults in
APR remain but there is still p.ex.:
if (!(flag & APR_FILE_NOCLEANUP)){
that can again prevent the child_cleanup_fn from being set (seems to
unused in open at the moment).
It seems to me that the apr_file_inherit_set had been in because up
to including httpd 2.0.39 a bug in apr prevented _set/_unset from
working as expected (the 2^24 vs. 2<<24 flag check).
Thus _set did the correct thing that _unset should have done.
If the _sets had been in for some specific reason we sould perhaps
use _unset.
So there needs to be further tracking on why the _sets had been
committed to apache code.
Comments ?
--
Bjoern A. Zeeb bzeeb at Zabbadoz dot NeT
56 69 73 69 74 http://www.zabbadoz.net/