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 2016/08/27 10:39:25 UTC

svn commit: r1757985 - in /httpd/httpd/trunk: ./ modules/http2/

Author: icing
Date: Sat Aug 27 10:39:24 2016
New Revision: 1757985

URL: http://svn.apache.org/viewvc?rev=1757985&view=rev
Log:
mod_http2: fix for stream buffer handling during shutdown

Modified:
    httpd/httpd/trunk/CHANGES
    httpd/httpd/trunk/modules/http2/h2_bucket_beam.c
    httpd/httpd/trunk/modules/http2/h2_bucket_beam.h
    httpd/httpd/trunk/modules/http2/h2_mplx.c
    httpd/httpd/trunk/modules/http2/h2_session.c
    httpd/httpd/trunk/modules/http2/h2_stream.c
    httpd/httpd/trunk/modules/http2/h2_stream.h
    httpd/httpd/trunk/modules/http2/h2_task.c
    httpd/httpd/trunk/modules/http2/h2_task.h
    httpd/httpd/trunk/modules/http2/h2_version.h

Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1757985&r1=1757984&r2=1757985&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Sat Aug 27 10:39:24 2016
@@ -1,6 +1,9 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.5.0
 
+  *) mod_http2: fixed handling of stream buffers during shutdown.
+     [Stefan Eissing]
+     
   *) mod_proxy_hcheck: Set health check URI and expression correctly for health
      check worker. PR 60038 [zdeno <zd...@scnet.sk>]
 

Modified: httpd/httpd/trunk/modules/http2/h2_bucket_beam.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_bucket_beam.c?rev=1757985&r1=1757984&r2=1757985&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_bucket_beam.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_bucket_beam.c Sat Aug 27 10:39:24 2016
@@ -550,6 +550,11 @@ apr_status_t h2_beam_shutdown(h2_bucket_
         if (clear_buffers) {
             r_purge_reds(beam);
             h2_blist_cleanup(&beam->red);
+            if (!bl.mutex && beam->green) {
+                /* not protected, may process green in red call */
+                apr_brigade_destroy(beam->green);
+                beam->green = NULL;
+            }
         }
         beam_close(beam);
         
@@ -984,6 +989,18 @@ int h2_beam_empty(h2_bucket_beam *beam)
     return empty;
 }
 
+int h2_beam_holds_proxies(h2_bucket_beam *beam)
+{
+    int has_proxies = 1;
+    h2_beam_lock bl;
+    
+    if (enter_yellow(beam, &bl) == APR_SUCCESS) {
+        has_proxies = !H2_BPROXY_LIST_EMPTY(&beam->proxies);
+        leave_yellow(beam, &bl);
+    }
+    return has_proxies;
+}
+
 int h2_beam_closed(h2_bucket_beam *beam)
 {
     return beam->closed;

Modified: httpd/httpd/trunk/modules/http2/h2_bucket_beam.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_bucket_beam.h?rev=1757985&r1=1757984&r2=1757985&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_bucket_beam.h (original)
+++ httpd/httpd/trunk/modules/http2/h2_bucket_beam.h Sat Aug 27 10:39:24 2016
@@ -273,6 +273,13 @@ int h2_beam_closed(h2_bucket_beam *beam)
 int h2_beam_empty(h2_bucket_beam *beam);
 
 /**
+ * Determine if beam has handed out proxy buckets that are not destroyed. 
+ * 
+ * Call from red or green side.
+ */
+int h2_beam_holds_proxies(h2_bucket_beam *beam);
+
+/**
  * Abort the beam. Will cleanup any buffered buckets and answer all send
  * and receives with APR_ECONNABORTED.
  * 

Modified: httpd/httpd/trunk/modules/http2/h2_mplx.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_mplx.c?rev=1757985&r1=1757984&r2=1757985&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_mplx.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_mplx.c Sat Aug 27 10:39:24 2016
@@ -197,15 +197,19 @@ static int purge_stream(void *ctx, void
     h2_mplx *m = ctx;
     h2_stream *stream = val;
     int stream_id = stream->id;
-    h2_task *task = h2_ihash_get(m->tasks, stream_id);
+    h2_task *task;
+
+    /* stream_cleanup clears all buffers and destroys any buckets
+     * that might hold references into task space. Needs to be done
+     * before task destruction, otherwise it will complain. */
+    h2_stream_cleanup(stream);
     
-    h2_ihash_remove(m->spurge, stream_id);
-    h2_stream_destroy(stream);
+    task = h2_ihash_get(m->tasks, stream_id);    
     if (task) {
         task_destroy(m, task, 1);
     }
-    /* FIXME: task_destroy() might in some twisted way place the
-     * stream in the spurge hash again. Remove it last. */
+    
+    h2_stream_destroy(stream);
     h2_ihash_remove(m->spurge, stream_id);
     return 0;
 }
@@ -373,7 +377,10 @@ static void task_destroy(h2_mplx *m, h2_
         if (status != APR_SUCCESS){
             ap_log_cerror(APLOG_MARK, APLOG_WARNING, status, m->c, 
                           APLOGNO(03385) "h2_task(%s): output shutdown "
-                          "incomplete", task->id);
+                          "incomplete, beam empty=%d, holds proxies=%d", 
+                          task->id,
+                          h2_beam_empty(task->output.beam),
+                          h2_beam_holds_proxies(task->output.beam));
         }
     }
     

Modified: httpd/httpd/trunk/modules/http2/h2_session.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_session.c?rev=1757985&r1=1757984&r2=1757985&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_session.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_session.c Sat Aug 27 10:39:24 2016
@@ -604,6 +604,7 @@ static int on_send_data_cb(nghttp2_sessi
         ap_log_cerror(APLOG_MARK, APLOG_TRACE1, status, session->c,
                       "h2_stream(%ld-%d): send_data_cb, reading stream",
                       session->id, (int)stream_id);
+        apr_brigade_cleanup(session->bbtmp);
         return NGHTTP2_ERR_CALLBACK_FAILURE;
     }
     else if (len != length) {
@@ -611,6 +612,7 @@ static int on_send_data_cb(nghttp2_sessi
                       "h2_stream(%ld-%d): send_data_cb, wanted %ld bytes, "
                       "got %ld from stream",
                       session->id, (int)stream_id, (long)length, (long)len);
+        apr_brigade_cleanup(session->bbtmp);
         return NGHTTP2_ERR_CALLBACK_FAILURE;
     }
     
@@ -621,8 +623,8 @@ static int on_send_data_cb(nghttp2_sessi
     }
     
     status = h2_conn_io_pass(&session->io, session->bbtmp);
-        
     apr_brigade_cleanup(session->bbtmp);
+    
     if (status == APR_SUCCESS) {
         stream->out_data_frames++;
         stream->out_data_octets += length;
@@ -775,6 +777,7 @@ static apr_status_t h2_session_shutdown(
     dispatch_event(session, H2_SESSION_EV_LOCAL_GOAWAY, error, msg);
     
     if (force_close) {
+        apr_brigade_cleanup(session->bbtmp);
         h2_mplx_abort(session->mplx);
     }
     
@@ -1987,6 +1990,7 @@ static void dispatch_event(h2_session *s
     }
     
     if (session->state == H2_SESSION_ST_DONE) {
+        apr_brigade_cleanup(session->bbtmp);
         h2_mplx_abort(session->mplx);
     }
 }

Modified: httpd/httpd/trunk/modules/http2/h2_stream.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_stream.c?rev=1757985&r1=1757984&r2=1757985&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_stream.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_stream.c Sat Aug 27 10:39:24 2016
@@ -429,6 +429,7 @@ apr_status_t h2_stream_write_data(h2_str
 {
     conn_rec *c = stream->session->c;
     apr_status_t status = APR_SUCCESS;
+    apr_bucket_brigade *tmp;
     
     AP_DEBUG_ASSERT(stream);
     if (!stream->input) {
@@ -461,18 +462,15 @@ apr_status_t h2_stream_write_data(h2_str
         }
     }
     
-    if (!stream->tmp) {
-        stream->tmp = apr_brigade_create(stream->pool, c->bucket_alloc);
-    }
-    apr_brigade_write(stream->tmp, NULL, NULL, data, len);
+    tmp = apr_brigade_create(stream->pool, c->bucket_alloc);
+    apr_brigade_write(tmp, NULL, NULL, data, len);
     if (eos) {
-        APR_BRIGADE_INSERT_TAIL(stream->tmp, 
-                                apr_bucket_eos_create(c->bucket_alloc)); 
+        APR_BRIGADE_INSERT_TAIL(tmp, apr_bucket_eos_create(c->bucket_alloc)); 
         close_input(stream);
     }
+    status = h2_beam_send(stream->input, tmp, APR_BLOCK_READ);
+    apr_brigade_destroy(tmp);
     
-    status = h2_beam_send(stream->input, stream->tmp, APR_BLOCK_READ);
-    apr_brigade_cleanup(stream->tmp);
     stream->in_data_frames++;
     stream->in_data_octets += len;
     

Modified: httpd/httpd/trunk/modules/http2/h2_stream.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_stream.h?rev=1757985&r1=1757984&r2=1757985&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_stream.h (original)
+++ httpd/httpd/trunk/modules/http2/h2_stream.h Sat Aug 27 10:39:24 2016
@@ -56,7 +56,6 @@ struct h2_stream {
     struct h2_response *last_sent;
     struct h2_bucket_beam *output;
     apr_bucket_brigade *buffer;
-    apr_bucket_brigade *tmp;
     apr_array_header_t *files;  /* apr_file_t* we collected during I/O */
 
     int rst_error;              /* stream error for RST_STREAM */

Modified: httpd/httpd/trunk/modules/http2/h2_task.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_task.c?rev=1757985&r1=1757984&r2=1757985&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_task.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_task.c Sat Aug 27 10:39:24 2016
@@ -92,7 +92,7 @@ static apr_status_t input_handle_eos(h2_
     apr_table_t *t = task->request? task->request->trailers : NULL;
 
     if (task->input.chunked) {
-        task->input.tmp = apr_brigade_split_ex(bb, b, task->input.tmp);
+        apr_bucket_brigade *tmp = apr_brigade_split_ex(bb, b, NULL);
         if (t && !apr_is_empty_table(t)) {
             status = apr_brigade_puts(bb, NULL, NULL, "0\r\n");
             apr_table_do(input_ser_header, task, t, NULL);
@@ -101,7 +101,8 @@ static apr_status_t input_handle_eos(h2_
         else {
             status = apr_brigade_puts(bb, NULL, NULL, "0\r\n\r\n");
         }
-        APR_BRIGADE_CONCAT(bb, task->input.tmp);
+        APR_BRIGADE_CONCAT(bb, tmp);
+        apr_brigade_destroy(tmp);
     }
     else if (r && t && !apr_is_empty_table(t)){
         /* trailers passed in directly. */

Modified: httpd/httpd/trunk/modules/http2/h2_task.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_task.h?rev=1757985&r1=1757984&r2=1757985&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_task.h (original)
+++ httpd/httpd/trunk/modules/http2/h2_task.h Sat Aug 27 10:39:24 2016
@@ -61,7 +61,6 @@ struct h2_task {
     struct {
         struct h2_bucket_beam *beam;
         apr_bucket_brigade *bb;
-        apr_bucket_brigade *tmp;
         apr_read_type_e block;
         unsigned int chunked : 1;
         unsigned int eos : 1;

Modified: httpd/httpd/trunk/modules/http2/h2_version.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_version.h?rev=1757985&r1=1757984&r2=1757985&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_version.h (original)
+++ httpd/httpd/trunk/modules/http2/h2_version.h Sat Aug 27 10:39:24 2016
@@ -26,7 +26,7 @@
  * @macro
  * Version number of the http2 module as c string
  */
-#define MOD_HTTP2_VERSION "1.6.0-DEV"
+#define MOD_HTTP2_VERSION "1.6.1-DEV"
 
 /**
  * @macro
@@ -34,7 +34,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 0x010600
+#define MOD_HTTP2_VERSION_NUM 0x010601
 
 
 #endif /* mod_h2_h2_version_h */