*-login: Fixed dropping oldest connection when reaching all limits.

--HG--
branch : HEAD

diff --git a/src/lib-master/master-auth.c b/src/lib-master/master-auth.c
index fbee1f781dc9b49676f043c878c6133c58635ea9..4b5dbc5fa85d954d00e0eba74a1bdc91cf4b23a6 100644 (file)
--- a/src/lib-master/master-auth.c
+++ b/src/lib-master/master-auth.c
@@ -103,11 +103,29 @@ void master_auth_request_abort(struct master_service *service, unsigned int tag)
        auth->free_nodes = node;
 }
 
+static void
+master_notify_have_more_avail_processes(struct master_service *service,
+                                       bool have_more)
+{
+       if (!have_more) {
+               /* make sure we're listening for more connections */
+               master_service_io_listeners_add(service);
+       }
+       service->call_avail_overflow = !have_more;
+}
+
 static void request_handle(struct master_auth *auth,
                           struct master_auth_reply *reply)
 {
         struct master_auth_request_node *node;
 
+       if (reply->tag == 0) {
+               /* notification from master */
+               master_notify_have_more_avail_processes(auth->service,
+                                                       reply->status == 0);
+               return;
+       }
+
        node = hash_table_lookup(auth->requests, POINTER_CAST(reply->tag));
        if (node == NULL)
                i_error("Master sent reply with unknown tag %u", reply->tag);

diff --git a/src/lib-master/master-interface.h b/src/lib-master/master-interface.h
index 03bceade55a603f0eabcae70a7bba0a1cde3dbba..6597806b548d909be5840c14f98a11cdf7dd90d7 100644 (file)
--- a/src/lib-master/master-interface.h
+++ b/src/lib-master/master-interface.h
@@ -64,6 +64,7 @@ enum master_auth_status {
 };
 
 struct master_auth_reply {
+       /* tag=0 are notifications from master */
        unsigned int tag;
        enum master_auth_status status;
        /* PID of the post-login mail process handling this connection */

diff --git a/src/lib-master/master-service-private.h b/src/lib-master/master-service-private.h
index ecedc20e77d73cc128da06d7bc90e82675927075..d8a3dbe66209e21c45266da560e82af4a9ab645e 100644 (file)
--- a/src/lib-master/master-service-private.h
+++ b/src/lib-master/master-service-private.h
@@ -34,6 +34,8 @@ struct master_service {
        unsigned int total_available_count;
        struct master_status master_status;
 
+       void (*avail_overflow_callback)(void);
+
         struct master_auth *auth;
        master_service_connection_callback_t *callback;
 
@@ -46,6 +48,9 @@ struct master_service {
        unsigned int initial_status_sent:1;
        unsigned int default_settings:1;
        unsigned int die_with_master:1;
+       unsigned int call_avail_overflow:1;
 };
 
+void master_service_io_listeners_add(struct master_service *service);
+
 #endif

diff --git a/src/lib-master/master-service.c b/src/lib-master/master-service.c
index 6018d03b254657f70856ebe706253b0303300687..a31c099173c11ae01d9b1c93942c1045a5b96664 100644 (file)
--- a/src/lib-master/master-service.c
+++ b/src/lib-master/master-service.c
@@ -28,7 +28,6 @@
 
 struct master_service *master_service;
 
-static void io_listeners_add(struct master_service *service);
 static void io_listeners_remove(struct master_service *service);
 static void master_status_update(struct master_service *service);
 
@@ -294,7 +293,7 @@ void master_service_init_finish(struct master_service *service)
                master_service_set_service_count(service, 1);
        }
 
-       io_listeners_add(service);
+       master_service_io_listeners_add(service);
 
        if ((service->flags & MASTER_SERVICE_FLAG_STD_CLIENT) != 0) {
                /* we already have a connection to be served */
@@ -380,6 +379,12 @@ unsigned int master_service_get_socket_count(struct master_service *service)
        return service->socket_count;
 }
 
+void master_service_set_avail_overflow_callback(struct master_service *service,
+                                               void (*callback)(void))
+{
+       service->avail_overflow_callback = callback;
+}
+
 const char *master_service_get_config_path(struct master_service *service)
 {
        return service->config_path;
@@ -433,7 +438,7 @@ void master_service_anvil_send(struct master_service *service, const char *cmd)
 void master_service_client_connection_destroyed(struct master_service *service)
 {
        /* we can listen again */
-       io_listeners_add(service);
+       master_service_io_listeners_add(service);
 
        i_assert(service->total_available_count > 0);
 
@@ -497,12 +502,20 @@ void master_service_deinit(struct master_service **_service)
 
 static void master_service_listen(struct master_service_listener *l)
 {
+       struct master_service *service = l->service;
        struct master_service_connection conn;
 
-       if (l->service->master_status.available_count == 0) {
-               /* we are full. stop listening for now. */
-               io_listeners_remove(l->service);
-               return;
+       if (service->master_status.available_count == 0) {
+               /* we are full. stop listening for now, unless overflow
+                  callback destroys one of the existing connections */
+               if (service->call_avail_overflow &&
+                   service->avail_overflow_callback != NULL)
+                       service->avail_overflow_callback();
+
+               if (service->master_status.available_count == 0) {
+                       io_listeners_remove(service);
+                       return;
+               }
        }
 
        memset(&conn, 0, sizeof(conn));
@@ -514,7 +527,7 @@ static void master_service_listen(struct master_service_listener *l)
 
                if (errno != ENOTSOCK) {
                        i_error("net_accept() failed: %m");
-                       master_service_error(l->service);
+                       master_service_error(service);
                        return;
                }
                /* it's not a socket. probably a fifo. use the "listener"
@@ -529,10 +542,10 @@ static void master_service_listen(struct master_service_listener *l)
        conn.ssl = l->ssl;
        net_set_nonblock(conn.fd, TRUE);
 
-       l->service->master_status.available_count--;
-        master_status_update(l->service);
+       service->master_status.available_count--;
+        master_status_update(service);
 
-        l->service->callback(&conn);
+        service->callback(&conn);
 }
 
 static void io_listeners_init(struct master_service *service)
@@ -556,7 +569,7 @@ static void io_listeners_init(struct master_service *service)
        }
 }
 
-static void io_listeners_add(struct master_service *service)
+void master_service_io_listeners_add(struct master_service *service)
 {
        unsigned int i;
 

diff --git a/src/lib-master/master-service.h b/src/lib-master/master-service.h
index 7a88bf223a80c2d409327822ea66f982770f1530..29935f9592b56a621564bca2f736643a7d4203fc 100644 (file)
--- a/src/lib-master/master-service.h
+++ b/src/lib-master/master-service.h
@@ -56,6 +56,11 @@ void master_service_init_log(struct master_service *service,
    Normally all existing clients are handled first. */
 void master_service_set_die_with_master(struct master_service *service,
                                        bool set);
+/* Call the given callback when there are no available connections and master
+   has indicated that it can't create any more processes to handle requests.
+   The callback could decide to kill one of the existing connections. */
+void master_service_set_avail_overflow_callback(struct master_service *service,
+                                               void (*callback)(void));
 
 /* Set maximum number of clients we can handle. Default is given by master. */
 void master_service_set_client_limit(struct master_service *service,

diff --git a/src/login-common/client-common.c b/src/login-common/client-common.c
index 36011a88182072e085c7ee1b7c99da724e96bc84..9fe3bed6b97c2296763070099ce7cfc148abcd5c 100644 (file)
--- a/src/login-common/client-common.c
+++ b/src/login-common/client-common.c
@@ -19,12 +19,7 @@
 
 #include <stdlib.h>
 
-/* When max. number of simultaneous connections is reached, few of the
-   oldest connections are disconnected. Since we have to go through all of the
-   clients, it's faster if we disconnect multiple clients. */
-#define CLIENT_DESTROY_OLDEST_COUNT 16
-
-struct client *clients = NULL;
+struct client *clients = NULL, *last_client = NULL;
 static unsigned int clients_count = 0;
 
 static void client_idle_disconnect_timeout(struct client *client)
@@ -51,12 +46,6 @@ struct client *client_create(int fd, bool ssl, pool_t pool,
 
        i_assert(fd != -1);
 
-       if (clients_get_count() >= set->login_max_connections) {
-               /* reached max. users count, kill few of the
-                  oldest connections */
-               client_destroy_oldest();
-       }
-
        /* always use nonblocking I/O */
        net_set_nonblock(fd, TRUE);
 
@@ -80,6 +69,8 @@ struct client *client_create(int fd, bool ssl, pool_t pool,
        client->secured = ssl || client->trusted ||
                net_ip_compare(remote_ip, local_ip);
 
+       if (last_client == NULL)
+               last_client = client;
        DLLIST_PREPEND(&clients, client);
        clients_count++;
 
@@ -114,6 +105,10 @@ void client_destroy(struct client *client, const char *reason)
 
        i_assert(clients_count > 0);
        clients_count--;
+       if (last_client == client) {
+               i_assert(client->prev != NULL || clients_count == 0);
+               last_client = client->prev;
+       }
        DLLIST_REMOVE(&clients, client);
 
        if (client->input != NULL)
@@ -213,39 +208,24 @@ bool client_unref(struct client *client)
 
 void client_destroy_oldest(void)
 {
-       unsigned int max_connections =
-               global_login_settings->login_max_connections;
        struct client *client;
-       struct client *destroy_buf[CLIENT_DESTROY_OLDEST_COUNT];
-       unsigned int i, destroy_count;
-
-       /* find the oldest clients and put them to destroy-buffer */
-       memset(destroy_buf, 0, sizeof(destroy_buf));
-
-       destroy_count = max_connections > CLIENT_DESTROY_OLDEST_COUNT*2 ?
-               CLIENT_DESTROY_OLDEST_COUNT : I_MIN(max_connections/2, 1);
-       for (client = clients; client != NULL; client = client->next) {
-               for (i = 0; i < destroy_count; i++) {
-                       if (destroy_buf[i] == NULL ||
-                           destroy_buf[i]->created > client->created) {
-                               /* @UNSAFE */
-                               memmove(destroy_buf+i+1, destroy_buf+i,
-                                       sizeof(destroy_buf) -
-                                       (i+1) * sizeof(destroy_buf[0]));
-                               destroy_buf[i] = client;
-                               break;
-                       }
-               }
+
+       if (last_client == NULL) {
+               /* we have no clients */
+               return;
        }
 
-       /* then kill them */
-       for (i = 0; i < destroy_count; i++) {
-               if (destroy_buf[i] == NULL)
+       /* destroy the last client that hasn't successfully authenticated yet.
+          this is usually the last client, but don't kill it if it's just
+          waiting for master to finish its job. */
+       for (client = last_client; client != NULL; client = client->prev) {
+               if (client->master_tag == 0)
                        break;
-
-               client_destroy(destroy_buf[i],
-                              "Disconnected: Connection queue full");
        }
+       if (client == NULL)
+               client = last_client;
+
+       client_destroy(client, "Disconnected: Connection queue full");
 }
 
 void clients_destroy_all(void)

diff --git a/src/login-common/login-settings.c b/src/login-common/login-settings.c
index f7b638e671d9fddfaaabfd0494c678d44d640a18..1be6b05811fdcc05bff0075dfa3192b119ae384c 100644 (file)
--- a/src/login-common/login-settings.c
+++ b/src/login-common/login-settings.c
@@ -39,7 +39,6 @@ static struct setting_define login_setting_defines[] = {
        DEF(SET_BOOL, auth_debug),
        DEF(SET_BOOL, verbose_proctitle),
 
-       DEF(SET_UINT, login_max_connections),
        DEF(SET_UINT, mail_max_userip_connections),
 
        SETTING_DEFINE_LIST_END
@@ -70,7 +69,6 @@ static struct login_settings login_default_settings = {
        MEMBER(auth_debug) FALSE,
        MEMBER(verbose_proctitle) FALSE,
 
-       MEMBER(login_max_connections) 256,
        MEMBER(mail_max_userip_connections) 10
 };
 
@@ -132,10 +130,6 @@ static bool login_settings_check(void *_set, pool_t pool, const char **error_r)
                /* if we require valid cert, make sure we also ask for it */
                set->ssl_verify_client_cert = TRUE;
        }
-       if (set->login_max_connections < 1) {
-               *error_r = "login_max_connections must be at least 1";
-               return FALSE;
-       }
 
        if (strcmp(set->ssl, "no") == 0) {
                /* disabled */

diff --git a/src/login-common/login-settings.h b/src/login-common/login-settings.h
index b1b85236592d2d5ecffd46e078bb07c9823964cf..baa2327836cab93ffd926d3fb0a7d043e7f91e7f 100644 (file)
--- a/src/login-common/login-settings.h
+++ b/src/login-common/login-settings.h
@@ -27,7 +27,6 @@ struct login_settings {
        bool auth_debug;
        bool verbose_proctitle;
 
-       unsigned int login_max_connections;
        unsigned int mail_max_userip_connections;
 
        /* generated: */

diff --git a/src/login-common/main.c b/src/login-common/main.c
index 85972110fb151728cb9da8fed529262fd4e7a120..d308d4e8addeeb39348f2a081903e0851ef6fae4 100644 (file)
--- a/src/login-common/main.c
+++ b/src/login-common/main.c
@@ -101,10 +101,17 @@ static void main_preinit(void)
 
        /* set the number of fds we want to use. it may get increased or
           decreased. leave a couple of extra fds for auth sockets and such.
-          normal connections each use one fd, but SSL connections use two */
+
+          worst case each connection can use:
+
+           - 1 for client
+           - 1 for login proxy
+           - 2 for client-side ssl proxy
+           - 2 for server-side ssl proxy (with login proxy)
+       */
        max_fds = MASTER_LISTEN_FD_FIRST + 16 +
                master_service_get_socket_count(master_service) +
-               global_login_settings->login_max_connections*2;
+               master_service_get_client_limit(master_service)*6;
        restrict_fd_limit(max_fds);
        io_loop_set_max_fd_count(current_ioloop, max_fds);
 
@@ -127,6 +134,9 @@ static void main_init(void)
                        i_fatal("chdir(login) failed: %m");
        }
 
+       master_service_set_avail_overflow_callback(master_service,
+                                                  client_destroy_oldest);
+
        auth_client = auth_client_new((unsigned int)getpid());
         auth_client_set_connect_notify(auth_client, auth_connect_notify, NULL);
 

diff --git a/src/master/service-auth-source.c b/src/master/service-auth-source.c
index e9da6197347cbee28fcc822626ac751fb805e9e9..17c53f43551dcd53a3e761d21bb70d4532a4ffe8 100644 (file)
--- a/src/master/service-auth-source.c
+++ b/src/master/service-auth-source.c
@@ -271,3 +271,32 @@ void service_process_auth_source_deinit(struct service_process *_process)
 
        service_process_auth_source_close(process);
 }
+
+void service_processes_auth_source_notify(struct service *service,
+                                         bool all_processes_created)
+{
+       struct hash_iterate_context *iter;
+       void *key, *value;
+       enum master_auth_status status;
+
+       i_assert(service->type == SERVICE_TYPE_AUTH_SOURCE);
+
+       status = all_processes_created ? 1 : 0;
+
+       iter = hash_table_iterate_init(service_pids);
+       while (hash_table_iterate(iter, &key, &value)) {
+               struct service_process *process = value;
+               struct service_process_auth_source *auth_process;
+
+               if (process->service != service)
+                       continue;
+
+               auth_process = (struct service_process_auth_source *)process;
+               if (auth_process->last_notify_status != (int)status) {
+                       auth_process->last_notify_status = (int)status;
+                       service_process_auth_source_send_reply(auth_process,
+                                                              0, status);
+               }
+       }
+       hash_table_iterate_deinit(&iter);
+}

diff --git a/src/master/service-auth-source.h b/src/master/service-auth-source.h
index dee27a27dff591189d7f7570f603f405166abfda..273e71f4580b0dc72cad2637a2e5ad9725355f7d 100644 (file)
--- a/src/master/service-auth-source.h
+++ b/src/master/service-auth-source.h
@@ -11,4 +11,7 @@ void service_process_auth_source_send_reply(struct service_process_auth_source *
                                            unsigned int tag,
                                            enum master_auth_status status);
 
+void service_processes_auth_source_notify(struct service *service,
+                                         bool all_processes_created);
+
 #endif

diff --git a/src/master/service-monitor.c b/src/master/service-monitor.c
index dd10b33370d369ed1c43cc27a70c17b5f9ce47f8..d8db2105ec43854af48dbe5c8e84f94e6db69a46 100644 (file)
--- a/src/master/service-monitor.c
+++ b/src/master/service-monitor.c
@@ -6,6 +6,7 @@
 #include "fd-close-on-exec.h"
 #include "hash.h"
 #include "service.h"
+#include "service-auth-source.h"
 #include "service-process.h"
 #include "service-process-notify.h"
 #include "service-log.h"
@@ -18,6 +19,7 @@
 
 #define SERVICE_PROCESS_KILL_IDLE_MSECS (1000*60)
 #define SERVICE_STARTUP_FAILURE_THROTTLE_SECS 60
+#define SERVICE_DROP_WARN_INTERVAL_SECS 60
 
 static void service_monitor_start_extra_avail(struct service *service);
 
@@ -162,6 +164,26 @@ static void service_monitor_throttle(struct service *service)
        service_throttle(service, SERVICE_STARTUP_FAILURE_THROTTLE_SECS);
 }
 
+static void service_drop_connections(struct service *service)
+{
+       if (service->last_drop_warning +
+           SERVICE_DROP_WARN_INTERVAL_SECS < ioloop_time) {
+               service->last_drop_warning = ioloop_time;
+               i_warning("service(%s): process_limit reached, "
+                         "client connections are being dropped",
+                         service->set->name);
+       }
+       service->listen_pending = TRUE;
+       service_monitor_listen_stop(service);
+
+       if (service->type == SERVICE_TYPE_AUTH_SOURCE) {
+               /* reached process limit, notify processes that they
+                  need to start killing existing connections if they
+                  reach connection limit */
+               service_processes_auth_source_notify(service, TRUE);
+       }
+}
+
 static void service_accept(struct service *service)
 {
        i_assert(service->process_avail == 0);
@@ -169,11 +191,7 @@ static void service_accept(struct service *service)
        if (service->process_count == service->process_limit) {
                /* we've reached our limits, new clients will have to
                   wait until there are more processes available */
-               i_warning("service(%s): process_limit reached, "
-                         "client connections are being dropped",
-                         service->set->name);
-               service->listen_pending = TRUE;
-                service_monitor_listen_stop(service);
+               service_drop_connections(service);
                return;
        }
 
@@ -212,7 +230,9 @@ void service_monitor_listen_start(struct service *service)
        struct service_listener *const *listeners;
        unsigned int i, count;
 
-       if (service->process_avail > 0)
+       if (service->process_avail > 0 ||
+           (service->process_count == service->process_limit &&
+            service->listen_pending))
                return;
 
        service->listening = TRUE;

diff --git a/src/master/service-process.c b/src/master/service-process.c
index 9c890cce548de3a2b470618f5442af1b18500cbb..97a28ae469cfb220c79d199adea77740bfba379c 100644 (file)
--- a/src/master/service-process.c
+++ b/src/master/service-process.c
@@ -484,7 +484,7 @@ service_process_create(struct service *service, const char *const *auth_args,
        if (service->process_count >= service->process_limit) {
                /* we should get here only with auth dest services */
                i_warning("service(%s): process_limit reached, "
-                         "client connections are being dropped",
+                         "dropping this client connection",
                          service->set->name);
                return NULL;
        }
@@ -594,6 +594,11 @@ void service_process_destroy(struct service_process *process)
 
        process->destroyed = TRUE;
        service_process_unref(process);
+
+       if (service->process_count < service->process_limit &&
+           service->type == SERVICE_TYPE_AUTH_SOURCE)
+               service_processes_auth_source_notify(service, FALSE);
+
        service_list_unref(service_list);
 }
 

diff --git a/src/master/service-process.h b/src/master/service-process.h
index 0c5270082a654f363e4bfb3e9d193a8f0b2a0ee8..102c0b34fbc00931fd3633c2d9fa33f413bb45ad 100644 (file)
--- a/src/master/service-process.h
+++ b/src/master/service-process.h
@@ -48,6 +48,8 @@ struct service_process_auth_server {
 struct service_process_auth_source {
        struct service_process process;
 
+       int last_notify_status;
+
        int auth_fd;
        struct io *io_auth;
        struct ostream *auth_output;

diff --git a/src/master/service.h b/src/master/service.h
index b1a30cbafbe80e58c717f552880d7b1233ed4dd3..a755a5862d750bfd0d3b58ceb54e6b48a3bbc967 100644 (file)
--- a/src/master/service.h
+++ b/src/master/service.h
@@ -89,6 +89,9 @@ struct service {
           successful authentication. */
        struct service *auth_dest_service;
 
+       /* Last time a "dropping client connections" warning was logged */
+       time_t last_drop_warning;
+
        /* all processes are in use and new connections are coming */
        unsigned int listen_pending:1;
        /* service is currently listening for new connections */