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 2495bf79002d80a7aa02fddb3710d34bc5361bee..23ab298032ffa9a67a266d6e7a74e1a4d61bdbcc 100644 (file)
--- 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 2764501833767e92a78737ad0d38ad3d6e733d33..48c19cb7fd57a8602971e7e1f54e1581275a172c 100644 (file)
--- 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 1378128943e4f2f2d6d8c5ef8d499a970b37a04e..57cf3df132cb6dbaef8d7f4ee6a789ce2a60b80a 100644 (file)
--- 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 d456456a2a9a73d82a0987aa6a91b09f6152ad46..3848279a5f93f7a241dc03e1f2f6a06122438583 100644 (file)
--- 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 d83a8492c30007cb885bd095efe9c3900771fcf4..4343eba4b04a9d38d30e5c287f594c3c546a62bf 100644 (file)
--- 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)