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/01/19 12:57:08 UTC
svn commit: r1779459 - in /httpd/httpd/trunk: CHANGES
modules/http2/h2_bucket_eoc.c modules/http2/h2_session.c
Author: icing
Date: Thu Jan 19 12:57:08 2017
New Revision: 1779459
URL: http://svn.apache.org/viewvc?rev=1779459&view=rev
Log:
On the trunk:
*) mod_http2: change lifetime of h2_session record to address crashes
during connection shutdown. [Yann Ylavic, Stefan Eissing]
Modified:
httpd/httpd/trunk/CHANGES
httpd/httpd/trunk/modules/http2/h2_bucket_eoc.c
httpd/httpd/trunk/modules/http2/h2_session.c
Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1779459&r1=1779458&r2=1779459&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Thu Jan 19 12:57:08 2017
@@ -1,6 +1,9 @@
-*- coding: utf-8 -*-
Changes with Apache 2.5.0
+ *) mod_http2: change lifetime of h2_session record to address crashes
+ during connection shutdown. [Yann Ylavic, Stefan Eissing]
+
*) mod_proxy_fcgi: Return to 2.4.20-and-earlier behavior of leaving
a "proxy:fcgi://" prefix in the SCRIPT_FILENAME environment variable by
default. Add ProxyFCGIBackendType to allow the type of backend to be
Modified: httpd/httpd/trunk/modules/http2/h2_bucket_eoc.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_bucket_eoc.c?rev=1779459&r1=1779458&r2=1779459&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_bucket_eoc.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_bucket_eoc.c Thu Jan 19 12:57:08 2017
@@ -33,20 +33,6 @@ typedef struct {
h2_session *session;
} h2_bucket_eoc;
-static apr_status_t bucket_cleanup(void *data)
-{
- h2_session **psession = data;
-
- if (*psession) {
- /*
- * If bucket_destroy is called after us, this prevents
- * bucket_destroy from trying to destroy the pool again.
- */
- *psession = NULL;
- }
- return APR_SUCCESS;
-}
-
static apr_status_t bucket_read(apr_bucket *b, const char **str,
apr_size_t *len, apr_read_type_e block)
{
@@ -77,12 +63,7 @@ apr_bucket * h2_bucket_eoc_create(apr_bu
APR_BUCKET_INIT(b);
b->free = apr_bucket_free;
b->list = list;
- b = h2_bucket_eoc_make(b, session);
- if (session) {
- h2_bucket_eoc *h = b->data;
- apr_pool_pre_cleanup_register(session->pool, &h->session, bucket_cleanup);
- }
- return b;
+ return h2_bucket_eoc_make(b, session);
}
static void bucket_destroy(void *data)
@@ -92,10 +73,7 @@ static void bucket_destroy(void *data)
if (apr_bucket_shared_destroy(h)) {
h2_session *session = h->session;
apr_bucket_free(h);
- if (session) {
- h2_session_eoc_callback(session);
- /* all is gone now */
- }
+ h2_session_eoc_callback(session);
}
}
Modified: httpd/httpd/trunk/modules/http2/h2_session.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_session.c?rev=1779459&r1=1779458&r2=1779459&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_session.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_session.c Thu Jan 19 12:57:08 2017
@@ -730,7 +730,7 @@ static apr_status_t init_callbacks(conn_
return APR_SUCCESS;
}
-static void h2_session_destroy(h2_session *session)
+static void h2_session_cleanup(h2_session *session)
{
ap_assert(session);
@@ -752,13 +752,17 @@ static void h2_session_destroy(h2_sessio
if (APLOGctrace1(session->c)) {
ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, session->c,
- "h2_session(%ld): destroy", session->id);
- }
- if (session->pool) {
- apr_pool_destroy(session->pool);
+ "h2_session(%ld): cleanup", session->id);
}
}
+static void h2_session_destroy(h2_session *session)
+{
+ apr_pool_t *p = session->pool;
+ session->pool = NULL;
+ apr_pool_destroy(p);
+}
+
static apr_status_t h2_session_shutdown_notice(h2_session *session)
{
apr_status_t status;
@@ -857,9 +861,8 @@ static apr_status_t session_pool_cleanup
"goodbye, clients will be confused, should not happen",
session->id);
}
- /* keep us from destroying the pool, since that is already ongoing. */
+ h2_session_cleanup(session);
session->pool = NULL;
- h2_session_destroy(session);
return APR_SUCCESS;
}
@@ -918,7 +921,9 @@ static h2_session *h2_session_create_int
}
apr_pool_tag(pool, "h2_session");
- session = apr_pcalloc(pool, sizeof(h2_session));
+ /* get h2_session a lifetime beyond its pool and everything
+ * connected to it. */
+ session = apr_pcalloc(c->pool, sizeof(h2_session));
if (session) {
int rv;
nghttp2_mem *mem;
@@ -1007,7 +1012,7 @@ static h2_session *h2_session_create_int
h2_session_destroy(session);
return NULL;
}
-
+
n = h2_config_geti(session->config, H2_CONF_PUSH_DIARY_SIZE);
session->push_diary = h2_push_diary_create(session->pool, n);
@@ -1039,10 +1044,14 @@ h2_session *h2_session_rcreate(request_r
void h2_session_eoc_callback(h2_session *session)
{
- ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, session->c,
- "session(%ld): cleanup and destroy", session->id);
- apr_pool_cleanup_kill(session->pool, session, session_pool_cleanup);
- h2_session_destroy(session);
+ /* keep us from destroying the pool, if it's already done (cleanup). */
+ if (session->pool) {
+ ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, session->c,
+ "session(%ld): cleanup and destroy", session->id);
+ apr_pool_cleanup_kill(session->pool, session, session_pool_cleanup);
+ h2_session_cleanup(session);
+ h2_session_destroy(session);
+ }
}
static apr_status_t h2_session_start(h2_session *session, int *rv)