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 2017/09/28 12:26:28 UTC

svn commit: r1809981 - in /httpd/httpd/trunk: CHANGES modules/http2/h2_bucket_beam.c modules/http2/h2_bucket_beam.h modules/http2/h2_mplx.c modules/http2/h2_task.c modules/http2/h2_version.h modules/http2/h2_workers.c

Author: icing
Date: Thu Sep 28 12:26:28 2017
New Revision: 1809981

URL: http://svn.apache.org/viewvc?rev=1809981&view=rev
Log:
On the trunk:
mod_http2: v0.10.12, removed optimization for mutex handling in bucket beams that could lead to assertion failure in edge cases.

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_task.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=1809981&r1=1809980&r2=1809981&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Thu Sep 28 12:26:28 2017
@@ -1,6 +1,9 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.5.0
 
+  * mod_http2: v0.10.12, removed optimization for mutex handling in bucket beams that 
+    could lead to assertion failure in edge cases. [Stefan Eissing] 
+     
   *) mod_md: v0.9.7
      - Use of the new module flag
      - Removed obsolete function from interface to mod_ssl. 

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=1809981&r1=1809980&r2=1809981&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_bucket_beam.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_bucket_beam.c Thu Sep 28 12:26:28 2017
@@ -210,16 +210,7 @@ static apr_status_t mutex_enter(void *ct
 
 static apr_status_t enter_yellow(h2_bucket_beam *beam, h2_beam_lock *pbl)
 {
-    h2_beam_mutex_enter *enter = beam->m_enter;
-    if (enter) {
-        void *ctx = beam->m_ctx;
-        if (ctx) {
-            return enter(ctx, pbl);
-        }
-    }
-    pbl->mutex = NULL;
-    pbl->leave = NULL;
-    return APR_SUCCESS;
+    return mutex_enter(beam, pbl);
 }
 
 static void leave_yellow(h2_bucket_beam *beam, h2_beam_lock *pbl)
@@ -568,13 +559,12 @@ static apr_status_t beam_cleanup(void *d
 {
     h2_bucket_beam *beam = data;
     apr_status_t status = APR_SUCCESS;
-    int safe_send = !beam->m_enter || (beam->owner == H2_BEAM_OWNER_SEND);
-    int safe_recv = !beam->m_enter || (beam->owner == H2_BEAM_OWNER_RECV);
+    int safe_send = (beam->owner == H2_BEAM_OWNER_SEND);
+    int safe_recv = (beam->owner == H2_BEAM_OWNER_RECV);
     
     /* 
      * Owner of the beam is going away, depending on which side it owns,
-     * cleanup strategies will differ with multi-thread protection
-     * still in place (beam->m_enter).
+     * cleanup strategies will differ.
      *
      * In general, receiver holds references to memory from sender. 
      * Clean up receiver first, if safe, then cleanup sender, if safe.
@@ -680,29 +670,6 @@ apr_size_t h2_beam_buffer_size_get(h2_bu
     return buffer_size;
 }
 
-void h2_beam_mutex_set(h2_bucket_beam *beam, 
-                       h2_beam_mutex_enter m_enter,
-                       void *m_ctx)
-{
-    h2_beam_lock bl;
-    
-    if (enter_yellow(beam, &bl) == APR_SUCCESS) {
-        beam->m_enter = m_enter;
-        beam->m_ctx   = m_ctx;
-        leave_yellow(beam, &bl);
-    }
-}
-
-void h2_beam_mutex_enable(h2_bucket_beam *beam)
-{
-    h2_beam_mutex_set(beam, mutex_enter, beam);
-}
-
-void h2_beam_mutex_disable(h2_bucket_beam *beam)
-{
-    h2_beam_mutex_set(beam, NULL, NULL);
-}
-
 void h2_beam_timeout_set(h2_bucket_beam *beam, apr_interval_time_t timeout)
 {
     h2_beam_lock bl;

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=1809981&r1=1809980&r2=1809981&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_bucket_beam.h (original)
+++ httpd/httpd/trunk/modules/http2/h2_bucket_beam.h Thu Sep 28 12:26:28 2017
@@ -64,13 +64,11 @@ typedef struct {
  * via the h2_beam_send(). It gives the beam to the green thread which then
  * can receive buckets into its own brigade via h2_beam_receive().
  *
- * Sending and receiving can happen concurrently, if a thread mutex is set
- * for the beam, see h2_beam_mutex_set.
+ * Sending and receiving can happen concurrently.
  *
  * The beam can limit the amount of data it accepts via the buffer_size. This
- * can also be adjusted during its lifetime. When the beam not only gets a 
- * mutex but als a condition variable (in h2_beam_mutex_set()), sends and
- * receives can be done blocking. A timeout can be set for such blocks.
+ * can also be adjusted during its lifetime. Sends and receives can be done blocking. 
+ * A timeout can be set for such blocks.
  *
  * Care needs to be taken when terminating the beam. The beam registers at
  * the pool it was created with and will cleanup after itself. However, if
@@ -191,8 +189,6 @@ struct h2_bucket_beam {
 
     struct apr_thread_mutex_t *lock;
     struct apr_thread_cond_t *change;
-    void *m_ctx;
-    h2_beam_mutex_enter *m_enter;
     
     apr_off_t cons_bytes_reported;    /* amount of bytes reported as consumed */
     h2_beam_ev_callback *cons_ev_cb;
@@ -315,13 +311,6 @@ int h2_beam_is_closed(h2_bucket_beam *be
  */
 apr_status_t h2_beam_wait_empty(h2_bucket_beam *beam, apr_read_type_e block);
 
-void h2_beam_mutex_set(h2_bucket_beam *beam, 
-                       h2_beam_mutex_enter m_enter,
-                       void *m_ctx);
-
-void h2_beam_mutex_enable(h2_bucket_beam *beam);
-void h2_beam_mutex_disable(h2_bucket_beam *beam);
-
 /** 
  * Set/get the timeout for blocking read/write operations. Only works
  * if a mutex has been set for the beam.

Modified: httpd/httpd/trunk/modules/http2/h2_mplx.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_mplx.c?rev=1809981&r1=1809980&r2=1809981&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_mplx.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_mplx.c Thu Sep 28 12:26:28 2017
@@ -542,9 +542,6 @@ static apr_status_t out_open(h2_mplx *m,
         h2_beam_on_file_beam(stream->output, h2_beam_no_files, NULL);
     }
     
-    /* time to protect the beam against multi-threaded use */
-    h2_beam_mutex_enable(stream->output);
-    
     /* we might see some file buckets in the output, see
      * if we have enough handles reserved. */
     check_data_for(m, stream, 0);
@@ -852,10 +849,6 @@ static void task_done(h2_mplx *m, h2_tas
                           H2_STRM_MSG(stream, "task_done, stream open")); 
             if (stream->input) {
                 h2_beam_leave(stream->input);
-                h2_beam_mutex_disable(stream->input);
-            }
-            if (stream->output) {
-                h2_beam_mutex_disable(stream->output);
             }
 
             /* more data will not arrive, resume the stream */
@@ -868,10 +861,6 @@ static void task_done(h2_mplx *m, h2_tas
                       H2_STRM_MSG(stream, "task_done, in hold"));
         if (stream->input) {
             h2_beam_leave(stream->input);
-            h2_beam_mutex_disable(stream->input);
-        }
-        if (stream->output) {
-            h2_beam_mutex_disable(stream->output);
         }
         stream_joined(m, 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=1809981&r1=1809980&r2=1809981&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_task.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_task.c Thu Sep 28 12:26:28 2017
@@ -613,10 +613,6 @@ apr_status_t h2_task_do(h2_task *task, a
     h2_ctx_create_for(c, task);
     apr_table_setn(c->notes, H2_TASK_ID_NOTE, task->id);
 
-    if (task->input.beam) {
-        h2_beam_mutex_enable(task->input.beam);
-    }
-    
     h2_slave_run_pre_connection(c, ap_get_conn_socket(c));            
 
     task->input.bb = apr_brigade_create(task->pool, c->bucket_alloc);

Modified: httpd/httpd/trunk/modules/http2/h2_version.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_version.h?rev=1809981&r1=1809980&r2=1809981&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_version.h (original)
+++ httpd/httpd/trunk/modules/http2/h2_version.h Thu Sep 28 12:26:28 2017
@@ -26,7 +26,7 @@
  * @macro
  * Version number of the http2 module as c string
  */
-#define MOD_HTTP2_VERSION "1.10.11-DEV"
+#define MOD_HTTP2_VERSION "1.10.12-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 0x010a0a
+#define MOD_HTTP2_VERSION_NUM 0x010a0b
 
 
 #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=1809981&r1=1809980&r2=1809981&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_workers.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_workers.c Thu Sep 28 12:26:28 2017
@@ -98,6 +98,8 @@ static apr_status_t activate_slot(h2_wor
         }
     }
     
+    ap_log_error(APLOG_MARK, APLOG_TRACE2, 0, workers->s,
+                 "h2_workers: new thread for slot %d", slot->id); 
     /* thread will either immediately start work or add itself
      * to the idle queue */
     apr_thread_create(&slot->thread, workers->thread_attr, slot_run, slot,