From: Yann Ylavic Date: Sat, 13 Jan 2018 15:36:38 +0000 (+0000) Subject: Merge r1802618, r1820808 from trunk: X-Git-Tag: 2.4.30~149 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=67281386ce16fe952a0aee349f59486275a96b94;p=apache Merge r1802618, r1820808 from trunk: core, mpm_event: Add ap_update_sb_handle() to avoid a small memory leak of sizeof(ap_sb_handle_t) when re-entering event's process_socket(). Follow up to r1802618: CHANGES entry. Proposed by: ylavic Reviewed by: ylavic, icing, jim git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1821069 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/CHANGES b/CHANGES index 2495bf7900..23ab298032 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,10 @@ -*- coding: utf-8 -*- Changes with Apache 2.4.30 + *) core, mpm_event: Avoid a small memory leak of the scoreboard handle, for + the lifetime of the connection, each time it is processed by MPM event. + [Yann Ylavic] + *) mpm_event: Update scoreboard status for KeepAlive state. [Yann Ylavic] *) mod_ldap: Fix a case where a full LDAP cache would continually fail to diff --git a/include/ap_mmn.h b/include/ap_mmn.h index 2764501833..48c19cb7fd 100644 --- a/include/ap_mmn.h +++ b/include/ap_mmn.h @@ -496,6 +496,7 @@ * to ap_[r]getline() * 20120211.68 (2.4.26-dev) Add ap_get_basic_auth_components() and deprecate * ap_get_basic_auth_pw() + * 20120211.69 (2.4.30-dev) Add ap_update_sb_handle() */ #define MODULE_MAGIC_COOKIE 0x41503234UL /* "AP24" */ @@ -503,7 +504,7 @@ #ifndef MODULE_MAGIC_NUMBER_MAJOR #define MODULE_MAGIC_NUMBER_MAJOR 20120211 #endif -#define MODULE_MAGIC_NUMBER_MINOR 68 /* 0...n */ +#define MODULE_MAGIC_NUMBER_MINOR 69 /* 0...n */ /** * Determine if the server's current MODULE_MAGIC_NUMBER is at least a diff --git a/include/scoreboard.h b/include/scoreboard.h index 1378128943..57cf3df132 100644 --- a/include/scoreboard.h +++ b/include/scoreboard.h @@ -176,6 +176,8 @@ AP_DECLARE(int) ap_calc_scoreboard_size(void); AP_DECLARE(void) ap_create_sb_handle(ap_sb_handle_t **new_sbh, apr_pool_t *p, int child_num, int thread_num); +AP_DECLARE(void) ap_update_sb_handle(ap_sb_handle_t *sbh, + int child_num, int thread_num); AP_DECLARE(int) ap_find_child_by_pid(apr_proc_t *pid); AP_DECLARE(int) ap_update_child_status(ap_sb_handle_t *sbh, int status, request_rec *r); diff --git a/server/mpm/event/event.c b/server/mpm/event/event.c index d456456a2a..3848279a5f 100644 --- a/server/mpm/event/event.c +++ b/server/mpm/event/event.c @@ -229,6 +229,8 @@ struct event_conn_state_t { request_rec *r; /** server config this struct refers to */ event_srv_cfg *sc; + /** scoreboard handle for the conn_rec */ + ap_sb_handle_t *sbh; /** is the current conn_rec suspended? (disassociated with * a particular MPM thread; for suspend_/resume_connection * hooks) @@ -733,14 +735,14 @@ static apr_status_t decrement_connection_count(void *cs_) static void notify_suspend(event_conn_state_t *cs) { ap_run_suspend_connection(cs->c, cs->r); - cs->suspended = 1; cs->c->sbh = NULL; + cs->suspended = 1; } -static void notify_resume(event_conn_state_t *cs, ap_sb_handle_t *sbh) +static void notify_resume(event_conn_state_t *cs, int cleanup) { - cs->c->sbh = sbh; cs->suspended = 0; + cs->c->sbh = cleanup ? NULL : cs->sbh; ap_run_resume_connection(cs->c, cs->r); } @@ -861,7 +863,7 @@ static apr_status_t ptrans_pre_cleanup(void *dummy) event_conn_state_t *cs = dummy; if (cs->suspended) { - notify_resume(cs, NULL); + notify_resume(cs, 1); } return APR_SUCCESS; } @@ -927,17 +929,14 @@ static void process_socket(apr_thread_t *thd, apr_pool_t * p, apr_socket_t * soc conn_rec *c; long conn_id = ID_FROM_CHILD_THREAD(my_child_num, my_thread_num); int rc; - ap_sb_handle_t *sbh; - - /* XXX: This will cause unbounded mem usage for long lasting connections */ - ap_create_sb_handle(&sbh, p, my_child_num, my_thread_num); if (cs == NULL) { /* This is a new connection */ listener_poll_type *pt = apr_pcalloc(p, sizeof(*pt)); cs = apr_pcalloc(p, sizeof(event_conn_state_t)); cs->bucket_alloc = apr_bucket_alloc_create(p); + ap_create_sb_handle(&cs->sbh, p, my_child_num, my_thread_num); c = ap_run_create_connection(p, ap_server_conf, sock, - conn_id, sbh, cs->bucket_alloc); + conn_id, cs->sbh, cs->bucket_alloc); if (!c) { ap_push_pool(worker_queue_info, p); return; @@ -989,7 +988,8 @@ static void process_socket(apr_thread_t *thd, apr_pool_t * p, apr_socket_t * soc } else { c = cs->c; - notify_resume(cs, sbh); + ap_update_sb_handle(cs->sbh, my_child_num, my_thread_num); + notify_resume(cs, 0); c->current_thread = thd; /* Subsequent request on a conn, and thread number is part of ID */ c->id = conn_id; @@ -1061,7 +1061,7 @@ read_request: if (cs->pub.state == CONN_STATE_WRITE_COMPLETION) { ap_filter_t *output_filter = c->output_filters; apr_status_t rv; - ap_update_child_status(sbh, SERVER_BUSY_WRITE, NULL); + ap_update_child_status(cs->sbh, SERVER_BUSY_WRITE, NULL); while (output_filter->next != NULL) { output_filter = output_filter->next; } @@ -1126,7 +1126,7 @@ read_request: start_lingering_close_blocking(cs); } else if (cs->pub.state == CONN_STATE_CHECK_REQUEST_LINE_READABLE) { - ap_update_child_status(sbh, SERVER_BUSY_KEEPALIVE, NULL); + ap_update_child_status(cs->sbh, SERVER_BUSY_KEEPALIVE, NULL); /* It greatly simplifies the logic to use a single timeout value per q * because the new element can just be added to the end of the list and diff --git a/server/scoreboard.c b/server/scoreboard.c index d83a8492c3..4343eba4b0 100644 --- a/server/scoreboard.c +++ b/server/scoreboard.c @@ -410,12 +410,18 @@ AP_DECLARE(int) ap_find_child_by_pid(apr_proc_t *pid) return -1; } +AP_DECLARE(void) ap_update_sb_handle(ap_sb_handle_t *sbh, + int child_num, int thread_num) +{ + sbh->child_num = child_num; + sbh->thread_num = thread_num; +} + AP_DECLARE(void) ap_create_sb_handle(ap_sb_handle_t **new_sbh, apr_pool_t *p, int child_num, int thread_num) { *new_sbh = (ap_sb_handle_t *)apr_palloc(p, sizeof(ap_sb_handle_t)); - (*new_sbh)->child_num = child_num; - (*new_sbh)->thread_num = thread_num; + ap_update_sb_handle(*new_sbh, child_num, thread_num); } static void copy_request(char *rbuf, apr_size_t rbuflen, request_rec *r)