DISPATCH-390: refactor - remove dead code - qd_bind_state_t set but never used - threads_active set but never used - is_connector set but never used - pyflakes errors in system_tests_*manage*.py - mark private connection_manager.c functions as static
Project: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/repo Commit: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/commit/c2a1a32d Tree: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/tree/c2a1a32d Diff: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/diff/c2a1a32d Branch: refs/heads/master Commit: c2a1a32d1313eb7669aa67a5c216de32c63e70cc Parents: 5e61dd9 Author: Alan Conway <[email protected]> Authored: Fri Mar 3 00:33:29 2017 -0500 Committer: Alan Conway <[email protected]> Committed: Tue Apr 25 18:13:59 2017 -0400 ---------------------------------------------------------------------- include/qpid/dispatch/connection_manager.h | 8 -- include/qpid/dispatch/server.h | 20 ++++ src/connection_manager.c | 148 +++++++++++------------- src/server.c | 5 - src/server_private.h | 6 - tests/system_tests_management.py | 24 +--- tests/system_tests_qdmanage.py | 3 +- 7 files changed, 93 insertions(+), 121 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/c2a1a32d/include/qpid/dispatch/connection_manager.h ---------------------------------------------------------------------- diff --git a/include/qpid/dispatch/connection_manager.h b/include/qpid/dispatch/connection_manager.h index 28a944c..12ac35e 100644 --- a/include/qpid/dispatch/connection_manager.h +++ b/include/qpid/dispatch/connection_manager.h @@ -54,18 +54,10 @@ void qd_connection_manager_free(qd_connection_manager_t *cm); void qd_config_listener_free(qd_connection_manager_t *cm, qd_config_listener_t *cl); /** - * Free the SSL Profile. Only SSL Profiles that are not referenced from other connectors/listeners can be freed. - * @return true if the ssl_profile has been freed, false if it is being referenced and cannot be freed - */ -bool qd_config_ssl_profile_free(qd_connection_manager_t *cm, qd_config_ssl_profile_t *ssl_profile); - - -/** * Free all the resources associated with a config connector */ void qd_config_connector_free(qd_connection_manager_t *cm, qd_config_connector_t *cl); - /** * Start the configured Listeners and Connectors * http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/c2a1a32d/include/qpid/dispatch/server.h ---------------------------------------------------------------------- diff --git a/include/qpid/dispatch/server.h b/include/qpid/dispatch/server.h index 529c566..a466ec0 100644 --- a/include/qpid/dispatch/server.h +++ b/include/qpid/dispatch/server.h @@ -447,6 +447,26 @@ typedef struct qd_server_config_t { * Configured failover list */ qd_failover_list_t *failover_list; + + /** + * @name These fields are not primary configuration, they are computed. + * @{ + */ + + /** + * Concatenated connect/listen address "host:port" + */ + char *host_port; + + /** + * Set for listeners that are part of the initial router configuration. + * An error in setting up initial listeners must shut down the router. + */ + bool exit_on_error; + + /** + * @} + */ } qd_server_config_t; http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/c2a1a32d/src/connection_manager.c ---------------------------------------------------------------------- diff --git a/src/connection_manager.c b/src/connection_manager.c index d8a5ecc..10e6fe4 100644 --- a/src/connection_manager.c +++ b/src/connection_manager.c @@ -49,8 +49,6 @@ struct qd_config_ssl_profile_t { }; struct qd_config_listener_t { - bool is_connector; - qd_bind_state_t state; qd_listener_t *listener; qd_config_ssl_profile_t *ssl_profile; qd_server_config_t configuration; @@ -62,7 +60,6 @@ DEQ_DECLARE(qd_config_ssl_profile_t, qd_config_ssl_profile_list_t); struct qd_config_connector_t { - bool is_connector; DEQ_LINKS(qd_config_connector_t); qd_connector_t *connector; qd_server_config_t configuration; @@ -415,6 +412,28 @@ bool is_log_component_enabled(qd_log_bits log_message, char *component_name) { } +static bool config_ssl_profile_free(qd_connection_manager_t *cm, qd_config_ssl_profile_t *ssl_profile) +{ + if (sys_atomic_get(&ssl_profile->ref_count) != 0) { + return false; + } + + DEQ_REMOVE(cm->config_ssl_profiles, ssl_profile); + + free(ssl_profile->name); + free(ssl_profile->ssl_password); + free(ssl_profile->ssl_trusted_certificate_db); + free(ssl_profile->ssl_trusted_certificates); + free(ssl_profile->ssl_uid_format); + free(ssl_profile->ssl_display_name_file); + free(ssl_profile->ssl_certificate_file); + free(ssl_profile->ssl_private_key_file); + free(ssl_profile); + return true; + +} + + qd_config_ssl_profile_t *qd_dispatch_configure_ssl_profile(qd_dispatch_t *qd, qd_entity_t *entity) { qd_error_clear(); @@ -475,24 +494,37 @@ qd_config_ssl_profile_t *qd_dispatch_configure_ssl_profile(qd_dispatch_t *qd, qd error: qd_log(cm->log_source, QD_LOG_ERROR, "Unable to create ssl profile: %s", qd_error_message()); - qd_config_ssl_profile_free(cm, ssl_profile); + config_ssl_profile_free(cm, ssl_profile); return 0; } + +static void config_listener_free(qd_connection_manager_t *cm, qd_config_listener_t *cl) +{ + if (cl->listener) { + qd_server_listener_close(cl->listener); + qd_server_listener_free(cl->listener); + cl->listener = 0; + } + if (cl->ssl_profile) { + sys_atomic_dec(&cl->ssl_profile->ref_count); + } + free(cl); +} + + qd_config_listener_t *qd_dispatch_configure_listener(qd_dispatch_t *qd, qd_entity_t *entity) { qd_error_clear(); qd_connection_manager_t *cm = qd->connection_manager; qd_config_listener_t *cl = NEW(qd_config_listener_t); - cl->is_connector = false; - cl->state = QD_BIND_NONE; cl->listener = 0; cl->ssl_profile = 0; qd_config_ssl_profile_t *ssl_profile = 0; if (load_server_config(qd, &cl->configuration, entity, &ssl_profile) != QD_ERROR_NONE) { qd_log(cm->log_source, QD_LOG_ERROR, "Unable to create config listener: %s", qd_error_message()); - qd_config_listener_free(qd->connection_manager, cl); + config_listener_free(qd->connection_manager, cl); return 0; } cl->ssl_profile = ssl_profile; @@ -503,7 +535,7 @@ qd_config_listener_t *qd_dispatch_configure_listener(qd_dispatch_t *qd, qd_entit free(fol); if (cl->configuration.failover_list == 0) { qd_log(cm->log_source, QD_LOG_ERROR, "Error parsing failover list: %s", fol_error); - qd_config_listener_free(qd->connection_manager, cl); + config_listener_free(qd->connection_manager, cl); return 0; } } else @@ -535,6 +567,17 @@ qd_error_t qd_entity_refresh_connector(qd_entity_t* entity, void *impl) } +static void config_connector_free(qd_connection_manager_t *cm, qd_config_connector_t *cc) +{ + if (cc->connector) + qd_server_connector_free(cc->connector); + if (cc->ssl_profile) { + sys_atomic_dec(&cc->ssl_profile->ref_count); + } + free(cc); +} + + qd_config_connector_t *qd_dispatch_configure_connector(qd_dispatch_t *qd, qd_entity_t *entity) { qd_error_clear(); @@ -542,11 +585,10 @@ qd_config_connector_t *qd_dispatch_configure_connector(qd_dispatch_t *qd, qd_ent qd_config_connector_t *cc = NEW(qd_config_connector_t); ZERO(cc); - cc->is_connector = true; qd_config_ssl_profile_t *ssl_profile = 0; if (load_server_config(qd, &cc->configuration, entity, &ssl_profile) != QD_ERROR_NONE) { qd_log(cm->log_source, QD_LOG_ERROR, "Unable to create config connector: %s", qd_error_message()); - qd_config_connector_free(qd->connection_manager, cc); + config_connector_free(qd->connection_manager, cc); return 0; } cc->ssl_profile = ssl_profile; @@ -587,7 +629,7 @@ void qd_connection_manager_free(qd_connection_manager_t *cm) while (cl) { DEQ_REMOVE_HEAD(cm->config_listeners); qd_server_config_free(&cl->configuration); - qd_config_listener_free(cm, cl); + config_listener_free(cm, cl); cl = DEQ_HEAD(cm->config_listeners); } @@ -595,13 +637,13 @@ void qd_connection_manager_free(qd_connection_manager_t *cm) while (cc) { DEQ_REMOVE_HEAD(cm->config_connectors); qd_server_config_free(&cc->configuration); - qd_config_connector_free(cm, cc); + config_connector_free(cm, cc); cc = DEQ_HEAD(cm->config_connectors); } qd_config_ssl_profile_t *sslp = DEQ_HEAD(cm->config_ssl_profiles); while (sslp) { - qd_config_ssl_profile_free(cm, sslp); + config_ssl_profile_free(cm, sslp); sslp = DEQ_HEAD(cm->config_ssl_profiles); } @@ -609,6 +651,9 @@ void qd_connection_manager_free(qd_connection_manager_t *cm) } +/** NOTE: non-static qd_connection_manager_* functions are called from the python agent */ + + void qd_connection_manager_start(qd_dispatch_t *qd) { static bool first_start = true; @@ -616,20 +661,14 @@ void qd_connection_manager_start(qd_dispatch_t *qd) qd_config_connector_t *cc = DEQ_HEAD(qd->connection_manager->config_connectors); while (cl) { - if (cl->listener == 0 ) - if (cl->state == QD_BIND_NONE) { //Try to start listening only if we have never tried to listen on that port before - cl->listener = qd_server_listen(qd, &cl->configuration, cl); - if (cl->listener) - cl->state = QD_BIND_SUCCESSFUL; - else { - cl->state = QD_BIND_FAILED; - if (first_start) { - qd_log(qd->connection_manager->log_source, QD_LOG_CRITICAL, - "Socket bind failed during initial configuration - process exiting"); - exit(1); - } - } + if (cl->listener == 0 ) { + cl->listener = qd_server_listen(qd, &cl->configuration, cl); + if (!cl->listener && first_start) { + qd_log(qd->connection_manager->log_source, QD_LOG_CRITICAL, + "Socket bind failed during initial configuration"); + exit(1); } + } cl = DEQ_NEXT(cl); } @@ -643,57 +682,6 @@ void qd_connection_manager_start(qd_dispatch_t *qd) } -void qd_config_connector_free(qd_connection_manager_t *cm, qd_config_connector_t *cc) -{ - if (cc->connector) - qd_server_connector_free(cc->connector); - - if (cc->ssl_profile) { - sys_atomic_dec(&cc->ssl_profile->ref_count); - } - - free(cc); -} - - -void qd_config_listener_free(qd_connection_manager_t *cm, qd_config_listener_t *cl) -{ - if (cl->listener) { - qd_server_listener_close(cl->listener); - qd_server_listener_free(cl->listener); - cl->listener = 0; - } - - if (cl->ssl_profile) { - sys_atomic_dec(&cl->ssl_profile->ref_count); - } - - free(cl); -} - - -bool qd_config_ssl_profile_free(qd_connection_manager_t *cm, qd_config_ssl_profile_t *ssl_profile) -{ - if (sys_atomic_get(&ssl_profile->ref_count) != 0) { - return false; - } - - DEQ_REMOVE(cm->config_ssl_profiles, ssl_profile); - - free(ssl_profile->name); - free(ssl_profile->ssl_password); - free(ssl_profile->ssl_trusted_certificate_db); - free(ssl_profile->ssl_trusted_certificates); - free(ssl_profile->ssl_uid_format); - free(ssl_profile->ssl_display_name_file); - free(ssl_profile->ssl_certificate_file); - free(ssl_profile->ssl_private_key_file); - free(ssl_profile); - return true; - -} - - void qd_connection_manager_delete_listener(qd_dispatch_t *qd, void *impl) { qd_config_listener_t *cl = (qd_config_listener_t*) impl; @@ -701,7 +689,7 @@ void qd_connection_manager_delete_listener(qd_dispatch_t *qd, void *impl) if (cl) { qd_server_listener_close(cl->listener); DEQ_REMOVE(qd->connection_manager->config_listeners, cl); - qd_config_listener_free(qd->connection_manager, cl); + config_listener_free(qd->connection_manager, cl); } } @@ -714,7 +702,7 @@ bool qd_connection_manager_delete_ssl_profile(qd_dispatch_t *qd, void *impl) { qd_config_ssl_profile_t *ssl_profile = (qd_config_ssl_profile_t*) impl; if (ssl_profile) - return qd_config_ssl_profile_free(qd->connection_manager, ssl_profile); + return config_ssl_profile_free(qd->connection_manager, ssl_profile); return false; } @@ -725,7 +713,7 @@ void qd_connection_manager_delete_connector(qd_dispatch_t *qd, void *impl) if (cc) { DEQ_REMOVE(qd->connection_manager->config_connectors, cc); - qd_config_connector_free(qd->connection_manager, cc); + config_connector_free(qd->connection_manager, cc); } } http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/c2a1a32d/src/server.c ---------------------------------------------------------------------- diff --git a/src/server.c b/src/server.c index 31322ea..d5ad5c4 100644 --- a/src/server.c +++ b/src/server.c @@ -76,7 +76,6 @@ struct qd_server_t { qd_work_list_t work_queue; qd_timer_list_t pending_timers; bool a_thread_is_waiting; - int threads_active; int pause_requests; int threads_paused; int pause_next_sequence; @@ -1021,7 +1020,6 @@ static void *thread_run(void *arg) if (ctx->owner_thread == CONTEXT_NO_OWNER) { ctx->owner_thread = thread->thread_id; ctx->enqueued = 0; - qd_server->threads_active++; cxtr = work->cxtr; free_qd_work_item_t(work); } else { @@ -1073,7 +1071,6 @@ static void *thread_run(void *arg) 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); } else { // @@ -1081,7 +1078,6 @@ static void *thread_run(void *arg) // sys_mutex_lock(qd_server->lock); ctx->owner_thread = CONTEXT_NO_OWNER; - qd_server->threads_active--; sys_mutex_unlock(qd_server->lock); } @@ -1344,7 +1340,6 @@ qd_server_t *qd_server(qd_dispatch_t *qd, int thread_count, const char *containe DEQ_INIT(qd_server->work_queue); DEQ_INIT(qd_server->pending_timers); qd_server->a_thread_is_waiting = false; - qd_server->threads_active = 0; qd_server->pause_requests = 0; qd_server->threads_paused = 0; qd_server->pause_next_sequence = 0; http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/c2a1a32d/src/server_private.h ---------------------------------------------------------------------- diff --git a/src/server_private.h b/src/server_private.h index 49e70b2..9581196 100644 --- a/src/server_private.h +++ b/src/server_private.h @@ -49,12 +49,6 @@ qd_http_listener_t *qd_listener_http(qd_listener_t *l); #define CONTEXT_UNSPECIFIED_OWNER -2 typedef enum { - QD_BIND_SUCCESSFUL, // Bind to socket was attempted and the bind succeeded - QD_BIND_FAILED, // Bind to socket was attempted and bind failed - QD_BIND_NONE, // Bind to socket not attempted yet -} qd_bind_state_t; - -typedef enum { CXTR_STATE_CONNECTING = 0, CXTR_STATE_OPEN, CXTR_STATE_FAILED http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/c2a1a32d/tests/system_tests_management.py ---------------------------------------------------------------------- diff --git a/tests/system_tests_management.py b/tests/system_tests_management.py index 19f69fc..b9535a8 100644 --- a/tests/system_tests_management.py +++ b/tests/system_tests_management.py @@ -20,13 +20,11 @@ """System tests for management of qdrouter""" import unittest, system_test, re, os, json, sys -from qpid_dispatch.management.client import Node, ManagementError, Url, BadRequestStatus, NotImplementedStatus, NotFoundStatus, ForbiddenStatus +from qpid_dispatch.management.client import Node, ManagementError, Url, BadRequestStatus, NotImplementedStatus, NotFoundStatus from qpid_dispatch_internal.management.qdrouter import QdSchema -from qpid_dispatch_internal.compat import OrderedDict, dictify -from system_test import Qdrouterd, message, retry, retry_exception, wait_ports, Process -from proton import ConnectionException +from qpid_dispatch_internal.compat import dictify +from system_test import Qdrouterd, message, Process from itertools import chain -from time import sleep PREFIX = u'org.apache.qpid.dispatch.' MANAGEMENT = PREFIX + 'management' @@ -305,7 +303,7 @@ class ManagementTest(system_test.TestCase): self.assertEqual( {u'operation': u'callme', u'type': DUMMY, u'identity': identity, u'data': data}, dummy.call('callme', data=data)) - except TypeError, e: + except TypeError: extype, value, trace = sys.exc_info() raise extype, "data=%r: %s" % (data, value), trace @@ -373,16 +371,6 @@ class ManagementTest(system_test.TestCase): """Test that we can access management info of remote nodes using get_mgmt_nodes addresses""" nodes = [self.cleanup(Node.connect(Url(r.addresses[0]))) for r in self.routers] remotes = sum([n.get_mgmt_nodes() for n in nodes], []) - self.assertEqual([u'amqp:/_topo/0/router2/$management', u'amqp:/_topo/0/router1/$management'], remotes) - # Query router2 indirectly via router1 - remote_url = Url(self.routers[0].addresses[0], path=Url(remotes[0]).path) - remote = self.cleanup(Node.connect(remote_url)) - self.assertEqual(["router2"], [r.id for r in remote.query(type=ROUTER).get_entities()]) - - def test_remote_node(self): - """Test that we can access management info of remote nodes using get_mgmt_nodes addresses""" - nodes = [self.cleanup(Node.connect(Url(r.addresses[0]))) for r in self.routers] - remotes = sum([n.get_mgmt_nodes() for n in nodes], []) self.assertEqual(set([u'amqp:/_topo/0/router%s/$management' % i for i in [0, 1, 2]]), set(remotes)) self.assertEqual(9, len(remotes)) @@ -398,10 +386,6 @@ class ManagementTest(system_test.TestCase): self.assertIn(CONFIGURATION, types[LISTENER]) self.assertIn(OPERATIONAL, types[LINK]) - def test_get_attributes(self): - types = self.node.get_attributes() - self.assertIn(SSL_PROFILE, types[CONNECTOR]) - def test_get_operations(self): result = self.node.get_operations(type=DUMMY) self.assertEqual({DUMMY: ["CREATE", "READ", "UPDATE", "DELETE", "CALLME"]}, result) http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/c2a1a32d/tests/system_tests_qdmanage.py ---------------------------------------------------------------------- diff --git a/tests/system_tests_qdmanage.py b/tests/system_tests_qdmanage.py index 585fc4b..bdbd823 100644 --- a/tests/system_tests_qdmanage.py +++ b/tests/system_tests_qdmanage.py @@ -21,7 +21,7 @@ import json, unittest, os from system_test import TestCase, Process, Qdrouterd, main_module, TIMEOUT, DIR, wait_port from subprocess import PIPE, STDOUT -from qpid_dispatch_internal.compat import OrderedDict, dictify +from qpid_dispatch_internal.compat import dictify from qpid_dispatch_internal.management.qdrouter import QdSchema DUMMY = "org.apache.qpid.dispatch.dummy" @@ -215,7 +215,6 @@ class QdmanageTest(TestCase): self.assertEquals ( good_logs, len(logs) ) - def test_update(self): exception = False try: --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
