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.


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1809981 13f79535-47bb-0310-9956-ffa450edef68

6 files changed, 11 insertions, 68 deletions

diff --git a/modules/http2/h2_bucket_beam.c b/modules/http2/h2_bucket_beam.c
index 9f3bc70911..6b8c645be2 100644
--- a/modules/http2/h2_bucket_beam.c
+++ b/modules/http2/h2_bucket_beam.c
@@ -210,16 +210,7 @@ static apr_status_t mutex_enter(void *ctx, h2_beam_lock *pbl)
 
 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 *data)
 {
     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_bucket_beam *beam)
     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;
diff --git a/modules/http2/h2_bucket_beam.h b/modules/http2/h2_bucket_beam.h
index 7a67455baa..c48d15382b 100644
--- a/modules/http2/h2_bucket_beam.h
+++ b/modules/http2/h2_bucket_beam.h
@@ -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 *beam);
  */
 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.
diff --git a/modules/http2/h2_mplx.c b/modules/http2/h2_mplx.c
index 12300fc334..7da8fb65de 100644
--- a/modules/http2/h2_mplx.c
+++ b/modules/http2/h2_mplx.c
@@ -542,9 +542,6 @@ static apr_status_t out_open(h2_mplx *m, int stream_id, h2_bucket_beam *beam)
         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_task *task, h2_req_engine *ngn)
                           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_task *task, h2_req_engine *ngn)
                       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);
     }
diff --git a/modules/http2/h2_task.c b/modules/http2/h2_task.c
index 2ab4145586..1f2f4c2578 100644
--- a/modules/http2/h2_task.c
+++ b/modules/http2/h2_task.c
@@ -613,10 +613,6 @@ apr_status_t h2_task_do(h2_task *task, apr_thread_t *thread, int worker_id)
     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);
diff --git a/modules/http2/h2_version.h b/modules/http2/h2_version.h
index 50e9abc03f..24a96f9965 100644
--- a/modules/http2/h2_version.h
+++ b/modules/http2/h2_version.h
@@ -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 */
diff --git a/modules/http2/h2_workers.c b/modules/http2/h2_workers.c
index 5abddd4a1d..1276250644 100644
--- a/modules/http2/h2_workers.c
+++ b/modules/http2/h2_workers.c
@@ -98,6 +98,8 @@ static apr_status_t activate_slot(h2_workers *workers, h2_slot *slot)
         }
     }
     
+    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,