MXS-2196: Replace listener iterator

The iteration of listeners is now done via the global list of
listeners. This removes the need to have a service before a listener is
accessed which also reflects how the actual configuration is laid out. It
also guarantees that any results returned by the find functions will be
valid as long as the results are used.

include/maxscale/listener.hh

@@ -16,6 +16,7 @@
 
 #include <string>
 #include <memory>
+#include <vector>
 
 #include <maxbase/jansson.h>
 #include <maxscale/protocol.h>
@@ -56,11 +57,6 @@ public:
 
 using SListener = std::shared_ptr<SERV_LISTENER>;
 
-typedef struct listener_iterator
-{
-    SERV_LISTENER* current;
-} LISTENER_ITERATOR;
-
 /**
  * @brief Serialize a listener to a file
  *
@@ -142,6 +138,24 @@ bool listener_stop(SERV_LISTENER* listener);
  */
 bool listener_start(SERV_LISTENER* listener);
 
+/**
+ * Find a listener
+ *
+ * @param name Name of the listener
+ *
+ * @return The listener if it exists or an empty SListener if it doesn't
+ */
+SListener listener_find(const std::string& name);
+
+/**
+ * Find all listeners that point to a service
+ *
+ * @param service Service whose listeners are returned
+ *
+ * @return The listeners that point to the service
+ */
+std::vector<SListener> listener_find_by_service(const SERVICE* service);
+
 int  listener_set_ssl_version(SSL_LISTENER* ssl_listener, const char* version);
 void listener_set_certificates(SSL_LISTENER* ssl_listener, char* cert, char* key, char* ca_cert);
 
@@ -187,25 +201,6 @@ bool listener_is_active(SERV_LISTENER* listener);
  */
 void listener_set_active(SERV_LISTENER* listener, bool active);
 
-/**
- * @brief Initialize a listener iterator for iterating service listeners
- *
- * @param service Service whose listeners are iterated
- * @param iter    Pointer to iterator to initialize
- *
- * @return The first value pointed by the iterator
- */
-SERV_LISTENER* listener_iterator_init(const SERVICE* service, LISTENER_ITERATOR* iter);
-
-/**
- * @brief Get the next listener
- *
- * @param iter Listener iterator
- *
- * @return The next listener or NULL on end of list
- */
-SERV_LISTENER* listener_iterator_next(LISTENER_ITERATOR* iter);
-
 /**
  * Get listener state as a string
  *

server/core/listener.cc

@@ -23,7 +23,6 @@
 #include <memory>
 #include <mutex>
 #include <string>
-#include <vector>
 
 #include <maxscale/paths.h>
 #include <maxscale/ssl.h>
@@ -164,6 +163,39 @@ bool listener_start(SERV_LISTENER* listener)
     return rval;
 }
 
+SListener listener_find(const std::string& name)
+{
+    SListener rval;
+    std::lock_guard<std::mutex> guard(listener_lock);
+
+    for (const auto& a : all_listeners)
+    {
+        if (listener_is_active(a) && a->name == name)
+        {
+            rval = a;
+            break;
+        }
+    }
+
+    return rval;
+}
+
+std::vector<SListener> listener_find_by_service(const SERVICE* service)
+{
+    std::vector<SListener> rval;
+    std::lock_guard<std::mutex> guard(listener_lock);
+
+    for (const auto& a : all_listeners)
+    {
+        if (listener_is_active(a) && a->service == service)
+        {
+            rval.push_back(a);
+        }
+    }
+
+    return rval;
+}
+
 /**
  * Set the maximum SSL/TLS version the listener will support
  * @param ssl_listener Listener data to configure
@@ -594,30 +626,6 @@ bool listener_is_active(SERV_LISTENER* listener)
     return atomic_load_int32(&listener->active);
 }
 
-static inline SERV_LISTENER* load_port(SERV_LISTENER const* const* const port)
-{
-    return (SERV_LISTENER*)atomic_load_ptr((void**)port);
-}
-
-SERV_LISTENER* listener_iterator_init(const SERVICE* service, LISTENER_ITERATOR* iter)
-{
-    mxb_assert(iter);
-    iter->current = load_port(&service->ports);
-    return iter->current;
-}
-
-SERV_LISTENER* listener_iterator_next(LISTENER_ITERATOR* iter)
-{
-    mxb_assert(iter);
-
-    if (iter->current)
-    {
-        iter->current = load_port(&iter->current->next);
-    }
-
-    return iter->current;
-}
-
 const char* listener_state_to_string(const SERV_LISTENER* listener)
 {
     mxb_assert(listener);

server/core/service.cc

@@ -587,46 +587,14 @@ bool serviceLaunchListener(Service* service, SERV_LISTENER* port)
 
 bool serviceStopListener(SERVICE* svc, const char* name)
 {
-    Service* service = static_cast<Service*>(svc);
+    auto listener = listener_find(name);
-    bool rval = false;
+    return listener && listener->service == svc && listener_stop(listener);
-    LISTENER_ITERATOR iter;
-
-    for (SERV_LISTENER* listener = listener_iterator_init(service, &iter);
-         listener; listener = listener_iterator_next(&iter))
-    {
-        if (listener_is_active(listener) && listener->name == name)
-        {
-            if (listener_stop(listener))
-            {
-                rval = true;
-            }
-            break;
-        }
-    }
-
-    return rval;
 }
 
 bool serviceStartListener(SERVICE* svc, const char* name)
 {
-    Service* service = static_cast<Service*>(svc);
+    auto listener = listener_find(name);
-    bool rval = false;
+    return listener && listener->service == svc && listener_start(listener);
-    LISTENER_ITERATOR iter;
-
-    for (SERV_LISTENER* listener = listener_iterator_init(service, &iter);
-         listener; listener = listener_iterator_next(&iter))
-    {
-        if (listener_is_active(listener) && listener->name == name)
-        {
-            if (listener_start(listener))
-            {
-                rval = true;
-            }
-            break;
-        }
-    }
-
-    return rval;
 }
 
 int service_launch_all()
@@ -664,12 +632,9 @@ bool serviceStop(SERVICE* service)
 
     if (service)
     {
-        LISTENER_ITERATOR iter;
+        for (const auto& listener : listener_find_by_service(service))
-
-        for (SERV_LISTENER* listener = listener_iterator_init(service, &iter);
-             listener; listener = listener_iterator_next(&iter))
         {
-            if (listener_is_active(listener) && listener_stop(listener))
+            if (listener_stop(listener))
             {
                 listeners++;
             }
@@ -695,12 +660,9 @@ bool serviceStart(SERVICE* service)
 
     if (service)
     {
-        LISTENER_ITERATOR iter;
+        for (const auto& listener : listener_find_by_service(service))
-
-        for (SERV_LISTENER* listener = listener_iterator_init(service, &iter);
-             listener; listener = listener_iterator_next(&iter))
         {
-            if (listener_is_active(listener) && listener_start(listener))
+            if (listener_start(listener))
             {
                 listeners++;
             }
@@ -734,17 +696,12 @@ static void service_add_listener(SERVICE* service, SERV_LISTENER* proto)
 bool service_remove_listener(Service* service, const char* target)
 {
     bool rval = false;
-    LISTENER_ITERATOR iter;
+    auto listener = listener_find(target);
 
-    for (SERV_LISTENER* listener = listener_iterator_init(service, &iter);
+    if (listener && listener->service == service)
-         listener; listener = listener_iterator_next(&iter))
     {
-        if (listener_is_active(listener) && listener->name == target)
+        listener_destroy(listener);
-        {
+        rval = true;
-            listener_destroy(listener);
-            rval = true;
-            break;
-        }
     }
 
     return rval;
@@ -793,12 +750,9 @@ SERV_LISTENER* service_find_listener(Service* service,
                                      const char* address,
                                      unsigned short port)
 {
-    LISTENER_ITERATOR iter;
+    for (const auto& listener : listener_find_by_service(service))
-
-    for (SERV_LISTENER* listener = listener_iterator_init(service, &iter);
-         listener; listener = listener_iterator_next(&iter))
     {
-        if (listener_is_active(listener) && port == listener->port)
+        if (port == listener->port)
         {
             if ((!address && listener->address.empty()) || listener->address == address
                 || (!socket && listener->address.empty()) || listener->address == socket)
@@ -826,12 +780,9 @@ bool serviceHasListener(Service* service,
                         const char* address,
                         unsigned short port)
 {
-    LISTENER_ITERATOR iter;
+    for (const auto& listener : listener_find_by_service(service))
-
-    for (SERV_LISTENER* listener = listener_iterator_init(service, &iter);
-         listener; listener = listener_iterator_next(&iter))
     {
-        if (listener_is_active(listener) && listener->port == port)
+        if (listener->port == port)
         {
             if ((!address && listener->address.empty()) || listener->address == address)
             {
@@ -845,34 +796,13 @@ bool serviceHasListener(Service* service,
 
 bool service_has_named_listener(Service* service, const char* name)
 {
-    LISTENER_ITERATOR iter;
+    auto listener = listener_find(name);
-
+    return listener && listener->service == service;
-    for (SERV_LISTENER* listener = listener_iterator_init(service, &iter);
-         listener; listener = listener_iterator_next(&iter))
-    {
-        if (listener_is_active(listener) && listener->name == name)
-        {
-            return true;
-        }
-    }
-
-    return false;
 }
 
 bool service_can_be_destroyed(Service* service)
 {
-    bool rval = true;
+    bool rval = listener_find_by_service(service).empty();
-    LISTENER_ITERATOR iter;
-
-    for (SERV_LISTENER* listener = listener_iterator_init(service, &iter);
-         listener; listener = listener_iterator_next(&iter))
-    {
-        if (listener_is_active(listener))
-        {
-            rval = false;
-            break;
-        }
-    }
 
     if (rval)
     {
@@ -1350,22 +1280,16 @@ void dListListeners(DCB* dcb)
     }
     for (Service* service : this_unit.services)
     {
-        LISTENER_ITERATOR iter;
+        for (const auto& listener : listener_find_by_service(service))
-
-        for (SERV_LISTENER* listener = listener_iterator_init(service, &iter);
-             listener; listener = listener_iterator_next(&iter))
         {
-            if (listener_is_active(listener))
+            dcb_printf(dcb,
-            {
+                       "%-20s | %-19s | %-18s | %-15s | %5d | %s\n",
-                dcb_printf(dcb,
+                       listener->name.c_str(),
-                           "%-20s | %-19s | %-18s | %-15s | %5d | %s\n",
+                       service->name,
-                           listener->name.c_str(),
+                       listener->protocol.c_str(),
-                           service->name,
+                       (listener && !listener->address.empty()) ? listener->address.c_str() : "*",
-                           listener->protocol.c_str(),
+                       listener->port,
-                           (listener && !listener->address.empty()) ? listener->address.c_str() : "*",
+                       listener_state_to_string(listener));
-                           listener->port,
-                           listener_state_to_string(listener));
-            }
         }
     }
     if (!this_unit.services.empty())
@@ -1414,16 +1338,12 @@ bool Service::refresh_users()
         m_rate_limits[self].last = now;
         m_rate_limits[self].warned = false;
 
-        LISTENER_ITERATOR iter;
+        for (const auto& listener : listener_find_by_service(this))
-
-        for (SERV_LISTENER* listener = listener_iterator_init(this, &iter);
-             listener; listener = listener_iterator_next(&iter))
         {
             /** Load the authentication users before before starting the listener */
-            if (listener_is_active(listener) && listener->listener
+            if (listener->listener && listener->listener->authfunc.loadusers)
-                && listener->listener->authfunc.loadusers)
             {
-                switch (listener->listener->authfunc.loadusers(listener))
+                switch (listener->listener->authfunc.loadusers(listener.get()))
                 {
                 case MXS_AUTH_LOADUSERS_FATAL:
                     MXS_ERROR("[%s] Fatal error when loading users for listener '%s',"
@@ -1586,13 +1506,10 @@ std::unique_ptr<ResultSet> serviceGetListenerList()
 
     for (Service* service : this_unit.services)
     {
-        LISTENER_ITERATOR iter;
+        for (const auto& listener : listener_find_by_service(service))
-
-        for (SERV_LISTENER* lptr = listener_iterator_init(service, &iter);
-             lptr; lptr = listener_iterator_next(&iter))
         {
-            set->add_row({service->name, lptr->protocol, lptr->address,
+            set->add_row({service->name, listener->protocol, listener->address,
-                          std::to_string(lptr->port), listener_state_to_string(lptr)});
+                          std::to_string(listener->port), listener_state_to_string(listener)});
         }
     }
 
@@ -1641,10 +1558,7 @@ bool service_all_services_have_listeners()
 
     for (Service* service : this_unit.services)
     {
-        LISTENER_ITERATOR iter;
+        if (listener_find_by_service(service).empty())
-        SERV_LISTENER* listener = listener_iterator_init(service, &iter);
-
-        if (listener == NULL)
         {
             MXS_ERROR("Service '%s' has no listeners.", service->name);
             rval = false;
@@ -1886,17 +1800,13 @@ bool service_serialize(const Service* service)
 
 void service_print_users(DCB* dcb, const SERVICE* service)
 {
-    LISTENER_ITERATOR iter;
+    for (const auto& listener : listener_find_by_service(service))
-
-    for (SERV_LISTENER* listener = listener_iterator_init(service, &iter);
-         listener; listener = listener_iterator_next(&iter))
     {
-        if (listener_is_active(listener) && listener->listener
+        if (listener->listener && listener->listener->authfunc.diagnostic)
-            && listener->listener->authfunc.diagnostic)
         {
             dcb_printf(dcb, "User names (%s): ", listener->name.c_str());
 
-            listener->listener->authfunc.diagnostic(dcb, listener);
+            listener->listener->authfunc.diagnostic(dcb, listener.get());
 
             dcb_printf(dcb, "\n");
         }
@@ -1910,12 +1820,9 @@ bool service_port_is_used(unsigned short port)
 
     for (Service* service : this_unit.services)
     {
-        LISTENER_ITERATOR iter;
+        for (const auto& listener : listener_find_by_service(service))
-
-        for (SERV_LISTENER* listener = listener_iterator_init(service, &iter);
-             listener; listener = listener_iterator_next(&iter))
         {
-            if (listener_is_active(listener) && listener->port == port)
+            if (listener->port == port)
             {
                 rval = true;
                 break;
@@ -1983,15 +1890,10 @@ static inline bool have_active_servers(const SERVICE* service)
 static json_t* service_all_listeners_json_data(const SERVICE* service)
 {
     json_t* arr = json_array();
-    LISTENER_ITERATOR iter;
 
-    for (SERV_LISTENER* listener = listener_iterator_init(service, &iter);
+    for (const auto& listener : listener_find_by_service(service))
-         listener; listener = listener_iterator_next(&iter))
     {
-        if (listener_is_active(listener))
+        json_array_append_new(arr, listener_to_json(listener.get()));
-        {
-            json_array_append_new(arr, listener_to_json(listener));
-        }
     }
 
     return arr;
@@ -1999,15 +1901,11 @@ static json_t* service_all_listeners_json_data(const SERVICE* service)
 
 static json_t* service_listener_json_data(const SERVICE* service, const char* name)
 {
-    LISTENER_ITERATOR iter;
+    auto listener = listener_find(name);
 
-    for (SERV_LISTENER* listener = listener_iterator_init(service, &iter);
+    if (listener && listener->service == service)
-         listener; listener = listener_iterator_next(&iter))
     {
-        if (listener_is_active(listener) && listener->name == name)
+        return listener_to_json(listener.get());
-        {
-            return listener_to_json(listener);
-        }
     }
 
     return NULL;

server/modules/protocol/MySQL/mariadb_client.cc

@@ -266,10 +266,8 @@ LocalClient* LocalClient::create(MYSQL_session* session, MySQLProtocol* proto, c
 LocalClient* LocalClient::create(MYSQL_session* session, MySQLProtocol* proto, SERVICE* service)
 {
     LocalClient* rval = NULL;
-    LISTENER_ITERATOR iter;
 
-    for (SERV_LISTENER* listener = listener_iterator_init(service, &iter);
+    for (const auto& listener : listener_find_by_service(service))
-         listener; listener = listener_iterator_next(&iter))
     {
         if (listener->port > 0)
         {