httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeff Trawick <traw...@gmail.com>
Subject Re: svn commit: r933657 - /httpd/httpd/trunk/os/unix/unixd.c
Date Tue, 13 Apr 2010 20:26:14 GMT
On Tue, Apr 13, 2010 at 1:16 PM, Ruediger Pluem <rpluem@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?

Mime
View raw message