You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@httpd.apache.org by GitBox <gi...@apache.org> on 2021/07/16 09:48:15 UTC
[GitHub] [httpd] rpluem commented on a change in pull request #208: Event child graceful stop
rpluem commented on a change in pull request #208:
URL: https://github.com/apache/httpd/pull/208#discussion_r671047138
##########
File path: server/mpm/event/event.c
##########
@@ -1020,14 +1011,14 @@ static void process_socket(apr_thread_t *thd, apr_pool_t * p, apr_socket_t * soc
apr_pool_cleanup_null);
ap_set_module_config(c->conn_config, &mpm_event_module, cs);
c->current_thread = thd;
+ c->cs = &cs->pub;
cs->c = c;
- c->cs = &(cs->pub);
cs->p = p;
Review comment:
Why changing the order?
##########
File path: server/mpm/event/event.c
##########
@@ -1961,13 +2004,21 @@ static void * APR_THREAD_FUNC listener_thread(apr_thread_t * thd, void *dummy)
num = 0;
}
- if (listener_may_exit) {
- close_listeners(&closed);
- if (terminate_mode == ST_UNGRACEFUL
- || apr_atomic_read32(&connection_count) == 0)
- break;
+ if (APLOGtrace7(ap_server_conf)) {
+ now = apr_time_now();
Review comment:
Is it wise to use `now` here or should we use a new temporary variable here to use it for the `ap_log_error` parameters? Otherwise trace7 logs have side effects.
##########
File path: server/mpm/event/event.c
##########
@@ -1189,17 +1180,19 @@ static void process_socket(apr_thread_t *thd, apr_pool_t * p, apr_socket_t * soc
}
if (pending != DECLINED
|| c->aborted
- || c->keepalive != AP_CONN_KEEPALIVE
- || listener_may_exit) {
+ || c->keepalive != AP_CONN_KEEPALIVE) {
cs->pub.state = CONN_STATE_LINGER;
}
else if (ap_run_input_pending(c) == OK) {
cs->pub.state = CONN_STATE_READ_REQUEST_LINE;
goto read_request;
}
- else {
Review comment:
Why now run `ap_run_input_pending` when `listener_may_exit` is set?
##########
File path: server/mpm/event/event.c
##########
@@ -1228,8 +1221,8 @@ static void process_socket(apr_thread_t *thd, apr_pool_t * p, apr_socket_t * soc
ap_log_error(APLOG_MARK, APLOG_ERR, rv, ap_server_conf, APLOGNO(03093)
"process_socket: apr_pollset_add failure for "
"keep alive");
- apr_socket_close(cs->pfd.desc.s);
- ap_queue_info_push_pool(worker_queue_info, cs->p);
+ close_connection(cs);
+ signal_threads(ST_GRACEFUL);
Review comment:
Why now shuting down the process if adding to the pollset failed?
##########
File path: server/mpm/event/event.c
##########
@@ -2249,7 +2303,6 @@ static void * APR_THREAD_FUNC listener_thread(apr_thread_t * thd, void *dummy)
}
} /* listener main loop */
- close_listeners(&closed);
Review comment:
Why is this no longer needed?
##########
File path: server/mpm/event/event.c
##########
@@ -2020,25 +2071,21 @@ static void * APR_THREAD_FUNC listener_thread(apr_thread_t * thd, void *dummy)
AP_DEBUG_ASSERT(0);
ap_log_error(APLOG_MARK, APLOG_ERR, rc, ap_server_conf,
APLOGNO(03094) "pollset remove failed");
- start_lingering_close_nonblocking(cs);
+ close_connection(cs);
+ signal_threads(ST_GRACEFUL);
Review comment:
Why now shuting down the process if removing the pollset failed?
##########
File path: server/mpm/event/event.c
##########
@@ -1921,37 +1961,40 @@ static void * APR_THREAD_FUNC listener_thread(apr_thread_t * thd, void *dummy)
}
/* Same for queues, use their next expiry, if any. */
- timeout_time = queues_next_expiry;
- if (timeout_time
- && (timeout_interval < 0
- || timeout_time <= now
- || timeout_interval > timeout_time - now)) {
- timeout_interval = timeout_time > now ? timeout_time - now : 1;
+ expiry = queues_next_expiry;
+ if (expiry
+ && (timeout < 0
+ || expiry <= now
+ || timeout > expiry - now)) {
+ timeout = expiry > now ? expiry - now : 0;
Review comment:
Why now `0` in case `expiry <= now` instead of `1` before?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For additional commands, e-mail: notifications-help@httpd.apache.org