You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2010/04/13 19:16:19 UTC
Re: svn commit: r933657 - /httpd/httpd/trunk/os/unix/unixd.c
On 13.04.2010 16:58, trawick@apache.org wrote:
> Author: trawick
> Date: Tue Apr 13 14:58:03 2010
> New Revision: 933657
>
> URL: http://svn.apache.org/viewvc?rev=933657&view=rev
> Log:
> generalize the existing (r589761) platform- and errno-specific
> logic to suppress an error message if accept() fails after the
> socket has been marked inactive
>
> a message will still be logged, but at debug level instead of error
>
> PR: 49058
>
> Modified:
> httpd/httpd/trunk/os/unix/unixd.c
>
> Modified: httpd/httpd/trunk/os/unix/unixd.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/os/unix/unixd.c?rev=933657&r1=933656&r2=933657&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/os/unix/unixd.c (original)
> +++ httpd/httpd/trunk/os/unix/unixd.c Tue Apr 13 14:58:03 2010
> @@ -416,14 +416,15 @@ AP_DECLARE(apr_status_t) ap_unixd_accept
> #endif /*ENETDOWN*/
>
> default:
> -#ifdef _OSD_POSIX /* Possibly on other platforms too */
> /* If the socket has been closed in ap_close_listeners()
> * by the restart/stop action, we may get EBADF.
> * Do not print an error in this case.
> */
> - if (!lr->active && status == EBADF)
> + if (!lr->active) {
> + ap_log_error(APLOG_MARK, APLOG_DEBUG, status, ap_server_conf,
> + "apr_socket_accept failed for inactive listener");
> return status;
> -#endif
> + }
> ap_log_error(APLOG_MARK, APLOG_ERR, status, ap_server_conf,
> "apr_socket_accept: (client socket)");
> return APR_EGENERAL;
Hm, why do we return the real error code in case the socket is already closed
and APR_EGENERAL in the other case. Shouldn't this be consistent?
Regards
Rüdiger
Re: svn commit: r933657 - /httpd/httpd/trunk/os/unix/unixd.c
Posted by Jeff Trawick <tr...@gmail.com>.
On Tue, Apr 13, 2010 at 1:16 PM, Ruediger Pluem <rp...@apache.org> wrote:
> On 13.04.2010 16:58, trawick@apache.org wrote:
>> Author: trawick
>> Date: Tue Apr 13 14:58:03 2010
>> New Revision: 933657
>>
>> URL: http://svn.apache.org/viewvc?rev=933657&view=rev
>> Log:
>> generalize the existing (r589761) platform- and errno-specific
>> logic to suppress an error message if accept() fails after the
>> socket has been marked inactive
>>
>> a message will still be logged, but at debug level instead of error
>>
>> PR: 49058
>>
>> Modified:
>> httpd/httpd/trunk/os/unix/unixd.c
>>
>> Modified: httpd/httpd/trunk/os/unix/unixd.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/os/unix/unixd.c?rev=933657&r1=933656&r2=933657&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/os/unix/unixd.c (original)
>> +++ httpd/httpd/trunk/os/unix/unixd.c Tue Apr 13 14:58:03 2010
>> @@ -416,14 +416,15 @@ AP_DECLARE(apr_status_t) ap_unixd_accept
>> #endif /*ENETDOWN*/
>>
>> default:
>> -#ifdef _OSD_POSIX /* Possibly on other platforms too */
>> /* If the socket has been closed in ap_close_listeners()
>> * by the restart/stop action, we may get EBADF.
>> * Do not print an error in this case.
>> */
>> - if (!lr->active && status == EBADF)
>> + if (!lr->active) {
>> + ap_log_error(APLOG_MARK, APLOG_DEBUG, status, ap_server_conf,
>> + "apr_socket_accept failed for inactive listener");
>> return status;
>> -#endif
>> + }
>> ap_log_error(APLOG_MARK, APLOG_ERR, status, ap_server_conf,
>> "apr_socket_accept: (client socket)");
>> return APR_EGENERAL;
>
> Hm, why do we return the real error code in case the socket is already closed
> and APR_EGENERAL in the other case. Shouldn't this be consistent?
A failure when !lr->active isn't bad, and the MPM should be cleaning
up that process already.
APR_EGENERAL here means something bad happened and the MPM should
clean up the process for that reason.
prefork has this logic:
if (status == APR_EGENERAL) {
/* resource shortage or should-not-occur occured */
clean_child_exit(1);
}
It would exit with status 1 for APR_EGENERAL but exit with status 0 if
in the expected graceful restart/accept->EBADF case.
I guess a more formal interface w.r.t. failures could be helpful;
e.g., the MPM could see a different, specific return code if
!lr->active, or it could be responsible for first checking lr->active,
to distinguish between errors during process shutdown vs. real
problems.
Any further thoughts?