httpd-cvs mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ic...@apache.org
Subject svn commit: r1779459 - in /httpd/httpd/trunk: CHANGES modules/http2/h2_bucket_eoc.c modules/http2/h2_session.c
Date Thu, 19 Jan 2017 12:57:08 GMT
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)



Mime
View raw message