director: Don't send USERs in handshake that were already sent between handshake

If the user was refreshed since the handshake was started, it means that
the same user was already sent to the other side (added to the stream
immediately after it was received/handled). There's no need to send it
again.

This fixes a potentally infinite handshake when users are rapidly changing
and the handshake iterator never sees the end of the list. (Each refreshed
user is moved to the end of the list, so handshaking can keep sending the
same user over and over again.)

diff --git a/src/director/director-connection.c b/src/director/director-connection.c
index 9a176ea982fbdfdefba23b3ff3f87739e21e34e0..1063ec24a4918d7d81dc19b7fcf4cfafedfa6b9f 100644 (file)
--- a/src/director/director-connection.c
+++ b/src/director/director-connection.c
@@ -2255,7 +2255,16 @@ static void director_finish_sending_handshake(struct director_connection *conn)
        director_connection_send_hosts(conn);
 
        i_assert(conn->user_iter == NULL);
-       conn->user_iter = director_iterate_users_init(conn->dir);
+       /* Iterate only through users that aren't refreshed since the
+          iteration started. The refreshed users will already be sent as
+          regular USER updates, so they don't need to be sent again.
+
+          We especially don't want to send these users again, because
+          otherwise in a rapidly changing director we might never end up
+          sending all the users when they constantly keep being added to the
+          end of the list. (The iteration lists users in order from older to
+          newer.) */
+       conn->user_iter = director_iterate_users_init(conn->dir, TRUE);
 
        if (director_connection_send_users(conn) == 0)
                o_stream_set_flush_pending(conn->output, TRUE);

diff --git a/src/director/director.c b/src/director/director.c
index 30749a41d11a0df9fe35bf946693aad17e7c4cc7..9f6a6d5c2127e83815c6525b933244a257ffbc59 100644 (file)
--- a/src/director/director.c
+++ b/src/director/director.c
@@ -1520,12 +1520,15 @@ struct director_user_iter {
        struct director *dir;
        unsigned int tag_idx;
        struct user_directory_iter *user_iter;
+       bool iter_until_current_tail;
 };
 
-struct director_user_iter *director_iterate_users_init(struct director *dir)
+struct director_user_iter *
+director_iterate_users_init(struct director *dir, bool iter_until_current_tail)
 {
        struct director_user_iter *iter = i_new(struct director_user_iter, 1);
        iter->dir = dir;
+       iter->iter_until_current_tail = iter_until_current_tail;
        return iter;
 }
 
@@ -1541,7 +1544,8 @@ struct user *director_iterate_users_next(struct director_user_iter *iter)
                if (iter->tag_idx >= array_count(tags))
                        return NULL;
                struct mail_tag *const *tagp = array_idx(tags, iter->tag_idx);
-               iter->user_iter = user_directory_iter_init((*tagp)->users);
+               iter->user_iter = user_directory_iter_init((*tagp)->users,
+                       iter->iter_until_current_tail);
        }
        user = user_directory_iter_next(iter->user_iter);
        if (user == NULL) {

diff --git a/src/director/director.h b/src/director/director.h
index 2f7acd4ccb5f1ef83e3ecf10101a463d49acbece..812ac0d71750962a0be32301e14e902b47b81e21 100644 (file)
--- a/src/director/director.h
+++ b/src/director/director.h
@@ -266,7 +266,8 @@ void directors_deinit(void);
 
 void dir_debug(const char *fmt, ...) ATTR_FORMAT(1, 2);
 
-struct director_user_iter *director_iterate_users_init(struct director *dir);
+struct director_user_iter *
+director_iterate_users_init(struct director *dir, bool iter_until_current_tail);
 struct user *director_iterate_users_next(struct director_user_iter *iter);
 void director_iterate_users_deinit(struct director_user_iter **_iter);
 

diff --git a/src/director/doveadm-connection.c b/src/director/doveadm-connection.c
index a8cfe02e44949967da69f26d687d5fa5fdca0715..baa0a34f54b174420f76f81f1fb858146beb8611 100644 (file)
--- a/src/director/doveadm-connection.c
+++ b/src/director/doveadm-connection.c
@@ -522,7 +522,7 @@ director_host_reset_users(struct director_reset_cmd *cmd,
                director_connection_cork(dir->right);
 
        if (cmd->iter == NULL) {
-               cmd->iter = director_iterate_users_init(dir);
+               cmd->iter = director_iterate_users_init(dir, FALSE);
                cmd->users_killed = FALSE;
        }
 
@@ -715,7 +715,7 @@ doveadm_cmd_user_list(struct doveadm_connection *conn, const char *const *args)
                ip.family = 0;
        }
 
-       iter = director_iterate_users_init(conn->dir);
+       iter = director_iterate_users_init(conn->dir, FALSE);
        while ((user = director_iterate_users_next(iter)) != NULL) {
                if (ip.family == 0 ||
                    net_ip_compare(&ip, &user->host->ip)) T_BEGIN {

diff --git a/src/director/test-user-directory.c b/src/director/test-user-directory.c
index ec26c663681ad77f1303926a2517c611d6b9a07a..87e2eb208685c33b70dd1ad1c5fd383053959d64 100644 (file)
--- a/src/director/test-user-directory.c
+++ b/src/director/test-user-directory.c
@@ -19,7 +19,7 @@ verify_user_directory(struct user_directory *dir, unsigned int user_count)
        struct user *user, *prev = NULL;
        unsigned int prev_stamp = 0, iter_count = 0;
 
-       iter = user_directory_iter_init(dir);
+       iter = user_directory_iter_init(dir, FALSE);
        while ((user = user_directory_iter_next(iter)) != NULL) {
                test_assert(prev_stamp <= user->timestamp);
                test_assert(user->prev == prev);

diff --git a/src/director/user-directory.c b/src/director/user-directory.c
index 05b05bb27febf3b9aa856a7a98289b4cbc418b89..f0fca7f35b22c87748fd2aca0a35c47e84a7a1e8 100644 (file)
--- a/src/director/user-directory.c
+++ b/src/director/user-directory.c
@@ -17,7 +17,7 @@
 
 struct user_directory_iter {
        struct user_directory *dir;
-       struct user *pos;
+       struct user *pos, *stop_after_tail;
 };
 
 struct user_directory {
@@ -46,6 +46,10 @@ static void user_move_iters(struct user_directory *dir, struct user *user)
        array_foreach(&dir->iters, iterp) {
                if ((*iterp)->pos == user)
                        (*iterp)->pos = user->next;
+               if ((*iterp)->stop_after_tail == user) {
+                       (*iterp)->stop_after_tail =
+                               user->prev != NULL ? user->prev : user->next;
+               }
        }
 }
 
@@ -291,13 +295,15 @@ void user_directory_deinit(struct user_directory **_dir)
 }
 
 struct user_directory_iter *
-user_directory_iter_init(struct user_directory *dir)
+user_directory_iter_init(struct user_directory *dir,
+                        bool iter_until_current_tail)
 {
        struct user_directory_iter *iter;
 
        iter = i_new(struct user_directory_iter, 1);
        iter->dir = dir;
        iter->pos = dir->head;
+       iter->stop_after_tail = iter_until_current_tail ? dir->tail : NULL;
        array_append(&dir->iters, &iter, 1);
        user_directory_drop_expired(dir);
        return iter;
@@ -312,6 +318,10 @@ struct user *user_directory_iter_next(struct user_directory_iter *iter)
                return NULL;
 
        iter->pos = user->next;
+       if (user == iter->stop_after_tail) {
+               /* this is the last user we want to iterate */
+               iter->pos = NULL;
+       }
        return user;
 }
 

diff --git a/src/director/user-directory.h b/src/director/user-directory.h
index 5b46330a664d2637189c2e18c0183ad5f1e425c2..9a7ed495845a25700319457fa0e8eea24ec6d4e1 100644 (file)
--- a/src/director/user-directory.h
+++ b/src/director/user-directory.h
@@ -62,10 +62,15 @@ bool user_directory_user_is_near_expiring(struct user_directory *dir,
                                          struct user *user);
 
 /* Iterate through users in the directory. It's safe to modify user directory
-   while iterators are running. The moved/removed users will just be skipped
-   over. */
+   while iterators are running. The removed users will just be skipped over.
+   Users that are refreshed (= moved to end of list) may be processed twice.
+
+   Using iter_until_current_tail=TRUE causes the iterator to not iterate
+   through any users that were added/refreshed since the iteration began.
+   Note that this may skip some users entirely. */
 struct user_directory_iter *
-user_directory_iter_init(struct user_directory *dir);
+user_directory_iter_init(struct user_directory *dir,
+                        bool iter_until_current_tail);
 struct user *user_directory_iter_next(struct user_directory_iter *iter);
 void user_directory_iter_deinit(struct user_directory_iter **iter);