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/12/23 11:34:33 UTC
svn commit: r1775813 - 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_proxy_session.c
modules/http2/h2_stream.c modules/http2/h2_task.c
Author: icing
Date: Fri Dec 23 11:34:32 2016
New Revision: 1775813
URL: http://svn.apache.org/viewvc?rev=1775813&view=rev
Log:
On the trunk:
Fix mod_h2/github issue #126: correct lifetime of data sent on temp pools
* modules/http2/h2_bucket_beam.c
- ignore send pools that are sub-pools of the existing one
- added h2_beam_send_from() to allow explicit registering of the
correct pool for the sending
* modules/http2/h2_bucket_beam.h
- add prototype for h2_beam_send_from()
* modules/http2/h2_mplx.c
- adding logging of output beam state
* modules/http2/h2_stream.c
- register stream pool for sending data on input beam
* modules/http2/h2_task.c
- register task pool on output beam on creation
- adding trace logging
* modules/http2/h2_proxy_session.c
- fixing a type in a comment while we're at it
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_proxy_session.c
httpd/httpd/trunk/modules/http2/h2_stream.c
httpd/httpd/trunk/modules/http2/h2_task.c
Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1775813&r1=1775812&r2=1775813&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Fri Dec 23 11:34:32 2016
@@ -1,6 +1,10 @@
-*- coding: utf-8 -*-
Changes with Apache 2.5.0
+ *) mod_http2: fixes https://github.com/icing/mod_h2/issues/126 e.g. beam
+ bucket lifetime handling when data is sent over temporary pools.
+ [Stefan Eissing]
+
*) mod_proxy_{ajp,fcgi}: Fix a possible crash when reusing an established
backend connection, happening with LogLevel trace2 or higher configured,
or at any log level with compilers not detected as C99 compliant (e.g.
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=1775813&r1=1775812&r2=1775813&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_bucket_beam.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_bucket_beam.c Fri Dec 23 11:34:32 2016
@@ -461,6 +461,12 @@ static void beam_set_send_pool(h2_bucket
{
/* if the beam owner is the receiver, monitor sender pool lifetime */
if (beam->owner == H2_BEAM_OWNER_RECV && beam->send_pool != pool) {
+ if (beam->send_pool && pool
+ && apr_pool_is_ancestor(beam->send_pool, pool)) {
+ /* when sender uses sub-pools to transmit data, stick
+ * to the lifetime of the pool we already have. */
+ return;
+ }
if (beam->send_pool) {
apr_pool_cleanup_kill(beam->send_pool, beam, beam_send_cleanup);
}
@@ -620,6 +626,8 @@ void h2_beam_abort(h2_bucket_beam *beam)
r_purge_sent(beam);
h2_blist_cleanup(&beam->send_list);
report_consumption(beam, 0);
+ ap_log_perror(APLOG_MARK, APLOG_WARNING, 0, beam->send_pool,
+ "h2_beam(%d-%s): aborted", beam->id, beam->tag);
}
if (beam->m_cond) {
apr_thread_cond_broadcast(beam->m_cond);
@@ -799,6 +807,17 @@ static apr_status_t append_bucket(h2_buc
return APR_SUCCESS;
}
+void h2_beam_send_from(h2_bucket_beam *beam, apr_pool_t *p)
+{
+ h2_beam_lock bl;
+ /* Called from the red thread to add buckets to the beam */
+ if (enter_yellow(beam, &bl) == APR_SUCCESS) {
+ r_purge_sent(beam);
+ beam_set_send_pool(beam, p);
+ leave_yellow(beam, &bl);
+ }
+}
+
apr_status_t h2_beam_send(h2_bucket_beam *beam,
apr_bucket_brigade *red_brigade,
apr_read_type_e block)
@@ -809,8 +828,10 @@ apr_status_t h2_beam_send(h2_bucket_beam
/* Called from the red thread to add buckets to the beam */
if (enter_yellow(beam, &bl) == APR_SUCCESS) {
+ ap_log_perror(APLOG_MARK, APLOG_WARNING, 0, beam->send_pool,
+ "h2_beam(%d-%s): send", beam->id, beam->tag);
r_purge_sent(beam);
- if (red_brigade) {
+ if (red_brigade && !beam->send_pool) {
beam_set_send_pool(beam, red_brigade->p);
}
@@ -850,6 +871,8 @@ apr_status_t h2_beam_receive(h2_bucket_b
/* Called from the green thread to take buckets from the beam */
if (enter_yellow(beam, &bl) == APR_SUCCESS) {
transfer:
+ ap_log_perror(APLOG_MARK, APLOG_WARNING, 0, beam->send_pool,
+ "h2_beam(%d-%s): receive", beam->id, beam->tag);
if (beam->aborted) {
if (beam->recv_buffer && !APR_BRIGADE_EMPTY(beam->recv_buffer)) {
apr_brigade_cleanup(beam->recv_buffer);
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=1775813&r1=1775812&r2=1775813&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_bucket_beam.h (original)
+++ httpd/httpd/trunk/modules/http2/h2_bucket_beam.h Fri Dec 23 11:34:32 2016
@@ -255,6 +255,13 @@ apr_status_t h2_beam_send(h2_bucket_beam
apr_read_type_e block);
/**
+ * Register the pool from which future buckets are send. This defines
+ * the lifetime of the buckets, e.g. the pool should not be cleared/destroyed
+ * until the data is no longer needed (or has been received).
+ */
+void h2_beam_send_from(h2_bucket_beam *beam, apr_pool_t *p);
+
+/**
* Receive buckets from the beam into the given brigade. Will return APR_EOF
* when reading past an EOS bucket. Reads can be blocking until data is
* available or the beam has been closed. Non-blocking calls return APR_EAGAIN
Modified: httpd/httpd/trunk/modules/http2/h2_mplx.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_mplx.c?rev=1775813&r1=1775812&r2=1775813&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_mplx.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_mplx.c Fri Dec 23 11:34:32 2016
@@ -53,8 +53,8 @@ static void h2_beam_log(h2_bucket_beam *
apr_size_t off = 0;
off += apr_snprintf(buffer+off, H2_ALEN(buffer)-off, "cl=%d, ", beam->closed);
- off += h2_util_bl_print(buffer+off, H2_ALEN(buffer)-off, "red", ", ", &beam->send_list);
- off += h2_util_bb_print(buffer+off, H2_ALEN(buffer)-off, "green", ", ", beam->recv_buffer);
+ off += h2_util_bl_print(buffer+off, H2_ALEN(buffer)-off, "to_send", ", ", &beam->send_list);
+ off += h2_util_bb_print(buffer+off, H2_ALEN(buffer)-off, "recv_buffer", ", ", beam->recv_buffer);
off += h2_util_bl_print(buffer+off, H2_ALEN(buffer)-off, "hold", ", ", &beam->hold_list);
off += h2_util_bl_print(buffer+off, H2_ALEN(buffer)-off, "purge", "", &beam->purge_list);
@@ -703,9 +703,14 @@ static apr_status_t out_open(h2_mplx *m,
return APR_ECONNABORTED;
}
- ap_log_cerror(APLOG_MARK, APLOG_TRACE1, status, m->c,
- "h2_mplx(%s): out open", task->id);
-
+ if (APLOGctrace2(m->c)) {
+ h2_beam_log(beam, stream_id, "out_open", m->c, APLOG_TRACE2);
+ }
+ else {
+ ap_log_cerror(APLOG_MARK, APLOG_TRACE1, status, m->c,
+ "h2_mplx(%s): out open", task->id);
+ }
+
h2_beam_on_consumed(stream->output, stream_output_consumed, task);
h2_beam_on_produced(stream->output, output_produced, m);
beamed_count = h2_beam_get_files_beamed(stream->output);
Modified: httpd/httpd/trunk/modules/http2/h2_proxy_session.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_proxy_session.c?rev=1775813&r1=1775812&r2=1775813&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_proxy_session.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_proxy_session.c Fri Dec 23 11:34:32 2016
@@ -344,7 +344,7 @@ static void h2_proxy_stream_end_headers_
/* If USE_CANONICAL_NAME_OFF was configured for the proxy virtual host,
* then the server name returned by ap_get_server_name() is the
- * origin server name (which does make too much sense with Via: headers)
+ * origin server name (which doesn't make sense with Via: headers)
* so we use the proxy vhost's name instead.
*/
if (server_name == stream->r->hostname) {
Modified: httpd/httpd/trunk/modules/http2/h2_stream.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_stream.c?rev=1775813&r1=1775812&r2=1775813&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_stream.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_stream.c Fri Dec 23 11:34:32 2016
@@ -203,6 +203,7 @@ h2_stream *h2_stream_open(int id, apr_po
stream->can_be_cleaned = 1;
h2_beam_create(&stream->input, pool, id, "input", H2_BEAM_OWNER_SEND, 0);
+ h2_beam_send_from(stream->input, stream->pool);
h2_beam_create(&stream->output, pool, id, "output", H2_BEAM_OWNER_RECV, 0);
set_state(stream, H2_STREAM_ST_OPEN);
Modified: httpd/httpd/trunk/modules/http2/h2_task.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_task.c?rev=1775813&r1=1775812&r2=1775813&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_task.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_task.c Fri Dec 23 11:34:32 2016
@@ -64,6 +64,24 @@ static void H2_TASK_OUT_LOG(int lvl, h2_
}
}
+static void h2_beam_log(h2_bucket_beam *beam, int id, const char *msg,
+ conn_rec *c, int level)
+{
+ if (beam && APLOG_C_IS_LEVEL(c,level)) {
+ char buffer[2048];
+ apr_size_t off = 0;
+
+ off += apr_snprintf(buffer+off, H2_ALEN(buffer)-off, "cl=%d, ", beam->closed);
+ off += h2_util_bl_print(buffer+off, H2_ALEN(buffer)-off, "red", ", ", &beam->send_list);
+ off += h2_util_bb_print(buffer+off, H2_ALEN(buffer)-off, "green", ", ", beam->recv_buffer);
+ off += h2_util_bl_print(buffer+off, H2_ALEN(buffer)-off, "hold", ", ", &beam->hold_list);
+ off += h2_util_bl_print(buffer+off, H2_ALEN(buffer)-off, "purge", "", &beam->purge_list);
+
+ ap_log_cerror(APLOG_MARK, level, 0, c, "beam(%ld-%d): %s %s",
+ c->id, id, msg, buffer);
+ }
+}
+
/*******************************************************************************
* task input handling
******************************************************************************/
@@ -97,9 +115,12 @@ static apr_status_t send_out(h2_task *ta
apr_brigade_length(bb, 0, &written);
H2_TASK_OUT_LOG(APLOG_TRACE2, task, bb, "h2_task send_out");
+ h2_beam_log(task->output.beam, task->stream_id, "send_out(before)", task->c, APLOG_TRACE2);
/* engines send unblocking */
status = h2_beam_send(task->output.beam, bb,
block? APR_BLOCK_READ : APR_NONBLOCK_READ);
+ h2_beam_log(task->output.beam, task->stream_id, "send_out(after)", task->c, APLOG_TRACE2);
+
if (APR_STATUS_IS_EAGAIN(status)) {
apr_brigade_length(bb, 0, &left);
written -= left;
@@ -508,9 +529,10 @@ h2_task *h2_task_create(conn_rec *c, int
task->input.beam = input;
task->output.beam = output;
+ h2_beam_send_from(output, task->pool);
apr_thread_cond_create(&task->cond, pool);
-
h2_ctx_create_for(c, task);
+
return task;
}