Re: mod_http2 and Frequent wake-ups for mpm_event

[Yann Ylavic]

Hi Stefan,

On Mon, Jan 23, 2017 at 9:54 PM, Stefan Eissing
<stefan.eiss...@greenbytes.de> wrote:
>
> I am not aware of any special expectations now. Almost all is triggered by 
> (parent) pool cleanups and is therefore more deterministic than before. The 
> only explicit destroy is done on finished streams and slave connections no 
> longer used. When the master conn disappears, all is deallocated as the force 
> wills it.

I wonder if the attached patch makes sense.
If beam_{recv,send}_cleanup() are to be executed on (parent) pool
destroy, there will be before beam_cleanup() itelf (which also calls
beam_send_cleanup() explicitly), so it should avoid potential a double
cleanup in this case.

WDYT?

Index: modules/http2/h2_bucket_beam.c
===================================================================
--- modules/http2/h2_bucket_beam.c      (revision 1779961)
+++ modules/http2/h2_bucket_beam.c      (working copy)
@@ -500,16 +500,25 @@ static apr_status_t beam_cleanup(void *data)
     beam_close(beam);
     switch (beam->owner) {
         case H2_BEAM_OWNER_SEND:
-            status = beam_send_cleanup(beam);
-            beam->recv_buffer = NULL;
+            if (beam->send_pool) {
+                apr_pool_cleanup_kill(beam->send_pool, beam, 
beam_send_cleanup);
+                status = beam_send_cleanup(beam);
+            }
+            if (beam->recv_buffer) {
+                apr_brigade_destroy(beam->recv_buffer);
+                beam->recv_buffer = NULL;
+            }
             beam->recv_pool = NULL;
             break;
         case H2_BEAM_OWNER_RECV:
             if (beam->recv_buffer) {
                 apr_brigade_destroy(beam->recv_buffer);
+                beam->recv_buffer = NULL;
             }
-            beam->recv_buffer = NULL;
-            beam->recv_pool = NULL;
+            if (beam->recv_pool) {
+                apr_pool_cleanup_kill(beam->recv_pool, beam, 
beam_recv_cleanup);
+                status = beam_recv_cleanup(beam);
+            }
             if (!H2_BLIST_EMPTY(&beam->send_list)) {
                 ap_assert(beam->send_pool);
             }
@@ -520,8 +529,8 @@ static apr_status_t beam_cleanup(void *data)
                  * beam should have lost its mutex protection, meaning
                  * it is no longer used multi-threaded and we can safely
                  * purge all remaining sender buckets. */
-                apr_pool_cleanup_kill(beam->send_pool, beam, 
beam_send_cleanup);
                 ap_assert(!beam->m_enter);
+                beam_set_send_pool(beam, NULL); 
                 beam_send_cleanup(beam);
             }
             ap_assert(H2_BPROXY_LIST_EMPTY(&beam->proxies));