You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by ic...@apache.org on 2015/12/30 17:12:36 UTC

svn commit: r1722369 - in /httpd/httpd/trunk: ./ docs/manual/mod/ modules/http2/

Author: icing
Date: Wed Dec 30 16:12:35 2015
New Revision: 1722369

URL: http://svn.apache.org/viewvc?rev=1722369&view=rev
Log:
fixes after fuzzing tests, changed H2KeepAliveTimeout default

Modified:
    httpd/httpd/trunk/CHANGES
    httpd/httpd/trunk/docs/manual/mod/mod_http2.xml
    httpd/httpd/trunk/modules/http2/h2_config.c
    httpd/httpd/trunk/modules/http2/h2_conn.c
    httpd/httpd/trunk/modules/http2/h2_conn_io.c
    httpd/httpd/trunk/modules/http2/h2_conn_io.h
    httpd/httpd/trunk/modules/http2/h2_mplx.c
    httpd/httpd/trunk/modules/http2/h2_session.c
    httpd/httpd/trunk/modules/http2/h2_session.h
    httpd/httpd/trunk/modules/http2/h2_switch.c
    httpd/httpd/trunk/modules/http2/h2_version.h
    httpd/httpd/trunk/modules/http2/h2_workers.c

Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1722369&r1=1722368&r2=1722369&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Wed Dec 30 16:12:35 2015
@@ -1,6 +1,11 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.5.0
 
+  *) mod_http2: Fixed several errors when connections are closed in the middle
+     of requests, changed H2KeepAliveTimeout defaults to be the same as H2Timeout
+     for synchronous MPMs and leave keepalive timeout to async MPMs default.
+     [Stefan Eissing]
+
   *) mod_cache_socache: Fix a possible cached entity body corruption when it
      is received from an origin server in multiple batches and forwarded by
      mod_proxy.  [Yann Ylavic]

Modified: httpd/httpd/trunk/docs/manual/mod/mod_http2.xml
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/manual/mod/mod_http2.xml?rev=1722369&r1=1722368&r2=1722369&view=diff
==============================================================================
--- httpd/httpd/trunk/docs/manual/mod/mod_http2.xml (original)
+++ httpd/httpd/trunk/docs/manual/mod/mod_http2.xml Wed Dec 30 16:12:35 2015
@@ -761,7 +761,6 @@ H2PushPriority text/css   interleaved
         <name>H2KeepAliveTimeout</name>
         <description>Timeout (in seconds) for idle HTTP/2 connections</description>
         <syntax>H2KeepAliveTimeout seconds</syntax>
-        <default>H2KeepAliveTimeout 300</default>
         <contextlist>
             <context>server config</context>
             <context>virtual host</context>
@@ -781,7 +780,9 @@ H2PushPriority text/css   interleaved
                 idle when no streams are open, e.g. no requests are ongoing.
             </p>
             <p>
-                A value of 0 enforces no timeout.
+                By default, for non-async MPMs (prefork, worker) the keepalive timeout
+                will be the same as H2Timeout. For async MPMs, the keepalive handling for
+                HTTP/1 connections applies as no special action is taken.
             </p>
         </usage>
     </directivesynopsis>
@@ -790,7 +791,7 @@ H2PushPriority text/css   interleaved
         <name>H2StreamTimeout</name>
         <description>Timeout (in seconds) for idle HTTP/2 connections</description>
         <syntax>H2StreamTimeout seconds</syntax>
-        <default>H2StreamTimeout 120</default>
+        <default>H2StreamTimeout 0</default>
         <contextlist>
             <context>server config</context>
             <context>virtual host</context>

Modified: httpd/httpd/trunk/modules/http2/h2_config.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_config.c?rev=1722369&r1=1722368&r2=1722369&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_config.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_config.c Wed Dec 30 16:12:35 2015
@@ -60,7 +60,7 @@ static h2_config defconf = {
     1,                      /* HTTP/2 server push enabled */
     NULL,                   /* map of content-type to priorities */
     5,                      /* normal connection timeout */
-    5,                      /* keepalive timeout */
+    -1,                     /* keepalive timeout */
     0,                      /* stream timeout */
 };
 

Modified: httpd/httpd/trunk/modules/http2/h2_conn.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_conn.c?rev=1722369&r1=1722368&r2=1722369&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_conn.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_conn.c Wed Dec 30 16:12:35 2015
@@ -158,47 +158,71 @@ apr_status_t h2_conn_setup(h2_ctx *ctx,
 static apr_status_t h2_conn_process(h2_ctx *ctx)
 {
     h2_session *session;
+    apr_status_t status;
     
     session = h2_ctx_session_get(ctx);
     if (session->c->cs) {
         session->c->cs->sense = CONN_SENSE_DEFAULT;
     }
 
-    h2_session_process(session, async_mpm);
+    status = h2_session_process(session, async_mpm);
 
     session->c->keepalive = AP_CONN_KEEPALIVE;
     if (session->c->cs) {
         session->c->cs->state = CONN_STATE_WRITE_COMPLETION;
     }
     
+    if (APR_STATUS_IS_EOF(status)
+        || APR_STATUS_IS_ECONNRESET(status) 
+        || APR_STATUS_IS_ECONNABORTED(status)) {
+        /* fatal, probably client just closed connection. emergency shutdown */
+        /* Make sure this connection gets closed properly. */
+        ap_log_cerror( APLOG_MARK, APLOG_DEBUG, 0, session->c,
+                      "h2_session(%ld): aborted", session->id);
+        session->c->keepalive = AP_CONN_CLOSE;
+        
+        h2_ctx_clear(session->c);
+        h2_session_abort(session, status);
+        h2_session_eoc_callback(session);
+        /* hereafter session might be gone */
+        return APR_ECONNABORTED;
+    }
+    
     if (session->state == H2_SESSION_ST_CLOSING) {
         ap_log_cerror( APLOG_MARK, APLOG_DEBUG, 0, session->c,
-                      "h2_session(%ld): done", session->id);
+                      "h2_session(%ld): closing", session->id);
         /* Make sure this connection gets closed properly. */
-        ap_update_child_status_from_conn(session->c->sbh, SERVER_CLOSING, session->c);
         session->c->keepalive = AP_CONN_CLOSE;
         
+        h2_ctx_clear(session->c);
         h2_session_close(session);
         /* hereafter session may be gone */
     }
+    else if (session->state == H2_SESSION_ST_ABORTED) {
+        ap_log_cerror( APLOG_MARK, APLOG_DEBUG, 0, session->c,
+                      "h2_session(%ld): already aborted", session->id);
+        return APR_ECONNABORTED;
+    }
     
-    return DONE;
+    return APR_SUCCESS;
 }
 
 apr_status_t h2_conn_run(struct h2_ctx *ctx, conn_rec *c)
 {
     int mpm_state = 0;
+    apr_status_t status;
     do {
-        h2_conn_process(ctx);
+        status = h2_conn_process(ctx);
         
         if (ap_mpm_query(AP_MPMQ_MPM_STATE, &mpm_state)) {
             break;
         }
-    } while (!async_mpm 
+    } while (!async_mpm
+             && status == APR_SUCCESS
              && c->keepalive == AP_CONN_KEEPALIVE 
              && mpm_state != AP_MPMQ_STOPPING);
     
-    return DONE;
+    return status;
 }
 
 

Modified: httpd/httpd/trunk/modules/http2/h2_conn_io.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_conn_io.c?rev=1722369&r1=1722368&r2=1722369&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_conn_io.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_conn_io.c Wed Dec 30 16:12:35 2015
@@ -93,9 +93,15 @@ int h2_conn_io_is_buffered(h2_conn_io *i
     return io->bufsize > 0;
 }
 
+typedef struct {
+    conn_rec *c;
+    h2_conn_io *io;
+} pass_out_ctx;
+
 static apr_status_t pass_out(apr_bucket_brigade *bb, void *ctx) 
 {
-    h2_conn_io *io = (h2_conn_io*)ctx;
+    pass_out_ctx *pctx = ctx;
+    conn_rec *c = pctx->c;
     apr_status_t status;
     apr_off_t bblen;
     
@@ -103,19 +109,19 @@ static apr_status_t pass_out(apr_bucket_
         return APR_SUCCESS;
     }
     
-    ap_update_child_status(io->connection->sbh, SERVER_BUSY_WRITE, NULL);
+    ap_update_child_status(c->sbh, SERVER_BUSY_WRITE, NULL);
     status = apr_brigade_length(bb, 0, &bblen);
     if (status == APR_SUCCESS) {
-        ap_log_cerror(APLOG_MARK, APLOG_TRACE4, 0, io->connection,
+        ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, c,
                       "h2_conn_io(%ld): pass_out brigade %ld bytes",
-                      io->connection->id, (long)bblen);
-        status = ap_pass_brigade(io->connection->output_filters, bb);
-        if (status == APR_SUCCESS) {
-            io->bytes_written += (apr_size_t)bblen;
-            io->last_write = apr_time_now();
+                      c->id, (long)bblen);
+        status = ap_pass_brigade(c->output_filters, bb);
+        if (status == APR_SUCCESS && pctx->io) {
+            pctx->io->bytes_written += (apr_size_t)bblen;
+            pctx->io->last_write = apr_time_now();
         }
-        apr_brigade_cleanup(bb);
     }
+    apr_brigade_cleanup(bb);
     return status;
 }
 
@@ -169,7 +175,10 @@ apr_status_t h2_conn_io_write(h2_conn_io
                               const char *buf, size_t length)
 {
     apr_status_t status = APR_SUCCESS;
+    pass_out_ctx ctx;
     
+    ctx.c = io->connection;
+    ctx.io = io;
     io->unflushed = 1;
     if (io->bufsize > 0) {
         ap_log_cerror(APLOG_MARK, APLOG_TRACE4, 0, io->connection,
@@ -183,8 +192,9 @@ apr_status_t h2_conn_io_write(h2_conn_io
         while (length > 0 && (status == APR_SUCCESS)) {
             apr_size_t avail = io->bufsize - io->buflen;
             if (avail <= 0) {
+                
                 bucketeer_buffer(io);
-                status = pass_out(io->output, io);
+                status = pass_out(io->output, &ctx);
                 io->buflen = 0;
             }
             else if (length > avail) {
@@ -205,7 +215,7 @@ apr_status_t h2_conn_io_write(h2_conn_io
     else {
         ap_log_cerror(APLOG_MARK, APLOG_TRACE4, status, io->connection,
                       "h2_conn_io: writing %ld bytes to brigade", (long)length);
-        status = apr_brigade_write(io->output, pass_out, io, buf, length);
+        status = apr_brigade_write(io->output, pass_out, &ctx, buf, length);
     }
     
     return status;
@@ -242,9 +252,11 @@ apr_status_t h2_conn_io_consider_flush(h
     return status;
 }
 
-static apr_status_t h2_conn_io_flush_int(h2_conn_io *io, int force)
+static apr_status_t h2_conn_io_flush_int(h2_conn_io *io, int force, int eoc)
 {
     if (io->unflushed || force) {
+        pass_out_ctx ctx;
+        
         if (io->buflen > 0) {
             /* something in the buffer, put it in the output brigade */
             ap_log_cerror(APLOG_MARK, APLOG_TRACE4, 0, io->connection,
@@ -262,20 +274,29 @@ static apr_status_t h2_conn_io_flush_int
                       "h2_conn_io: flush");
         /* Send it out */
         io->unflushed = 0;
-        return pass_out(io->output, io);
+        
+        ctx.c = io->connection;
+        ctx.io = eoc? NULL : io;
+        return pass_out(io->output, &ctx);
         /* no more access after this, as we might have flushed an EOC bucket
          * that de-allocated us all. */
     }
     return APR_SUCCESS;
 }
 
+apr_status_t h2_conn_io_write_eoc(h2_conn_io *io, apr_bucket *b)
+{
+    APR_BRIGADE_INSERT_TAIL(io->output, b);
+    return h2_conn_io_flush_int(io, 1, 1);
+}
+
 apr_status_t h2_conn_io_flush(h2_conn_io *io)
 {
-    return h2_conn_io_flush_int(io, 1);
+    return h2_conn_io_flush_int(io, 1, 0);
 }
 
 apr_status_t h2_conn_io_pass(h2_conn_io *io)
 {
-    return h2_conn_io_flush_int(io, 0);
+    return h2_conn_io_flush_int(io, 0, 0);
 }
 

Modified: httpd/httpd/trunk/modules/http2/h2_conn_io.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_conn_io.h?rev=1722369&r1=1722368&r2=1722369&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_conn_io.h (original)
+++ httpd/httpd/trunk/modules/http2/h2_conn_io.h Wed Dec 30 16:12:35 2015
@@ -60,5 +60,6 @@ apr_status_t h2_conn_io_consider_flush(h
 
 apr_status_t h2_conn_io_pass(h2_conn_io *io);
 apr_status_t h2_conn_io_flush(h2_conn_io *io);
+apr_status_t h2_conn_io_write_eoc(h2_conn_io *io, apr_bucket *b);
 
 #endif /* defined(__mod_h2__h2_conn_io__) */

Modified: httpd/httpd/trunk/modules/http2/h2_mplx.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_mplx.c?rev=1722369&r1=1722368&r2=1722369&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_mplx.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_mplx.c Wed Dec 30 16:12:35 2015
@@ -264,6 +264,8 @@ apr_status_t h2_mplx_release_and_join(h2
     workers_unregister(m);
     status = apr_thread_mutex_lock(m->lock);
     if (APR_SUCCESS == status) {
+        int i;
+        
         /* disable WINDOW_UPDATE callbacks */
         h2_mplx_set_consumed_cb(m, NULL, NULL);
         while (!h2_io_set_iter(m->stream_ios, stream_done_iter, m)) {
@@ -271,14 +273,21 @@ apr_status_t h2_mplx_release_and_join(h2
         }
     
         release(m, 0);
-        while (m->refs > 0) {
+        for (i = 0; m->refs > 0; ++i) {
+            
             m->join_wait = wait;
             ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, m->c,
                           "h2_mplx(%ld): release_join, refs=%d, waiting...", 
                           m->id, m->refs);
-            apr_thread_cond_wait(wait, m->lock);
+                          
+            status = apr_thread_cond_timedwait(wait, m->lock, apr_time_from_sec(2));
+            if (APR_STATUS_IS_TIMEUP(status)) {
+                ap_log_cerror(APLOG_MARK, APLOG_INFO, 0, m->c,
+                              "h2_mplx(%ld): release timeup %d, refs=%d, waiting...", 
+                              m->id, i, m->refs);
+            }
         }
-        ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, m->c,
+        ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, m->c,
                       "h2_mplx(%ld): release_join -> destroy, (#ios=%ld)", 
                       m->id, (long)h2_io_set_size(m->stream_ios));
         apr_thread_mutex_unlock(m->lock);

Modified: httpd/httpd/trunk/modules/http2/h2_session.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_session.c?rev=1722369&r1=1722368&r2=1722369&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_session.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_session.c Wed Dec 30 16:12:35 2015
@@ -788,6 +788,9 @@ static h2_session *h2_session_create_int
         session->max_stream_mem = h2_config_geti(session->config, H2_CONF_STREAM_MAX_MEM);
         session->timeout_secs = h2_config_geti(session->config, H2_CONF_TIMEOUT_SECS);
         session->keepalive_secs = h2_config_geti(session->config, H2_CONF_KEEPALIVE_SECS);
+        if (session->keepalive_secs <= 0) {
+            session->keepalive_secs = session->timeout_secs;
+        }
         
         status = apr_thread_cond_create(&session->iowait, session->pool);
         if (status != APR_SUCCESS) {
@@ -878,17 +881,17 @@ void h2_session_eoc_callback(h2_session
     h2_session_destroy(session);
 }
 
-static apr_status_t h2_session_abort_int(h2_session *session, int reason)
+static apr_status_t h2_session_shutdown(h2_session *session, int reason)
 {
     AP_DEBUG_ASSERT(session);
     session->aborted = 1;
-    if (session->state != H2_SESSION_ST_CLOSING) {
-        session->state = H2_SESSION_ST_CLOSING;
+    if (session->state != H2_SESSION_ST_CLOSING
+        && session->state != H2_SESSION_ST_ABORTED) {
         if (session->client_goaway) {
             /* client sent us a GOAWAY, just terminate */
             nghttp2_session_terminate_session(session->ngh2, NGHTTP2_ERR_EOF);
             ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c,
-                          "session(%ld): closed, GOAWAY from client", session->id);
+                          "session(%ld): shutdown, GOAWAY from client", session->id);
         }
         else if (!reason) {
             nghttp2_submit_goaway(session->ngh2, NGHTTP2_FLAG_NONE, 
@@ -896,7 +899,7 @@ static apr_status_t h2_session_abort_int
                                   reason, NULL, 0);
             nghttp2_session_send(session->ngh2);
             ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c,
-                          "session(%ld): closed, no err", session->id);
+                          "session(%ld): shutdown, no err", session->id);
         }
         else {
             const char *err = nghttp2_strerror(reason);
@@ -906,39 +909,22 @@ static apr_status_t h2_session_abort_int
                                   strlen(err));
             nghttp2_session_send(session->ngh2);
             ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c,
-                          "session(%ld): closed, err=%d '%s'",
+                          "session(%ld): shutdown, err=%d '%s'",
                           session->id, reason, err);
         }
+        session->state = H2_SESSION_ST_CLOSING;
         h2_mplx_abort(session->mplx);
     }
     return APR_SUCCESS;
 }
 
-apr_status_t h2_session_abort(h2_session *session, apr_status_t reason, int rv)
+void h2_session_abort(h2_session *session, apr_status_t status)
 {
     AP_DEBUG_ASSERT(session);
-    if (rv == 0) {
-        rv = NGHTTP2_ERR_PROTO;
-        switch (reason) {
-            case APR_ENOMEM:
-                rv = NGHTTP2_ERR_NOMEM;
-                break;
-            case APR_SUCCESS:                            /* all fine, just... */
-            case APR_EOF:                         /* client closed its end... */
-            case APR_TIMEUP:                          /* got bored waiting... */
-                rv = 0;                            /* ...gracefully shut down */
-                break;
-            case APR_EBADF:        /* connection unusable, terminate silently */
-            default:
-                if (APR_STATUS_IS_ECONNABORTED(reason)
-                    || APR_STATUS_IS_ECONNRESET(reason)
-                    || APR_STATUS_IS_EBADF(reason)) {
-                    rv = NGHTTP2_ERR_EOF;
-                }
-                break;
-        }
-    }
-    return h2_session_abort_int(session, rv);
+    ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, session->c,
+                  "h2_session(%ld): aborting", session->id);
+    session->state = H2_SESSION_ST_ABORTED;
+    session->aborted = 1;
 }
 
 static apr_status_t h2_session_start(h2_session *session, int *rv)
@@ -1104,16 +1090,23 @@ h2_stream *h2_session_get_stream(h2_sess
 void h2_session_close(h2_session *session)
 {
     apr_bucket *b;
+    conn_rec *c = session->c;
+    apr_status_t status;
     
     AP_DEBUG_ASSERT(session);
     if (!session->aborted) {
-        h2_session_abort_int(session, 0);
+        h2_session_shutdown(session, 0);
     }
     h2_session_cleanup(session);
 
-    b = h2_bucket_eoc_create(session->c->bucket_alloc, session);
-    h2_conn_io_writeb(&session->io, b);
-    h2_conn_io_flush(&session->io);
+    ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, c,
+                  "h2_session(%ld): writing eoc", c->id);
+    b = h2_bucket_eoc_create(c->bucket_alloc, session);
+    status = h2_conn_io_write_eoc(&session->io, b);
+    if (status != APR_SUCCESS) {
+        ap_log_cerror(APLOG_MARK, APLOG_ERR, status, c,
+                      "h2_session(%ld): flushed eoc bucket", c->id);
+    } 
     /* and all is or will be destroyed */
 }
 
@@ -1300,7 +1293,7 @@ static apr_status_t submit_response(h2_s
 
     if (nghttp2_is_fatal(rv)) {
         status = APR_EGENERAL;
-        h2_session_abort_int(session, rv);
+        h2_session_shutdown(session, rv);
         ap_log_cerror(APLOG_MARK, APLOG_ERR, status, session->c,
                       APLOGNO(02940) "submit_response: %s", 
                       nghttp2_strerror(rv));
@@ -1584,7 +1577,7 @@ static apr_status_t h2_session_send(h2_s
         if (nghttp2_is_fatal(rv)) {
             ap_log_cerror( APLOG_MARK, APLOG_DEBUG, 0, session->c,
                           "h2_session: send gave error=%s", nghttp2_strerror(rv));
-            h2_session_abort_int(session, rv);
+            h2_session_shutdown(session, rv);
             return APR_EGENERAL;
         }
     }
@@ -1608,7 +1601,7 @@ static apr_status_t h2_session_receive(v
                           "h2_session: nghttp2_session_mem_recv error=%d",
                           (int)n);
             if (nghttp2_is_fatal((int)n)) {
-                h2_session_abort_int(session, (int)n);
+                h2_session_shutdown(session, (int)n);
                 return APR_EGENERAL;
             }
         }
@@ -1672,7 +1665,6 @@ static apr_status_t h2_session_read(h2_s
                                       "h2_session(%ld): error reading, terminating",
                                       session->id);
                     }
-                    h2_session_abort(session, status, 0);
                     return status;
                 }
                 /* subsequent failure after success(es), return initial
@@ -1730,7 +1722,7 @@ apr_status_t h2_session_process(h2_sessi
 
         if (session->aborted) {
             reason = "aborted";
-            status = APR_EOF;
+            status = APR_ECONNABORTED;
             goto out;
         }
         
@@ -1747,8 +1739,6 @@ apr_status_t h2_session_process(h2_sessi
                               session->s->server_hostname,
                               c->local_addr->port);
                 if (status != APR_SUCCESS) {
-                    h2_session_abort(session, status, rv);
-                    h2_session_eoc_callback(session);
                     reason = "start failed";
                     goto out;
                 }
@@ -1911,10 +1901,10 @@ apr_status_t h2_session_process(h2_sessi
                  * extend that with the H2KeepAliveTimeout. This works different
                  * for async MPMs. */
                 remain_secs = session->keepalive_secs - session->timeout_secs;
-                if (remain_secs <= 0) {
-                    /* keepalive is smaller than normal timeout, close the session */
+                if (!async && remain_secs <= 0) {
+                    /* not async, keepalive is smaller than normal timeout, close the session */
                     reason = "keepalive expired";
-                    h2_session_abort_int(session, 0);
+                    h2_session_shutdown(session, 0);
                     goto out;
                 }
                 ap_update_child_status_from_conn(c->sbh, SERVER_BUSY_KEEPALIVE, c);
@@ -1940,7 +1930,7 @@ apr_status_t h2_session_process(h2_sessi
                 status = h2_session_read(session, 1, 1);
                 if (APR_STATUS_IS_TIMEUP(status)) {
                     reason = "keepalive expired";
-                    h2_session_abort_int(session, 0);
+                    h2_session_shutdown(session, 0);
                     goto out;
                 }
                 else if (status != APR_SUCCESS) {
@@ -1964,6 +1954,11 @@ apr_status_t h2_session_process(h2_sessi
                 reason = "closing";
                 goto out;
                 
+            case H2_SESSION_ST_ABORTED:
+                ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, c,
+                              "h2_session(%ld): processing ABORTED", session->id);
+                return APR_ECONNABORTED;
+                
             default:
                 ap_log_cerror(APLOG_MARK, APLOG_ERR, APR_EGENERAL, c,
                               "h2_session(%ld): state %d", session->id, session->state);

Modified: httpd/httpd/trunk/modules/http2/h2_session.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_session.h?rev=1722369&r1=1722368&r2=1722369&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_session.h (original)
+++ httpd/httpd/trunk/modules/http2/h2_session.h Wed Dec 30 16:12:35 2015
@@ -60,6 +60,7 @@ typedef enum {
     H2_SESSION_ST_BUSY_WAIT,        /* waiting for tasks reporting back */
     H2_SESSION_ST_KEEPALIVE,        /* nothing to write, normal timeout passed */
     H2_SESSION_ST_CLOSING,          /* shuting down */
+    H2_SESSION_ST_ABORTED,          /* client closed connection or sky fall */
 } h2_session_state;
 
 typedef struct h2_session {
@@ -153,13 +154,12 @@ apr_status_t h2_session_process(h2_sessi
 void h2_session_eoc_callback(h2_session *session);
 
 /**
- * Called when an error occured and the session needs to shut down.
- * @param session the session to shut down
- * @param reason  the apache status that caused the shutdown
- * @param rv      the nghttp2 reason for shutdown, set to 0 if you have none.
- *
+ * Called when a serious error occured and the session needs to terminate
+ * without further connection io.
+ * @param session the session to abort
+ * @param reason  the apache status that caused the abort
  */
-apr_status_t h2_session_abort(h2_session *session, apr_status_t reason, int rv);
+void h2_session_abort(h2_session *session, apr_status_t reason);
 
 /**
  * Close and deallocate the given session.

Modified: httpd/httpd/trunk/modules/http2/h2_switch.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_switch.c?rev=1722369&r1=1722368&r2=1722369&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_switch.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_switch.c Wed Dec 30 16:12:35 2015
@@ -163,7 +163,8 @@ static int h2_protocol_switch(conn_rec *
                 return status;
             }
             
-            return h2_conn_run(ctx, c);
+            h2_conn_run(ctx, c);
+            return DONE;
         }
         return DONE;
     }

Modified: httpd/httpd/trunk/modules/http2/h2_version.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_version.h?rev=1722369&r1=1722368&r2=1722369&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_version.h (original)
+++ httpd/httpd/trunk/modules/http2/h2_version.h Wed Dec 30 16:12:35 2015
@@ -20,7 +20,7 @@
  * @macro
  * Version number of the h2 module as c string
  */
-#define MOD_HTTP2_VERSION "1.0.14-DEVa"
+#define MOD_HTTP2_VERSION "1.0.15-DEVa"
 
 /**
  * @macro
@@ -28,7 +28,7 @@
  * release. This is a 24 bit number with 8 bits for major number, 8 bits
  * for minor and 8 bits for patch. Version 1.2.3 becomes 0x010203.
  */
-#define MOD_HTTP2_VERSION_NUM 0x01000e
+#define MOD_HTTP2_VERSION_NUM 0x01000f
 
 
 #endif /* mod_h2_h2_version_h */

Modified: httpd/httpd/trunk/modules/http2/h2_workers.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_workers.c?rev=1722369&r1=1722368&r2=1722369&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_workers.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_workers.c Wed Dec 30 16:12:35 2015
@@ -272,7 +272,7 @@ h2_workers *h2_workers_create(server_rec
         apr_atomic_set32(&workers->max_idle_secs, 10);
         
         apr_threadattr_create(&workers->thread_attr, workers->pool);
-        apr_threadattr_detach_set(workers->thread_attr, 0);
+        apr_threadattr_detach_set(workers->thread_attr, 1);
         if (ap_thread_stacksize != 0) {
             apr_threadattr_stacksize_set(workers->thread_attr,
                                          ap_thread_stacksize);



Re: svn commit: r1722369 - in /httpd/httpd/trunk: ./ docs/manual/mod/ modules/http2/

Posted by Stefan Eissing <st...@greenbytes.de>.
Done, thanks!

> Am 04.01.2016 um 16:59 schrieb Yann Ylavic <yl...@gmail.com>:
> 
> On Wed, Dec 30, 2015 at 9:17 PM, Stefan Eissing
> <st...@greenbytes.de> wrote:
>> That feeds into the Grand Solution to make h2 workers part of the MPM. Not sure how to approach that.
> 
> How about calling apr_thread_join() in cleanup_zombies?


Re: svn commit: r1722369 - in /httpd/httpd/trunk: ./ docs/manual/mod/ modules/http2/

Posted by Yann Ylavic <yl...@gmail.com>.
On Wed, Dec 30, 2015 at 9:17 PM, Stefan Eissing
<st...@greenbytes.de> wrote:
> That feeds into the Grand Solution to make h2 workers part of the MPM. Not sure how to approach that.

How about calling apr_thread_join() in cleanup_zombies?

Re: svn commit: r1722369 - in /httpd/httpd/trunk: ./ docs/manual/mod/ modules/http2/

Posted by Stefan Eissing <st...@greenbytes.de>.
That feeds into the Grand Solution to make h2 workers part of the MPM. Not sure how to approach that. 

The common way today is that master connections do not return from run_connection if they are busy handling requests. If that takes longer than H2Timeout to produce results, it may happen nevertheless, but that should then drag down the conn anyway, I suppose.

> Am 30.12.2015 um 19:42 schrieb Yann Ylavic <yl...@gmail.com>:
> 
>> On Wed, Dec 30, 2015 at 5:12 PM,  <ic...@apache.org> wrote:
>> Author: icing
>> Date: Wed Dec 30 16:12:35 2015
>> New Revision: 1722369
>> 
>> URL: http://svn.apache.org/viewvc?rev=1722369&view=rev
>> Log:
>> fixes after fuzzing tests, changed H2KeepAliveTimeout default
> []
>> 
>> Modified: httpd/httpd/trunk/modules/http2/h2_workers.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_workers.c?rev=1722369&r1=1722368&r2=1722369&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/modules/http2/h2_workers.c (original)
>> +++ httpd/httpd/trunk/modules/http2/h2_workers.c Wed Dec 30 16:12:35 2015
>> @@ -272,7 +272,7 @@ h2_workers *h2_workers_create(server_rec
>>         apr_atomic_set32(&workers->max_idle_secs, 10);
>> 
>>         apr_threadattr_create(&workers->thread_attr, workers->pool);
>> -        apr_threadattr_detach_set(workers->thread_attr, 0);
>> +        apr_threadattr_detach_set(workers->thread_attr, 1);
> 
> Hmm, aren't worker threads joined?
> How does it work with eg. graceful restarts where (active) workers
> need to be waited before the child exits?
> 
> Regards,
> Yann.

Re: svn commit: r1722369 - in /httpd/httpd/trunk: ./ docs/manual/mod/ modules/http2/

Posted by Yann Ylavic <yl...@gmail.com>.
On Wed, Dec 30, 2015 at 5:12 PM,  <ic...@apache.org> wrote:
> Author: icing
> Date: Wed Dec 30 16:12:35 2015
> New Revision: 1722369
>
> URL: http://svn.apache.org/viewvc?rev=1722369&view=rev
> Log:
> fixes after fuzzing tests, changed H2KeepAliveTimeout default
>
[]
>
> Modified: httpd/httpd/trunk/modules/http2/h2_workers.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_workers.c?rev=1722369&r1=1722368&r2=1722369&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/http2/h2_workers.c (original)
> +++ httpd/httpd/trunk/modules/http2/h2_workers.c Wed Dec 30 16:12:35 2015
> @@ -272,7 +272,7 @@ h2_workers *h2_workers_create(server_rec
>          apr_atomic_set32(&workers->max_idle_secs, 10);
>
>          apr_threadattr_create(&workers->thread_attr, workers->pool);
> -        apr_threadattr_detach_set(workers->thread_attr, 0);
> +        apr_threadattr_detach_set(workers->thread_attr, 1);

Hmm, aren't worker threads joined?
How does it work with eg. graceful restarts where (active) workers
need to be waited before the child exits?

Regards,
Yann.