merge r1741564,r1741446 from trunk

mod_http2: fixes connection shutdown and updates log tags

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1741567 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/modules/http2/h2_bucket_beam.c b/modules/http2/h2_bucket_beam.c
index 6d41fadc5bbf6356402677bec145d7f676a973fd..2e9f4b1f39098162dc214102af0ecd78e0af8ad0 100644 (file)
--- a/modules/http2/h2_bucket_beam.c
+++ b/modules/http2/h2_bucket_beam.c
@@ -335,7 +335,7 @@ static void h2_beam_emitted(h2_bucket_beam *beam, h2_beam_proxy *proxy)
             else {
                 /* it should be there unless we screwed up */
                 ap_log_perror(APLOG_MARK, APLOG_WARNING, 0, beam->red_pool, 
-                              APLOGNO() "h2_beam(%d-%s): emitted bucket not "
+                              APLOGNO(03384) "h2_beam(%d-%s): emitted bucket not "
                               "in hold, n=%d", beam->id, beam->tag, 
                               (int)proxy->n);
                 AP_DEBUG_ASSERT(!proxy->bred);

diff --git a/modules/http2/h2_mplx.c b/modules/http2/h2_mplx.c
index e8d222eeafabe8ebece132371aa9d0a4d498f857..5fd23ae0f8efb4e489d015b1e512dcaecc35ff5f 100644 (file)
--- a/modules/http2/h2_mplx.c
+++ b/modules/http2/h2_mplx.c
@@ -319,7 +319,7 @@ static void task_destroy(h2_mplx *m, h2_task *task, int events)
     /* cleanup any buffered input */
     status = h2_task_shutdown(task, 0);
     if (status != APR_SUCCESS){
-        ap_log_cerror(APLOG_MARK, APLOG_WARNING, status, m->c, APLOGNO() 
+        ap_log_cerror(APLOG_MARK, APLOG_WARNING, status, m->c, APLOGNO(03385) 
                       "h2_task(%s): shutdown", task->id);
     }
     
@@ -364,43 +364,52 @@ static void task_destroy(h2_mplx *m, h2_task *task, int events)
     check_tx_free(m);
 }
 
-static int task_stream_done(h2_mplx *m, h2_task *task, int rst_error) 
+static void stream_done(h2_mplx *m, h2_stream *stream, int rst_error) 
 {
-    /* Remove io from ready set, we will never submit it */
-    h2_ihash_remove(m->ready_tasks, task->stream_id);
-    if (task->worker_done) {
-        /* already finished or not even started yet */
-        h2_iq_remove(m->q, task->stream_id);
-        task_destroy(m, task, 0);
-        return 0;
+    h2_task *task;
+    
+    h2_ihash_remove(m->streams, stream->id);
+    if (stream->input) {
+        apr_status_t status;
+        status = h2_beam_shutdown(stream->input, APR_NONBLOCK_READ);
+        if (status == APR_EAGAIN) {
+            ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, m->c, 
+                          "h2_stream(%ld-%d): wait on input shutdown", 
+                          m->id, stream->id);
+            status = h2_beam_shutdown(stream->input, APR_BLOCK_READ);
+            ap_log_cerror(APLOG_MARK, APLOG_TRACE2, status, m->c, 
+                          "h2_stream(%ld-%d): input shutdown returned", 
+                          m->id, stream->id);
+        }
     }
-    else {
-        /* cleanup once task is done */
-        task->orphaned = 1;
-        if (task->input.beam) {
-            apr_status_t status;
-            status = h2_beam_shutdown(task->input.beam, APR_NONBLOCK_READ);
-            if (status == APR_EAGAIN) {
-                ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, m->c, 
-                              "h2_stream(%ld-%d): wait on input shutdown", 
-                              m->id, task->stream_id);
-                status = h2_beam_shutdown(task->input.beam, APR_BLOCK_READ);
-                ap_log_cerror(APLOG_MARK, APLOG_TRACE2, status, m->c, 
-                              "h2_stream(%ld-%d): input shutdown returned", 
-                              m->id, task->stream_id);
-            }
-            task->input.beam = NULL;
+
+    task = h2_ihash_get(m->tasks, stream->id);
+    if (task) {
+        /* Remove task from ready set, we will never submit it */
+        h2_ihash_remove(m->ready_tasks, stream->id);
+        
+        if (task->worker_done) {
+            /* already finished or not even started yet */
+            h2_iq_remove(m->q, task->stream_id);
+            task_destroy(m, task, 0);
         }
-        if (rst_error) {
-            h2_task_rst(task, rst_error);
+        else {
+            /* task still running, cleanup once it is done */
+            task->orphaned = 1;
+            task->input.beam = NULL; 
+            if (rst_error) {
+                h2_task_rst(task, rst_error);
+            }
         }
-        return 1;
     }
 }
 
 static int stream_done_iter(void *ctx, void *val)
 {
-    return task_stream_done((h2_mplx*)ctx, val, 0);
+    h2_stream *stream = val;
+    stream_done((h2_mplx*)ctx, val, 0);
+    h2_stream_destroy(stream);
+    return 0;
 }
 
 static int task_print(void *ctx, void *val)
@@ -444,9 +453,10 @@ apr_status_t h2_mplx_release_and_join(h2_mplx *m, apr_thread_cond_t *wait)
         
         h2_iq_clear(m->q);
         apr_thread_cond_broadcast(m->task_thawed);
-        while (!h2_ihash_iter(m->tasks, stream_done_iter, m)) {
-            /* iterate until all ios have been orphaned or destroyed */
+        while (!h2_ihash_iter(m->streams, stream_done_iter, m)) {
+            /* iterate until all streams have been removed */
         }
+        AP_DEBUG_ASSERT(h2_ihash_empty(m->streams));
     
         /* If we still have busy workers, we cannot release our memory
          * pool yet, as slave connections have child pools of their respective
@@ -463,9 +473,6 @@ apr_status_t h2_mplx_release_and_join(h2_mplx *m, apr_thread_cond_t *wait)
                           
             status = apr_thread_cond_timedwait(wait, m->lock, apr_time_from_sec(wait_secs));
             
-            while (!h2_ihash_iter(m->tasks, stream_done_iter, m)) {
-                /* iterate until all ios have been orphaned or destroyed */
-            }
             if (APR_STATUS_IS_TIMEUP(status)) {
                 if (i > 0) {
                     /* Oh, oh. Still we wait for assigned  workers to report that 
@@ -520,17 +527,12 @@ apr_status_t h2_mplx_stream_done(h2_mplx *m, int stream_id, int rst_error)
      */
     AP_DEBUG_ASSERT(m);
     if ((status = enter_mutex(m, &acquired)) == APR_SUCCESS) {
-        h2_task *task = h2_ihash_get(m->tasks, stream_id);
-
-        h2_ihash_remove(m->streams, stream_id);
-        /* there should be an h2_io, once the stream has been scheduled
-         * for processing, e.g. when we received all HEADERs. But when
-         * a stream is cancelled very early, it will not exist. */
-        if (task) {
+        h2_stream *stream = h2_ihash_get(m->streams, stream_id);
+        if (stream) {
             ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, m->c, 
-                          "h2_mplx(%ld-%d): marking stream task as done.", 
+                          "h2_mplx(%ld-%d): marking stream as done.", 
                           m->id, stream_id);
-            task_stream_done(m, task, rst_error);
+            stream_done(m, stream, rst_error);
         }
         leave_mutex(m, acquired);
     }
@@ -929,7 +931,7 @@ static void task_done(h2_mplx *m, h2_task *task, h2_req_engine *ngn)
                 h2_ngn_shed_done_ngn(m->ngn_shed, task->engine);
             }
             
-            if (!task->orphaned && m->redo_tasks
+            if (!m->aborted && !task->orphaned && m->redo_tasks
                 && h2_ihash_get(m->redo_tasks, task->stream_id)) {
                 /* reset and schedule again */
                 h2_task_redo(task);

diff --git a/modules/http2/h2_session.c b/modules/http2/h2_session.c
index a760101768304efe339bc84880b742641fa82b8d..f96c50960425dc6de8a806834a25085983e6eec1 100644 (file)
--- a/modules/http2/h2_session.c
+++ b/modules/http2/h2_session.c
@@ -708,12 +708,13 @@ static void h2_session_cleanup(h2_session *session)
 static void h2_session_destroy(h2_session *session)
 {
     AP_DEBUG_ASSERT(session);
+    
     h2_session_cleanup(session);
-
+    h2_ihash_clear(session->streams);
+    
     if (APLOGctrace1(session->c)) {
         ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, session->c,
-                      "h2_session(%ld): destroy, %d streams open",
-                      session->id, (int)h2_ihash_count(session->streams));
+                      "h2_session(%ld): destroy", session->id);
     }
     if (session->mplx) {
         h2_mplx_set_consumed_cb(session->mplx, NULL, NULL);