Re: svn commit: r1779528 - /httpd/httpd/trunk/modules/http2/h2_mplx.c

[Yann Ylavic]

On Thu, Jan 19, 2017 at 10:09 PM,  <ic...@apache.org> wrote:
> Author: icing
> Date: Thu Jan 19 21:09:18 2017
> New Revision: 1779528
>
> URL: http://svn.apache.org/viewvc?rev=1779528&view=rev
> Log:
> On the trunk:
>
> reverting change 1779525.

We really need to not destroy m->pool in its parent's cleanup.

How about something like the attached instead, worth (other-)Stefan a try no?
That's temporary, we probably could avoid that with a flag (re)set in
a cleanup of mplx->pool itself...

Index: modules/http2/h2_mplx.c
===================================================================
--- modules/http2/h2_mplx.c	(revision 1779529)
+++ modules/http2/h2_mplx.c	(working copy)
@@ -231,7 +231,7 @@ static void purge_streams(h2_mplx *m)
     }
 }
 
-static void h2_mplx_destroy(h2_mplx *m)
+void h2_mplx_destroy(h2_mplx *m)
 {
     conn_rec **pslave;
     ap_assert(m);
@@ -633,7 +633,6 @@ apr_status_t h2_mplx_release_and_join(h2_mplx *m,
         /* 8. close the h2_req_enginge shed and self destruct */
         h2_ngn_shed_destroy(m->ngn_shed);
         m->ngn_shed = NULL;
-        h2_mplx_destroy(m);
     }
     return status;
 }
Index: modules/http2/h2_mplx.h
===================================================================
--- modules/http2/h2_mplx.h	(revision 1779529)
+++ modules/http2/h2_mplx.h	(working copy)
@@ -130,6 +130,11 @@ h2_mplx *h2_mplx_create(conn_rec *c, apr_pool_t *m
                         struct h2_workers *workers);
 
 /**
+ * Destroy the multiplexer.
+ */
+void h2_mplx_destroy(h2_mplx *m);
+
+/**
  * Decreases the reference counter of this mplx and waits for it
  * to reached 0, destroy the mplx afterwards.
  * This is to be called from the thread that created the mplx in
Index: modules/http2/h2_session.c
===================================================================
--- modules/http2/h2_session.c	(revision 1779529)
+++ modules/http2/h2_session.c	(working copy)
@@ -730,7 +730,7 @@ static apr_status_t init_callbacks(conn_rec *c, ng
     return APR_SUCCESS;
 }
 
-static void h2_session_cleanup(h2_session *session)
+static void h2_session_cleanup(h2_session *session, int pooled)
 {
     ap_assert(session);    
 
@@ -737,6 +737,9 @@ static apr_status_t init_callbacks(conn_rec *c, ng
     if (session->mplx) {
         h2_mplx_set_consumed_cb(session->mplx, NULL, NULL);
         h2_mplx_release_and_join(session->mplx, session->iowait);
+        if (pooled) {
+            h2_mplx_destroy(session->mplx);
+        }
         session->mplx = NULL;
     }
 
@@ -861,7 +864,7 @@ static apr_status_t session_pool_cleanup(void *dat
                       "goodbye, clients will be confused, should not happen", 
                       session->id);
     }
-    h2_session_cleanup(session);
+    h2_session_cleanup(session, 0);
     session->pool = NULL;
     return APR_SUCCESS;
 }
@@ -1049,7 +1052,7 @@ void h2_session_eoc_callback(h2_session *session)
         ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, session->c,
                       "session(%ld): cleanup and destroy", session->id);
         apr_pool_cleanup_kill(session->pool, session, session_pool_cleanup);
-        h2_session_cleanup(session);
+        h2_session_cleanup(session, 1);
         h2_session_destroy(session);
     }
 }