httpd-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: svn commit: r569947 - /httpd/httpd/branches/2.2.x/STATUS
Date Mon, 27 Aug 2007 10:58:06 GMT


> -----Ursprüngliche Nachricht-----
> Von: William A. Rowe, Jr.
> Gesendet: Montag, 27. August 2007 10:28
> An: dev@httpd.apache.org
> Betreff: Re: svn commit: r569947 - /httpd/httpd/branches/2.2.x/STATUS
> 

> 
> So the model didn't work, and for NT I propose to stop inheriting the
> handles other than stdin/out/err.  Leave only the currently dup2()'ed
> handle as inherited, and temporarily uninherit even those, when they
> have been overridden.  Why leave them inherited at all?  For good old
> classic apps that still rely on classic inheritance.
> 
> Since we can't do this in Win9x, there is a trivial patch that makes
> the problem of holding open the old error log go away there, too.
> 
> --- server/log.c        (revision 569931)
> +++ server/log.c        (working copy)
> @@ -265,6 +265,10 @@
>      apr_proc_t *procnew;
>      apr_file_t *errfile;
> 
> +    if (dummy_stderr)
> +        if ((rc = apr_file_open_stderr(&errfile, p)) == APR_SUCCESS)
> +            apr_file_inherit_unset(errfile);
> +
>      if (((rc = apr_procattr_create(&procattr, p)) == APR_SUCCESS)
>          && ((rc = apr_procattr_cmdtype_set(procattr,
>                                             
> APR_SHELLCMD_ENV)) == APR_SUCCESS)
> 
> Essentially, before we play with proc_create, we tell apr 
> that we don't want
> stderr ever inherited by any process, ever again.  This is a 
> noop on NT, and
> just forces stderr to be closed on fork before exec on unix 
> (before, I trust,
> stdout replaces the stderr.)  So on unix, it's also 
> effectively a no-op.

I wouldn't say that it is a no-op on Unix. Some logger programs might
expect an open stderr, even if this points to /dev/null. So I am not in
favour of this patch. Besides I understood that we no longer support
Win9x. So why making an exception here?
IMHO if things do not work correctly on Win9x, bad luck.

> 
> 
> wrowe@apache.org wrote:
> > Author: wrowe
> > Date: Sun Aug 26 18:24:43 2007
> > New Revision: 569947
> > 
> > URL: http://svn.apache.org/viewvc?rev=569947&view=rev
> > Log:
> > Some final suggestions for 2.2.6
> > 
> > Modified:
> >     httpd/httpd/branches/2.2.x/STATUS
> > 
> > Modified: httpd/httpd/branches/2.2.x/STATUS
> > URL: 
> http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/STATUS
?rev=569947&r1=569946&r2=569947&view=diff
> ==============================================================================
> --- httpd/httpd/branches/2.2.x/STATUS (original)
> +++ httpd/httpd/branches/2.2.x/STATUS Sun Aug 26 18:24:43 2007
> @@ -215,6 +215,25 @@
>        http://svn.apache.org/viewvc?view=rev&revision=569622
>        +1: niq, rpluem
>   
> +    * log core: ensure we use a special pool for stderr logging, so that
> +      the stderr channel remains valid from the time plog is destroyed,
> +      until the time the open_logs hook is called again.  [William Rowe]
> +        http://svn.apache.org/viewvc?view=rev&revision=569535
> +      Backported to 2.2;
> +        http://people.apache.org/~wrowe/r569535-backport-2.2.patch
> +      +1: wrowe


Excerpt from http://people.apache.org/~wrowe/r569535-backport-2.2.patch

@@ -365,13 +409,19 @@
         /* replace stderr with this new log */
         apr_file_flush(s_main->error_log);
         if ((rc = apr_file_open_stderr(&errfile, p)) == APR_SUCCESS) {

This line no longer makes sense. We do not use errfile. So why doing apr_file_open_stderr
into errfile?

-            rc = apr_file_dup2(errfile, s_main->error_log, p);
+            rc = apr_file_dup2(stderr_log, s_main->error_log, p);

This is the wrong pool. It needs to be stderr_p instead of p.

         }
         if (rc != APR_SUCCESS) {
             ap_log_error(APLOG_MARK, APLOG_CRIT, rc, s_main,
                          "unable to replace stderr with error_log");
         }
         else {
+            /* We are done with stderr_pool, close it, killing
+             * the previous generation's stderr logger
+             */
+            if (stderr_pool)
+                apr_pool_destroy(stderr_pool);
+            stderr_pool = stderr_p;
             replace_stderr = 0;
         }
     }

Regards

Rüdiger


Mime
View raw message