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

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)