You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by ac...@apache.org on 2017/04/20 14:10:38 UTC
[4/4] qpid-dispatch git commit: DISPATCH-739: Clear context pointer
on qdr_connection_t at close
DISPATCH-739: Clear context pointer on qdr_connection_t at close
Atomically clear the context pointer of a qdr_connection_t when an AMQP
connection closes to prevent the connection being used after it is freed.
Project: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/commit/0fb5b54d
Tree: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/tree/0fb5b54d
Diff: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/diff/0fb5b54d
Branch: refs/heads/master
Commit: 0fb5b54d1c69b12b035491554f535840a1db52a9
Parents: ef47800
Author: Alan Conway <ac...@redhat.com>
Authored: Thu Apr 20 06:39:41 2017 -0400
Committer: Alan Conway <ac...@redhat.com>
Committed: Thu Apr 20 10:04:06 2017 -0400
----------------------------------------------------------------------
src/router_core/connections.c | 17 +++++++++++++++--
src/router_core/router_core_private.h | 4 +++-
src/router_node.c | 2 ++
src/server.c | 3 ++-
4 files changed, 22 insertions(+), 4 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/0fb5b54d/src/router_core/connections.c
----------------------------------------------------------------------
diff --git a/src/router_core/connections.c b/src/router_core/connections.c
index be437a7..08791c2 100644
--- a/src/router_core/connections.c
+++ b/src/router_core/connections.c
@@ -89,6 +89,7 @@ qdr_connection_t *qdr_connection_opened(qdr_core_t *core,
DEQ_INIT(conn->work_list);
conn->connection_info->role = conn->role;
conn->work_lock = sys_mutex();
+ conn->context_lock = sys_mutex();
if (vhost) {
conn->tenant_space_len = strlen(vhost) + 1;
@@ -116,8 +117,14 @@ void qdr_connection_closed(qdr_connection_t *conn)
void qdr_connection_set_context(qdr_connection_t *conn, void *context)
{
- if (conn)
+ if (conn) {
+ /* TODO aconway 2017-04-20: note this could be an atomic pointer store,
+ * but it must provide a full memory barrier.
+ */
+ sys_mutex_lock(conn->context_lock);
conn->user_context = context;
+ sys_mutex_unlock(conn->context_lock);
+ }
}
qdr_connection_info_t *qdr_connection_info(bool is_encrypted,
@@ -180,7 +187,13 @@ qdr_connection_info_t *qdr_connection_info(bool is_encrypted,
void *qdr_connection_get_context(const qdr_connection_t *conn)
{
- return conn ? conn->user_context : 0;
+ void *ret = NULL;
+ if (conn) {
+ sys_mutex_lock(conn->context_lock);
+ ret = conn->user_context;
+ sys_mutex_unlock(conn->context_lock);
+ }
+ return ret;
}
http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/0fb5b54d/src/router_core/router_core_private.h
----------------------------------------------------------------------
diff --git a/src/router_core/router_core_private.h b/src/router_core/router_core_private.h
index bacf531..c60cd9c 100644
--- a/src/router_core/router_core_private.h
+++ b/src/router_core/router_core_private.h
@@ -517,7 +517,6 @@ struct qdr_connection_t {
DEQ_LINKS_N(ACTIVATE, qdr_connection_t);
uint64_t identity;
qdr_core_t *core;
- void *user_context;
bool incoming;
bool in_activate_list;
qdr_connection_role_t role;
@@ -535,6 +534,9 @@ struct qdr_connection_t {
char *tenant_space;
int tenant_space_len;
qdr_connection_info_t *connection_info;
+
+ sys_mutex_t *context_lock;
+ void *user_context; /* Updated from IO thread on close */
};
ALLOC_DECLARE(qdr_connection_t);
http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/0fb5b54d/src/router_node.c
----------------------------------------------------------------------
diff --git a/src/router_node.c b/src/router_node.c
index 914c736..9761468 100644
--- a/src/router_node.c
+++ b/src/router_node.c
@@ -761,6 +761,7 @@ static int AMQP_closed_handler(void *type_context, qd_connection_t *conn, void *
qdr_connection_t *qdrc = (qdr_connection_t*) qd_connection_get_context(conn);
if (qdrc) {
+ qdr_connection_set_context(qdrc, NULL);
qdr_connection_closed(qdrc);
qd_connection_set_context(conn, 0);
}
@@ -869,6 +870,7 @@ static void CORE_link_first_attach(void *context,
{
qd_router_t *router = (qd_router_t*) context;
qd_connection_t *qconn = (qd_connection_t*) qdr_connection_get_context(conn);
+ if (!qconn) return; /* Connection is already closed */
//
// Create a new link to be attached
http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/0fb5b54d/src/server.c
----------------------------------------------------------------------
diff --git a/src/server.c b/src/server.c
index 42fed58..31322ea 100644
--- a/src/server.c
+++ b/src/server.c
@@ -1069,9 +1069,9 @@ static void *thread_run(void *arg)
qd_policy_socket_close(qd_server->qd->policy, ctx);
}
- qdpn_connector_free(cxtr);
invoke_deferred_calls(ctx, true); // Discard any pending deferred calls
sys_mutex_free(ctx->deferred_call_lock);
+ qdpn_connector_free(cxtr);
free_qd_connection(ctx);
qd_server->threads_active--;
sys_mutex_unlock(qd_server->lock);
@@ -1557,6 +1557,7 @@ void qd_connection_set_context(qd_connection_t *conn, void *context)
void *qd_connection_get_context(qd_connection_t *conn)
{
+ /* FIXME aconway 2017-04-20: needs to be thread safe with respect to deletion */
return conn->user_context;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org