From 21160f88645298756f317dc13312c860ee0bd76d Mon Sep 17 00:00:00 2001 From: Yann Ylavic Date: Fri, 21 Jul 2017 17:22:05 +0000 Subject: [PATCH] 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(). git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1802618 13f79535-47bb-0310-9956-ffa450edef68 --- include/ap_mmn.h | 3 ++- include/scoreboard.h | 2 ++ server/mpm/event/event.c | 24 ++++++++++++------------ server/scoreboard.c | 10 ++++++++-- 4 files changed, 24 insertions(+), 15 deletions(-) diff --git a/include/ap_mmn.h b/include/ap_mmn.h index ca2484cf12..526cd3d3da 100644 --- a/include/ap_mmn.h +++ b/include/ap_mmn.h @@ -554,6 +554,7 @@ * 20161018.4 (2.5.0-dev) Add taint to request_rec and ap_request_tainted() * 20161018.5 (2.5.0-dev) Add ap_get_basic_auth_components() and deprecate * ap_get_basic_auth_pw() + * 20161018.6 (2.5.0-dev) Add ap_update_sb_handle() */ #define MODULE_MAGIC_COOKIE 0x41503235UL /* "AP25" */ @@ -561,7 +562,7 @@ #ifndef MODULE_MAGIC_NUMBER_MAJOR #define MODULE_MAGIC_NUMBER_MAJOR 20161018 #endif -#define MODULE_MAGIC_NUMBER_MINOR 5 /* 0...n */ +#define MODULE_MAGIC_NUMBER_MINOR 6 /* 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 8664983119..015aec97b8 100644 --- a/include/scoreboard.h +++ b/include/scoreboard.h @@ -177,6 +177,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 85018aba09..c7980435ed 100644 --- a/server/mpm/event/event.c +++ b/server/mpm/event/event.c @@ -230,6 +230,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) @@ -720,14 +722,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); } @@ -865,7 +867,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; } @@ -931,17 +933,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; @@ -993,7 +992,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; @@ -1032,7 +1032,7 @@ read_request: if (cs->pub.state == CONN_STATE_WRITE_COMPLETION) { int not_complete_yet; - ap_update_child_status(sbh, SERVER_BUSY_WRITE, NULL); + ap_update_child_status(cs->sbh, SERVER_BUSY_WRITE, NULL); not_complete_yet = ap_run_output_pending(c); @@ -1083,7 +1083,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 a30c7a4529..4eb1b05890 100644 --- a/server/scoreboard.c +++ b/server/scoreboard.c @@ -422,12 +422,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) -- 2.40.0