On Tue, Apr 13, 2010 at 1:16 PM, Ruediger Pluem <[email protected]> wrote:
> On 13.04.2010 16:58, [email protected] 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?
