You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by "William A. Rowe, Jr." <wr...@rowe-clan.net> on 2003/03/30 23:41:39 UTC
Re: [PATCH] don't try to clean up listening sockets twice
prior to exec
++1 - makes perfect sense.
One more oddball observation. Why repeatedly invoke the cleanups?
We need only a few handles in cgid, it seems that setting those few
handles aside and invoking cleanup_for_exec ourselves before we enter
the server loop in the fork()ed cgid worker would speed things up quite
considerably, over running cleanups hundreds of times.
Bill
At 11:07 AM 3/30/2003, you wrote:
>I'm not seeing any "close(-1)" calls on a trace of a mod_cgid-invoked script anymore now.
>
>old trace (in the new script child prior to exec):
>
>5108: close(4) = 0
>5108: close(3) = 0
>5108: close(-1) Err#9 EBADF
>5108: close(-1) Err#9 EBADF
>5108: close(9) = 0
>5108: close(6) = 0
>5108: close(8) = 0
>5108: close(7) = 0
>5108: close(13) = 0
>5108: fcntl(12, F_DUP2FD, 0x00000000) = 0
>5108: close(12) = 0
>5108: close(14) = 0
>5108: fcntl(15, F_DUP2FD, 0x00000001) = 1
>5108: close(15) = 0
>5108: close(16) = 0
>5108: fcntl(17, F_DUP2FD, 0x00000002) = 2
>5108: close(17) = 0
>5108: sigaction(SIGCLD, 0xFFBEF5C8, 0xFFBEF6D0) = 0
>5108: chdir("/export/home/trawick/apacheinst/cgi-bin/") = 0
>
>new trace:
>
>6616: close(4) = 0
>6616: close(3) = 0
>6616: close(9) = 0
>6616: close(6) = 0
>6616: close(8) = 0
>6616: close(7) = 0
>6616: close(13) = 0
>6616: fcntl(12, F_DUP2FD, 0x00000000) = 0
>6616: close(12) = 0
>6616: close(14) = 0
>6616: fcntl(15, F_DUP2FD, 0x00000001) = 1
>6616: close(15) = 0
>6616: close(16) = 0
>6616: fcntl(17, F_DUP2FD, 0x00000002) = 2
>6616: close(17) = 0
>6616: sigaction(SIGCLD, 0xFFBEF5C8, 0xFFBEF6D0) = 0
>6616: chdir("/export/home/trawick/apacheinst/cgi-bin/") = 0
>
>I'd been seeing these bogus closes for eons, but thought they were associated with pipes until I stepped through the cleanup-for-exec path early this a.m..
>
>
>
>Index: server/listen.c
>===================================================================
>RCS file: /home/cvs/httpd-2.0/server/listen.c,v
>retrieving revision 1.83.2.2
>diff -u -r1.83.2.2 listen.c
>--- server/listen.c 3 Feb 2003 17:32:00 -0000 1.83.2.2
>+++ server/listen.c 30 Mar 2003 16:55:24 -0000
>@@ -340,6 +340,8 @@
> ap_listen_rec *lr;
> ap_listen_rec *next;
> int num_open;
>+ const char *userdata_key = "ap_listen_open";
>+ void *data;
>
> /* Don't allocate a default listener. If we need to listen to a
> * port, then the user needs to have a Listen directive in their
>@@ -370,8 +372,17 @@
> }
> old_listeners = NULL;
>
>- apr_pool_cleanup_register(pool, NULL, apr_pool_cleanup_null,
>- close_listeners_on_exec);
>+ /* we come through here on both passes of the open logs phase
>+ * only register the cleanup once... otherwise we try to close
>+ * listening sockets twice when cleaning up prior to exec
>+ */
>+ apr_pool_userdata_get(&data, userdata_key, pool);
>+ if (!data) {
>+ apr_pool_userdata_set((const void *)1, userdata_key,
>+ apr_pool_cleanup_null, pool);
>+ apr_pool_cleanup_register(pool, NULL, apr_pool_cleanup_null,
>+ close_listeners_on_exec);
>+ }
>
> return num_open ? 0 : -1;
> }
Re: [PATCH] don't try to clean up listening sockets twice prior
to exec
Posted by Jeff Trawick <tr...@attglobal.net>.
William A. Rowe, Jr. wrote:
> ++1 - makes perfect sense.
>
> One more oddball observation. Why repeatedly invoke the cleanups?
>
> We need only a few handles in cgid, it seems that setting those few
> handles aside and invoking cleanup_for_exec ourselves before we enter
> the server loop in the fork()ed cgid worker would speed things up quite
> considerably, over running cleanups hundreds of times.
yeah, definitely some cleanup is needed as soon as cgid gets
initialized; I can't even explain the stuff that cgid has open on one of
my boxes
I'm not sure how cgid is going to have the info to cleanup the
unnecessary stuff or kill cleanups on the necessary stuff prior to
invoking cleanup-for-exec, but I guess that is a minor detail.