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 2015/10/23 13:54:42 UTC
svn commit: r1710185 - in /httpd/httpd/trunk: CHANGES
modules/http2/h2_conn.c modules/http2/h2_conn_io.c modules/http2/h2_mplx.c
modules/http2/h2_session.c modules/http2/h2_worker.c
modules/http2/h2_workers.c modules/http2/h2_workers.h
Author: icing
Date: Fri Oct 23 11:54:41 2015
New Revision: 1710185
URL: http://svn.apache.org/viewvc?rev=1710185&view=rev
Log:
improvements in http2 connection and worker shutdown, hopefully addressing high load segfaults
Modified:
httpd/httpd/trunk/CHANGES
httpd/httpd/trunk/modules/http2/h2_conn.c
httpd/httpd/trunk/modules/http2/h2_conn_io.c
httpd/httpd/trunk/modules/http2/h2_mplx.c
httpd/httpd/trunk/modules/http2/h2_session.c
httpd/httpd/trunk/modules/http2/h2_worker.c
httpd/httpd/trunk/modules/http2/h2_workers.c
httpd/httpd/trunk/modules/http2/h2_workers.h
Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1710185&r1=1710184&r2=1710185&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Fri Oct 23 11:54:41 2015
@@ -1,6 +1,11 @@
-*- coding: utf-8 -*-
Changes with Apache 2.5.0
+ *) mod_http2: reworked deallocation on connection shutdown and worker
+ abort. Separate parent pool for all workers. worker threads are joined
+ on planned worker shutdown.
+ [Yann Ylavic, Stefan Eissing]
+
*) mod_ssl: when receiving requests for other virtual hosts than the handshake
server, the SSL parameters are checked for equality. With equal
configuration, requests are passed for processing. Any change will trigger
Modified: httpd/httpd/trunk/modules/http2/h2_conn.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_conn.c?rev=1710185&r1=1710184&r2=1710185&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_conn.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_conn.c Fri Oct 23 11:54:41 2015
@@ -254,7 +254,7 @@ apr_status_t h2_session_process(h2_sessi
have_written = 1;
wait_micros = 0;
}
- else if (status == APR_EAGAIN) {
+ else if (APR_STATUS_IS_EAGAIN(status)) {
/* nop */
}
else if (status == APR_TIMEUP) {
Modified: httpd/httpd/trunk/modules/http2/h2_conn_io.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_conn_io.c?rev=1710185&r1=1710184&r2=1710185&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_conn_io.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_conn_io.c Fri Oct 23 11:54:41 2015
@@ -180,6 +180,10 @@ static apr_status_t flush_out(apr_bucket
apr_status_t status;
apr_off_t bblen;
+ if (APR_BRIGADE_EMPTY(bb)) {
+ return APR_SUCCESS;
+ }
+
ap_update_child_status(io->connection->sbh, SERVER_BUSY_WRITE, NULL);
status = apr_brigade_length(bb, 1, &bblen);
if (status == APR_SUCCESS) {
@@ -193,6 +197,9 @@ static apr_status_t flush_out(apr_bucket
return status;
}
+/* Bring the current buffer content into the output brigade, appropriately
+ * chunked.
+ */
static apr_status_t bucketeer_buffer(h2_conn_io *io) {
const char *data = io->buffer;
apr_size_t remaining = io->buflen;
@@ -282,30 +289,22 @@ apr_status_t h2_conn_io_flush(h2_conn_io
if (io->unflushed) {
apr_status_t status;
if (io->buflen > 0) {
+ /* something in the buffer, put it in the output brigade */
ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, io->connection,
"h2_conn_io: flush, flushing %ld bytes", (long)io->buflen);
bucketeer_buffer(io);
io->buflen = 0;
}
- /* Append flush.
- */
- APR_BRIGADE_INSERT_TAIL(io->output,
- apr_bucket_flush_create(io->output->bucket_alloc));
-
- /* Send it out through installed filters (TLS) to the client */
+ /* Send it out */
status = flush_out(io->output, io);
- if (status == APR_SUCCESS) {
- /* These are all fine and no reason for concern. Everything else
- * is interesting. */
- io->unflushed = 0;
- }
- else {
+ if (status != APR_SUCCESS) {
ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, io->connection,
- "h2_conn_io: flush error");
+ "h2_conn_io: flush");
+ return status;
}
-
- return status;
+
+ io->unflushed = 0;
}
return APR_SUCCESS;
}
Modified: httpd/httpd/trunk/modules/http2/h2_mplx.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_mplx.c?rev=1710185&r1=1710184&r2=1710185&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_mplx.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_mplx.c Fri Oct 23 11:54:41 2015
@@ -144,12 +144,18 @@ static void reference(h2_mplx *m)
apr_atomic_inc32(&m->refs);
}
-static void release(h2_mplx *m)
+static void release(h2_mplx *m, int lock)
{
if (!apr_atomic_dec32(&m->refs)) {
+ if (lock) {
+ apr_thread_mutex_lock(m->lock);
+ }
if (m->join_wait) {
apr_thread_cond_signal(m->join_wait);
}
+ if (lock) {
+ apr_thread_mutex_unlock(m->lock);
+ }
}
}
@@ -159,7 +165,7 @@ void h2_mplx_reference(h2_mplx *m)
}
void h2_mplx_release(h2_mplx *m)
{
- release(m);
+ release(m, 1);
}
static void workers_register(h2_mplx *m) {
@@ -188,29 +194,17 @@ apr_status_t h2_mplx_release_and_join(h2
status = apr_thread_mutex_lock(m->lock);
if (APR_SUCCESS == status) {
- int attempts = 0;
-
- release(m);
+ release(m, 0);
while (apr_atomic_read32(&m->refs) > 0) {
m->join_wait = wait;
- ap_log_cerror(APLOG_MARK, (attempts? APLOG_INFO : APLOG_DEBUG),
- 0, m->c,
+ ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, m->c,
"h2_mplx(%ld): release_join, refs=%d, waiting...",
m->id, m->refs);
- apr_thread_cond_timedwait(wait, m->lock, apr_time_from_sec(10));
- if (++attempts >= 6) {
- ap_log_cerror(APLOG_MARK, APLOG_WARNING, 0, m->c,
- APLOGNO(02952)
- "h2_mplx(%ld): join attempts exhausted, refs=%d",
- m->id, m->refs);
- break;
- }
- }
- if (m->join_wait) {
- ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, m->c,
- "h2_mplx(%ld): release_join -> destroy", m->id);
+ apr_thread_cond_wait(wait, m->lock);
}
m->join_wait = NULL;
+ ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, m->c,
+ "h2_mplx(%ld): release_join -> destroy", m->id);
apr_thread_mutex_unlock(m->lock);
h2_mplx_destroy(m);
}
@@ -353,7 +347,7 @@ apr_status_t h2_mplx_in_read(h2_mplx *m,
if (io) {
io->input_arrived = iowait;
status = h2_io_in_read(io, bb, 0);
- while (status == APR_EAGAIN
+ while (APR_STATUS_IS_EAGAIN(status)
&& !is_aborted(m, &status)
&& block == APR_BLOCK_READ) {
apr_thread_cond_wait(io->input_arrived, m->lock);
Modified: httpd/httpd/trunk/modules/http2/h2_session.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_session.c?rev=1710185&r1=1710184&r2=1710185&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_session.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_session.c Fri Oct 23 11:54:41 2015
@@ -41,17 +41,16 @@ static int frame_print(const nghttp2_fra
static int h2_session_status_from_apr_status(apr_status_t rv)
{
- switch (rv) {
- case APR_SUCCESS:
- return NGHTTP2_NO_ERROR;
- case APR_EAGAIN:
- case APR_TIMEUP:
- return NGHTTP2_ERR_WOULDBLOCK;
- case APR_EOF:
+ if (rv == APR_SUCCESS) {
+ return NGHTTP2_NO_ERROR;
+ }
+ else if (APR_STATUS_IS_EAGAIN(rv)) {
+ return NGHTTP2_ERR_WOULDBLOCK;
+ }
+ else if (APR_STATUS_IS_EOF(rv)) {
return NGHTTP2_ERR_EOF;
- default:
- return NGHTTP2_ERR_PROTO;
}
+ return NGHTTP2_ERR_PROTO;
}
static int stream_open(h2_session *session, int stream_id)
@@ -107,7 +106,7 @@ static ssize_t send_cb(nghttp2_session *
if (status == APR_SUCCESS) {
return length;
}
- if (status == APR_EAGAIN || status == APR_TIMEUP) {
+ if (APR_STATUS_IS_EAGAIN(status)) {
return NGHTTP2_ERR_WOULDBLOCK;
}
ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, session->c,
@@ -509,12 +508,11 @@ static int on_send_data_cb(nghttp2_sessi
if (status == APR_SUCCESS) {
return 0;
}
- else if (status != APR_EOF) {
- ap_log_cerror(APLOG_MARK, APLOG_ERR, status, session->c,
+ else {
+ ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, session->c,
APLOGNO(02925)
"h2_stream(%ld-%d): failed send_data_cb",
session->id, (int)stream_id);
- return NGHTTP2_ERR_CALLBACK_FAILURE;
}
return h2_session_status_from_apr_status(status);
@@ -732,10 +730,12 @@ apr_status_t h2_session_abort(h2_session
rv = 0; /* ...gracefully shut down */
break;
case APR_EBADF: /* connection unusable, terminate silently */
- case APR_ECONNABORTED:
- rv = NGHTTP2_ERR_EOF;
- break;
default:
+ if (APR_STATUS_IS_ECONNABORTED(reason)
+ || APR_STATUS_IS_ECONNRESET(reason)
+ || APR_STATUS_IS_EBADF(reason)) {
+ rv = NGHTTP2_ERR_EOF;
+ }
break;
}
}
@@ -911,7 +911,7 @@ apr_status_t h2_session_write(h2_session
if (status == APR_SUCCESS) {
flush_output = 1;
}
- else if (status != APR_EAGAIN) {
+ else if (!APR_STATUS_IS_EAGAIN(status)) {
return status;
}
Modified: httpd/httpd/trunk/modules/http2/h2_worker.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_worker.c?rev=1710185&r1=1710184&r2=1710185&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_worker.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_worker.c Fri Oct 23 11:54:41 2015
@@ -71,6 +71,19 @@ static void* APR_THREAD_FUNC execute(apr
return NULL;
}
+static apr_status_t cleanup_join_thread(void *ctx)
+{
+ h2_worker *w = ctx;
+ /* do the join only when the worker is aborted. Otherwise,
+ * we are probably in a process shutdown.
+ */
+ if (w->thread && w->aborted) {
+ apr_status_t rv;
+ apr_thread_join(&rv, w->thread);
+ }
+ return APR_SUCCESS;
+}
+
h2_worker *h2_worker_create(int id,
apr_pool_t *parent_pool,
apr_threadattr_t *attr,
@@ -110,6 +123,7 @@ h2_worker *h2_worker_create(int id,
return NULL;
}
+ apr_pool_pre_cleanup_register(pool, w, cleanup_join_thread);
apr_thread_create(&w->thread, attr, execute, w, pool);
}
return w;
Modified: httpd/httpd/trunk/modules/http2/h2_workers.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_workers.c?rev=1710185&r1=1710184&r2=1710185&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_workers.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_workers.c Fri Oct 23 11:54:41 2015
@@ -42,6 +42,22 @@ static int in_list(h2_workers *workers,
return 0;
}
+static void cleanup_zombies(h2_workers *workers, int lock) {
+ if (lock) {
+ apr_thread_mutex_lock(workers->lock);
+ }
+ while (!H2_WORKER_LIST_EMPTY(&workers->zombies)) {
+ h2_worker *zombie = H2_WORKER_LIST_FIRST(&workers->zombies);
+ H2_WORKER_REMOVE(zombie);
+ ap_log_error(APLOG_MARK, APLOG_TRACE1, 0, workers->s,
+ "h2_workers: cleanup zombie %d", zombie->id);
+ h2_worker_destroy(zombie);
+ }
+ if (lock) {
+ apr_thread_mutex_unlock(workers->lock);
+ }
+}
+
/**
* Get the next task for the given worker. Will block until a task arrives
@@ -123,23 +139,12 @@ static apr_status_t get_mplx_next(h2_wor
if (!task) {
/* Need to wait for either a new mplx to arrive.
*/
+ cleanup_zombies(workers, 0);
+
if (workers->worker_count > workers->min_size) {
apr_time_t now = apr_time_now();
if (now >= (start_wait + max_wait)) {
/* waited long enough without getting a task. */
- status = APR_TIMEUP;
- }
- else {
- ap_log_error(APLOG_MARK, APLOG_TRACE1, 0, workers->s,
- "h2_worker(%d): waiting signal, "
- "worker_count=%d", worker->id,
- (int)workers->worker_count);
- status = apr_thread_cond_timedwait(workers->mplx_added,
- workers->lock, max_wait);
- }
-
- if (status == APR_TIMEUP) {
- /* waited long enough */
if (workers->worker_count > workers->min_size) {
ap_log_error(APLOG_MARK, APLOG_TRACE1, 0,
workers->s,
@@ -148,6 +153,12 @@ static apr_status_t get_mplx_next(h2_wor
break;
}
}
+ ap_log_error(APLOG_MARK, APLOG_TRACE1, 0, workers->s,
+ "h2_worker(%d): waiting signal, "
+ "worker_count=%d", worker->id,
+ (int)workers->worker_count);
+ apr_thread_cond_timedwait(workers->mplx_added,
+ workers->lock, max_wait);
}
else {
ap_log_error(APLOG_MARK, APLOG_TRACE1, 0, workers->s,
@@ -194,11 +205,11 @@ static void worker_done(h2_worker *worke
h2_workers *workers = (h2_workers *)ctx;
apr_status_t status = apr_thread_mutex_lock(workers->lock);
if (status == APR_SUCCESS) {
- ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, workers->s,
+ ap_log_error(APLOG_MARK, APLOG_TRACE1, 0, workers->s,
"h2_worker(%d): done", h2_worker_get_id(worker));
H2_WORKER_REMOVE(worker);
--workers->worker_count;
- h2_worker_destroy(worker);
+ H2_WORKER_LIST_INSERT_TAIL(&workers->zombies, worker);
apr_thread_mutex_unlock(workers->lock);
}
@@ -213,7 +224,7 @@ static apr_status_t add_worker(h2_worker
if (!w) {
return APR_ENOMEM;
}
- ap_log_error(APLOG_MARK, APLOG_TRACE2, 0, workers->s,
+ ap_log_error(APLOG_MARK, APLOG_TRACE1, 0, workers->s,
"h2_workers: adding worker(%d)", h2_worker_get_id(w));
++workers->worker_count;
H2_WORKER_LIST_INSERT_TAIL(&workers->workers, w);
@@ -235,15 +246,22 @@ static apr_status_t h2_workers_start(h2_
return status;
}
-h2_workers *h2_workers_create(server_rec *s, apr_pool_t *pool,
+h2_workers *h2_workers_create(server_rec *s, apr_pool_t *server_pool,
int min_size, int max_size)
{
apr_status_t status;
h2_workers *workers;
+ apr_pool_t *pool;
+
AP_DEBUG_ASSERT(s);
- AP_DEBUG_ASSERT(pool);
- status = APR_SUCCESS;
+ AP_DEBUG_ASSERT(server_pool);
+ /* let's have our own pool that will be parent to all h2_worker
+ * instances we create. This happens in various threads, but always
+ * guarded by our lock. Without this pool, all subpool creations would
+ * happen on the pool handed to us, which we do not guard.
+ */
+ apr_pool_create(&pool, server_pool);
workers = apr_pcalloc(pool, sizeof(h2_workers));
if (workers) {
workers->s = s;
@@ -255,6 +273,7 @@ h2_workers *h2_workers_create(server_rec
apr_threadattr_create(&workers->thread_attr, workers->pool);
APR_RING_INIT(&workers->workers, h2_worker, link);
+ APR_RING_INIT(&workers->zombies, h2_worker, link);
APR_RING_INIT(&workers->mplxs, h2_mplx, link);
status = apr_thread_mutex_create(&workers->lock,
@@ -278,6 +297,9 @@ h2_workers *h2_workers_create(server_rec
void h2_workers_destroy(h2_workers *workers)
{
+ /* before we go, cleanup any zombie workers that may have accumulated */
+ cleanup_zombies(workers, 1);
+
if (workers->mplx_added) {
apr_thread_cond_destroy(workers->mplx_added);
workers->mplx_added = NULL;
@@ -294,6 +316,10 @@ void h2_workers_destroy(h2_workers *work
h2_worker *w = H2_WORKER_LIST_FIRST(&workers->workers);
H2_WORKER_REMOVE(w);
}
+ if (workers->pool) {
+ apr_pool_destroy(workers->pool);
+ /* workers is gone */
+ }
}
apr_status_t h2_workers_register(h2_workers *workers, struct h2_mplx *m)
@@ -320,6 +346,9 @@ apr_status_t h2_workers_register(h2_work
add_worker(workers);
}
+ /* cleanup any zombie workers that may have accumulated */
+ cleanup_zombies(workers, 0);
+
apr_thread_mutex_unlock(workers->lock);
}
return status;
@@ -334,6 +363,9 @@ apr_status_t h2_workers_unregister(h2_wo
H2_MPLX_REMOVE(m);
status = APR_SUCCESS;
}
+ /* cleanup any zombie workers that may have accumulated */
+ cleanup_zombies(workers, 0);
+
apr_thread_mutex_unlock(workers->lock);
}
return status;
Modified: httpd/httpd/trunk/modules/http2/h2_workers.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_workers.h?rev=1710185&r1=1710184&r2=1710185&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_workers.h (original)
+++ httpd/httpd/trunk/modules/http2/h2_workers.h Fri Oct 23 11:54:41 2015
@@ -42,6 +42,7 @@ struct h2_workers {
apr_threadattr_t *thread_attr;
APR_RING_HEAD(h2_worker_list, h2_worker) workers;
+ APR_RING_HEAD(h2_worker_zombies, h2_worker) zombies;
APR_RING_HEAD(h2_mplx_list, h2_mplx) mplxs;
int worker_count;