On Thu, Dec 21, 2017 at 7:04 PM, Yann Ylavic <ylavic....@gmail.com> wrote:
> On Thu, Dec 21, 2017 at 6:59 PM, Yann Ylavic <ylavic....@gmail.com> wrote:
>> On Thu, Dec 21, 2017 at 6:49 PM, Stefan Eissing <ste...@eissing.org> wrote:
>>> Oops, thought it was OK. That causes the hangers?
>>
>> Not anymore :)
>>
>> But h2_conn_run() seems to possibly return CONN_STATE_HANDLER, it
>> looks stange to me, and for once causes hanger ;)
>> What's the meaning of it?
>
> That's for when it's called from h2_protocol_switch() possibly, the
> issue is that it's also called from h2_h2_process_conn(), a
> process_connection hook which returns straight to the MPM...

Actually I don't see any other possible state than CONN_STATE_LINGER
after h2_conn_run(), the master connection has to be terminated in
both h2_h2_process_conn() hook and
core_upgrade_handler::ap_switch_protocol() cases.

So how about the attached patch?
It changes h2_conn_run() to finally/unconditionally set
CONN_STATE_LINGER after having flushed and aborted the connection
(aborting prevents further unnecessary actions like
ap_check_pipeline() or lingering before closing the socket, once the
connection is flushed...).
This patch does also s/return DONE/return OK/ where appropriate (the
DONE in core_upgrade_handler() is fine because we want to stop request
processing from there).
Index: modules/http2/h2_conn.c
===================================================================
--- modules/http2/h2_conn.c	(revision 1818982)
+++ modules/http2/h2_conn.c	(working copy)
@@ -253,25 +253,14 @@ apr_status_t h2_conn_run(struct h2_ctx *ctx, conn_
     } while (!async_mpm
              && c->keepalive == AP_CONN_KEEPALIVE 
              && mpm_state != AP_MPMQ_STOPPING);
-    
+
+    ap_flush_conn(c);
+    c->aborted = 1;
     if (c->cs) {
-        switch (session->state) {
-            case H2_SESSION_ST_INIT:
-            case H2_SESSION_ST_CLEANUP:
-            case H2_SESSION_ST_DONE:
-            case H2_SESSION_ST_IDLE:
-                c->cs->state = CONN_STATE_WRITE_COMPLETION;
-                break;
-            case H2_SESSION_ST_BUSY:
-            case H2_SESSION_ST_WAIT:
-            default:
-                c->cs->state = CONN_STATE_HANDLER;
-                break;
-                
-        }
+        c->cs->state = CONN_STATE_LINGER;
     }
-    
-    return DONE;
+
+    return APR_SUCCESS;
 }
 
 apr_status_t h2_conn_pre_close(struct h2_ctx *ctx, conn_rec *c)
Index: modules/http2/h2_h2.c
===================================================================
--- modules/http2/h2_h2.c	(revision 1818982)
+++ modules/http2/h2_h2.c	(working copy)
@@ -669,10 +669,11 @@ int h2_h2_process_conn(conn_rec* c)
             ap_log_cerror(APLOG_MARK, APLOG_TRACE1, status, c, "conn_setup");
             if (status != APR_SUCCESS) {
                 h2_ctx_clear(c);
-                return status;
+                return !OK;
             }
         }
-        return h2_conn_run(ctx, c);
+        h2_conn_run(ctx, c);
+        return OK;
     }
     
     ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c, "h2_h2, declined");
Index: modules/http2/h2_switch.c
===================================================================
--- modules/http2/h2_switch.c	(revision 1818982)
+++ modules/http2/h2_switch.c	(working copy)
@@ -185,13 +185,12 @@ static int h2_protocol_switch(conn_rec *c, request
                 ap_log_rerror(APLOG_MARK, APLOG_DEBUG, status, r, APLOGNO(03088)
                               "session setup");
                 h2_ctx_clear(c);
-                return status;
+                return !OK;
             }
             
             h2_conn_run(ctx, c);
-            return DONE;
         }
-        return DONE;
+        return OK;
     }
     
     return DECLINED;

Reply via email to