You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by kg...@apache.org on 2020/10/15 14:42:18 UTC

[qpid-dispatch] branch dev-protocol-adaptors-2 updated: DISPATCH-1802: HTTP/1.x prevent core connection activation race

This is an automated email from the ASF dual-hosted git repository.

kgiusti pushed a commit to branch dev-protocol-adaptors-2
in repository https://gitbox.apache.org/repos/asf/qpid-dispatch.git


The following commit(s) were added to refs/heads/dev-protocol-adaptors-2 by this push:
     new 22dd27b  DISPATCH-1802: HTTP/1.x prevent core connection activation race
22dd27b is described below

commit 22dd27bf239911683efccea440de5a353a3cbdc5
Author: Kenneth Giusti <kg...@apache.org>
AuthorDate: Tue Oct 13 16:05:04 2020 -0400

    DISPATCH-1802: HTTP/1.x prevent core connection activation race
    
    This closes #878
---
 src/adaptors/http1/http1_adaptor.c | 23 ++++++++++++++++-------
 src/adaptors/http1/http1_client.c  |  6 +++++-
 src/adaptors/http1/http1_private.h |  2 +-
 src/adaptors/http1/http1_server.c  |  8 +++++++-
 4 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/src/adaptors/http1/http1_adaptor.c b/src/adaptors/http1/http1_adaptor.c
index 4663385..e1fb155 100644
--- a/src/adaptors/http1/http1_adaptor.c
+++ b/src/adaptors/http1/http1_adaptor.c
@@ -417,15 +417,20 @@ void qdr_http1_free_written_buffers(qdr_http1_connection_t *hconn)
 //
 static void _core_connection_activate_CT(void *context, qdr_connection_t *conn)
 {
-    qdr_http1_connection_t *hconn = (qdr_http1_connection_t*) qdr_connection_get_context(conn);
-    if (!hconn) return;
+    bool activated = false;
 
-    qd_log(qdr_http1_adaptor->log, QD_LOG_DEBUG, "[C%"PRIu64"] Connection activate", hconn->conn_id);
+    sys_mutex_lock(qdr_http1_adaptor->lock);
+    qdr_http1_connection_t *hconn = (qdr_http1_connection_t*) qdr_connection_get_context(conn);
+    if (hconn) {
+        if (hconn->raw_conn) {
+            pn_raw_connection_wake(hconn->raw_conn);
+            activated = true;
+        }
+    }
+    sys_mutex_unlock(qdr_http1_adaptor->lock);
 
-    if (hconn->raw_conn)
-        pn_raw_connection_wake(hconn->raw_conn);
-    else
-        qd_log(qdr_http1_adaptor->log, QD_LOG_DEBUG, "[C%"PRIu64"] missing raw connection!", hconn->conn_id);
+    if (hconn && activated)
+        qd_log(qdr_http1_adaptor->log, QD_LOG_DEBUG, "[C%"PRIu64"] Connection activate", hconn->conn_id);
 }
 
 
@@ -601,7 +606,11 @@ static void _core_conn_close(void *context, qdr_connection_t *conn, qdr_error_t
 
         char *qdr_error = error ? qdr_error_description(error) : 0;
         qdr_http1_close_connection(hconn, qdr_error);
+
+        sys_mutex_lock(qdr_http1_adaptor->lock);
         qdr_connection_set_context(conn, 0);
+        sys_mutex_unlock(qdr_http1_adaptor->lock);
+
         hconn->qdr_conn = 0;
         hconn->in_link = hconn->out_link = 0;
         free(qdr_error);
diff --git a/src/adaptors/http1/http1_client.c b/src/adaptors/http1/http1_client.c
index 6298d8a..3ad9ad6 100644
--- a/src/adaptors/http1/http1_client.c
+++ b/src/adaptors/http1/http1_client.c
@@ -378,7 +378,12 @@ static void _handle_connection_events(pn_event_t *e, qd_server_t *qd_server, voi
     case PN_RAW_CONNECTION_DISCONNECTED: {
         qd_log(log, QD_LOG_INFO, "[C%i] Disconnected", hconn->conn_id);
         pn_raw_connection_set_context(hconn->raw_conn, 0);
+
+        // prevent core from waking this connection
+        sys_mutex_lock(qdr_http1_adaptor->lock);
+        qdr_connection_set_context(hconn->qdr_conn, 0);
         hconn->raw_conn = 0;
+        sys_mutex_unlock(qdr_http1_adaptor->lock);
 
         if (hconn->out_link) {
             qdr_link_set_context(hconn->out_link, 0);
@@ -391,7 +396,6 @@ static void _handle_connection_events(pn_event_t *e, qd_server_t *qd_server, voi
             hconn->in_link = 0;
         }
         if (hconn->qdr_conn) {
-            qdr_connection_set_context(hconn->qdr_conn, 0);
             qdr_connection_closed(hconn->qdr_conn);
             hconn->qdr_conn = 0;
         }
diff --git a/src/adaptors/http1/http1_private.h b/src/adaptors/http1/http1_private.h
index 8e29ae2..1e48146 100644
--- a/src/adaptors/http1/http1_private.h
+++ b/src/adaptors/http1/http1_private.h
@@ -49,7 +49,7 @@ typedef struct qdr_http1_adaptor_t {
     qdr_core_t                  *core;
     qdr_protocol_adaptor_t      *adaptor;
     qd_log_source_t             *log;
-    sys_mutex_t                 *lock;  // for the lists
+    sys_mutex_t                 *lock;  // for the lists and activation
     qd_http_lsnr_list_t          listeners;
     qd_http_connector_list_t     connectors;
     qdr_http1_connection_list_t  connections;
diff --git a/src/adaptors/http1/http1_server.c b/src/adaptors/http1/http1_server.c
index 81ca858..5c056f1 100644
--- a/src/adaptors/http1/http1_server.c
+++ b/src/adaptors/http1/http1_server.c
@@ -410,15 +410,21 @@ static void _handle_connection_events(pn_event_t *e, qd_server_t *qd_server, voi
     }
     case PN_RAW_CONNECTION_DISCONNECTED: {
         pn_raw_connection_set_context(hconn->raw_conn, 0);
-        hconn->raw_conn = 0;
         hconn->close_connection = false;
 
         if (!hconn->qdr_conn) {
             // the router has closed this connection so do not try to
             // re-establish it
             qd_log(log, QD_LOG_INFO, "[C%i] Connection closed", hconn->conn_id);
+            hconn->raw_conn = 0;
             _server_connection_free(hconn);
             return;
+        } else {
+            // prevent the core from activating this connection
+            // until we can re-establish it
+            sys_mutex_lock(qdr_http1_adaptor->lock);
+            hconn->raw_conn = 0;
+            sys_mutex_unlock(qdr_http1_adaptor->lock);
         }
 
         // if the current request was not completed, cancel it.  it's ok if


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org