qpid-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kgiu...@apache.org
Subject [qpid-dispatch] branch master updated: DISPATCH-1549: properly free terminus on transactional link rejection
Date Fri, 17 Jan 2020 14:22:26 GMT
This is an automated email from the ASF dual-hosted git repository.

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


The following commit(s) were added to refs/heads/master by this push:
     new 0e4c8d8  DISPATCH-1549: properly free terminus on transactional link rejection
0e4c8d8 is described below

commit 0e4c8d8bd6a66b1f0c5dab2e1c6bb19c59f2933c
Author: Kenneth Giusti <kgiusti@apache.org>
AuthorDate: Thu Jan 16 11:58:31 2020 -0500

    DISPATCH-1549: properly free terminus on transactional link rejection
---
 src/router_core/connections.c                      | 11 +++++---
 .../modules/address_lookup_client/lookup_client.c  | 29 ++++++++++------------
 tests/lsan.supp                                    |  1 -
 3 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/src/router_core/connections.c b/src/router_core/connections.c
index 1a44baf..b7284a4 100644
--- a/src/router_core/connections.c
+++ b/src/router_core/connections.c
@@ -573,6 +573,8 @@ void qdr_link_second_attach(qdr_link_t *link, qdr_terminus_t *source,
qdr_termin
 
     set_safe_ptr_qdr_connection_t(link->conn, &action->args.connection.conn);
     set_safe_ptr_qdr_link_t(link, &action->args.connection.link);
+
+    // ownership of source/target passed to core, core must free them when done
     action->args.connection.source = source;
     action->args.connection.target = target;
     qdr_action_enqueue(link->core, action);
@@ -1647,12 +1649,15 @@ static void qdr_link_inbound_second_attach_CT(qdr_core_t *core, qdr_action_t
*ac
 {
     qdr_link_t       *link = safe_deref_qdr_link_t(action->args.connection.link);
     qdr_connection_t *conn = safe_deref_qdr_connection_t(action->args.connection.conn);
-    if (discard || !link || !conn)
-        return;
-
     qdr_terminus_t   *source = action->args.connection.source;
     qdr_terminus_t   *target = action->args.connection.target;
 
+    if (discard || !link || !conn) {
+        qdr_terminus_free(source);
+        qdr_terminus_free(target);
+        return;
+    }
+
     link->oper_status = QDR_LINK_OPER_UP;
     link->attach_count++;
 
diff --git a/src/router_core/modules/address_lookup_client/lookup_client.c b/src/router_core/modules/address_lookup_client/lookup_client.c
index aa6f968..ea4c888 100644
--- a/src/router_core/modules/address_lookup_client/lookup_client.c
+++ b/src/router_core/modules/address_lookup_client/lookup_client.c
@@ -326,8 +326,8 @@ static void qdr_link_react_to_first_attach_CT(qdr_core_t       *core,
                                               qdr_address_t    *addr,
                                               qdr_link_t       *link,
                                               qd_direction_t    dir,
-                                              qdr_terminus_t   *source,
-                                              qdr_terminus_t   *target,
+                                              qdr_terminus_t   *source,  // must free when
done
+                                              qdr_terminus_t   *target,  // must free when
done
                                               bool              link_route,
                                               bool              unavailable,
                                               bool              core_endpoint,
@@ -337,27 +337,20 @@ static void qdr_link_react_to_first_attach_CT(qdr_core_t       *core,
 
     if (core_endpoint) {
         qdrc_endpoint_do_bound_attach_CT(core, addr, link, source, target);
+        source = target = 0;  // ownership passed to qdrc_endpoint_do_bound_attach_CT
     }
     else if (unavailable && qdr_terminus_is_coordinator(dir == QD_INCOMING ? target
: source) && !addr) {
         qdr_link_outbound_detach_CT(core, link, 0, QDR_CONDITION_COORDINATOR_PRECONDITION_FAILED,
true);
-        qdr_terminus_free(source);
-        qdr_terminus_free(target);
     }
     else if (unavailable) {
         qdr_link_outbound_detach_CT(core, link, qdr_error(QD_AMQP_COND_NOT_FOUND, "Node not
found"), 0, true);
-        qdr_terminus_free(source);
-        qdr_terminus_free(target);
     }
-
     else if (!addr) {
         //
         // No route to this destination, reject the link
         //
         qdr_link_outbound_detach_CT(core, link, 0, QDR_CONDITION_NO_ROUTE_TO_DESTINATION,
true);
-        qdr_terminus_free(source);
-        qdr_terminus_free(target);
     }
-
     else if (link_route) {
         //
         // This is a link-routed destination, forward the attach to the next hop
@@ -365,21 +358,18 @@ static void qdr_link_react_to_first_attach_CT(qdr_core_t       *core,
         qdr_terminus_t *term = dir == QD_INCOMING ? target : source;
         if (qdr_terminus_survives_disconnect(term) && !core->qd->allow_resumable_link_route)
{
             qdr_link_outbound_detach_CT(core, link, 0, QDR_CONDITION_INVALID_LINK_EXPIRATION,
true);
-            qdr_terminus_free(source);
-            qdr_terminus_free(target);
         } else {
             if (conn->role != QDR_ROLE_INTER_ROUTER && conn->connection_info)
{
                 link->disambiguated_name = disambiguated_link_name(conn->connection_info,
link->name);
             }
             bool success = qdr_forward_attach_CT(core, addr, link, source, target);
-            if (!success) {
+            if (success) {
+                source = target = 0;  // ownership passed to qdr_forward_attach_CT
+            } else {
                 qdr_link_outbound_detach_CT(core, link, 0, QDR_CONDITION_NO_ROUTE_TO_DESTINATION,
true);
-                qdr_terminus_free(source);
-                qdr_terminus_free(target);
             }
         }
     }
-
     else if (dir == QD_INCOMING && qdr_terminus_is_coordinator(target)) {
         //
         // This target terminus is a coordinator.
@@ -389,6 +379,7 @@ static void qdr_link_react_to_first_attach_CT(qdr_core_t       *core,
         // The attach response should have a null target to indicate refusal and the immediately
coming detach.
         //
         qdr_link_outbound_second_attach_CT(core, link, source, 0);
+        source = 0;  // ownership passed to qdr_link_outbound_second_attach_CT
 
         //
         // Now, send back a detach with the error amqp:precondition-failed
@@ -401,6 +392,7 @@ static void qdr_link_react_to_first_attach_CT(qdr_core_t       *core,
         //
         qdr_core_bind_address_link_CT(core, addr, link);
         qdr_link_outbound_second_attach_CT(core, link, source, target);
+        source = target = 0;  // ownership passed to qdr_link_outbound_second_attach_CT
 
         //
         // Issue the initial credit only if one of the following
@@ -427,6 +419,11 @@ static void qdr_link_react_to_first_attach_CT(qdr_core_t       *core,
         if (link->conn->role == QDR_ROLE_EDGE_CONNECTION)
             qdrc_event_link_raise(core, QDRC_EVENT_LINK_EDGE_DATA_ATTACHED, link);
     }
+
+    if (source)
+        qdr_terminus_free(source);
+    if (target)
+        qdr_terminus_free(target);
 }
 
 
diff --git a/tests/lsan.supp b/tests/lsan.supp
index a37c399..765aef8 100644
--- a/tests/lsan.supp
+++ b/tests/lsan.supp
@@ -35,7 +35,6 @@ leak:qdr_route_add_auto_link_CT
 leak:qdr_route_add_link_route_CT
 leak:qdr_route_connection_opened_CT
 leak:qdr_subscribe_CT
-leak:qdr_terminus
 leak:router_annotate_message
 leak:qd_container_register_node_type
 


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


Mime
View raw message