director: Prevent race conditions by adding USER_KILL_STATE_FLUSHING

In theory it's possible that a user is freed during a flush and added back
before flush is finished, possibly even being moved again. This check makes
sure that we don't finish such move unless we're actually at the correct
flushing state. (If there's another flush also running for the user it'll
be ignored.)

This is also useful for logging purposes.

diff --git a/src/director/director.c b/src/director/director.c
index cfac4e60c1a2fc77c860a363882f5d1f873042c1..92429793834e6605457447e5a577c18a55b585d8 100644 (file)
--- a/src/director/director.c
+++ b/src/director/director.c
@@ -717,9 +717,18 @@ director_flush_user_continue(int result,
        struct user *user =
                user_directory_lookup(ctx->dir->users, ctx->username_hash);
 
-       if (user != NULL)
+       if (user == NULL) {
+               dir_debug("User %u freed while flushing, result=%d",
+                         ctx->username_hash, result);
+       } else if (user->kill_state != USER_KILL_STATE_FLUSHING) {
+               dir_debug("User %u move state changed while flushing, result=%d",
+                         ctx->username_hash, result);
+       } else {
+               dir_debug("Flushing user %u finished, result=%d",
+                         ctx->username_hash, result);
                director_user_kill_finish_delayed(ctx->dir, user,
                                                  result == 1);
+       }
        if (result == 0) {
                struct istream *is = iostream_temp_finish(&ctx->reply, (size_t)-1);
                char *data;
@@ -781,6 +790,8 @@ director_flush_user(struct director *dir, struct user *user)
        const char *const args[] = {"FLUSH",
                t_strdup_printf("%u", user->username_hash), NULL};
 
+       user->kill_state = USER_KILL_STATE_FLUSHING;
+
        if ((program_client_create(ctx->socket_path, args, &set, FALSE,
                                   &ctx->pclient, &error)) != 0) {
                i_error("%s: Failed to flush user hash %u in host %s: %s",
@@ -857,6 +868,7 @@ struct director_kill_context {
 static void
 director_finish_user_kill(struct director *dir, struct user *user, bool self)
 {
+       i_assert(user->kill_state != USER_KILL_STATE_FLUSHING);
        i_assert(user->kill_state != USER_KILL_STATE_DELAY);
 
        if (dir->right == NULL) {
@@ -925,6 +937,7 @@ static void director_user_move_throttled(unsigned int new_events_count,
 
 static void director_user_move_timeout(struct user *user)
 {
+       i_assert(user->kill_state != USER_KILL_STATE_FLUSHING);
        i_assert(user->kill_state != USER_KILL_STATE_DELAY);
 
        if (log_throttle_accept(user_move_throttle)) {
@@ -1095,6 +1108,7 @@ void director_user_killed(struct director *dir, unsigned int username_hash)
                director_finish_user_kill(dir, user, TRUE);
                break;
        case USER_KILL_STATE_NONE:
+       case USER_KILL_STATE_FLUSHING:
        case USER_KILL_STATE_DELAY:
        case USER_KILL_STATE_KILLING_NOTIFY_RECEIVED:
                break;

diff --git a/src/director/user-directory.c b/src/director/user-directory.c
index 0dd8e2948331c70d5310c450a826af3d94ecf802..f516d4c565ee93d3b347c4598e0c70083c89a7f2 100644 (file)
--- a/src/director/user-directory.c
+++ b/src/director/user-directory.c
@@ -42,6 +42,7 @@ const char *user_kill_state_names[USER_KILL_STATE_DELAY+1] = {
        "notify-received",
        "waiting-for-notify",
        "waiting-for-everyone",
+       "flushing",
        "delay",
 };
 

diff --git a/src/director/user-directory.h b/src/director/user-directory.h
index b68d7e14bc28f263dcfd28d0bddf231e843de512..fe15babf5e9bcb23e59289d7c265138f3d04da9a 100644 (file)
--- a/src/director/user-directory.h
+++ b/src/director/user-directory.h
@@ -16,6 +16,8 @@ enum user_kill_state {
        /* We're done killing, but waiting for USER-KILLED-EVERYWHERE
           notification until this state gets reset. */
        USER_KILL_STATE_KILLED_WAITING_FOR_EVERYONE,
+       /* Waiting for the flush socket to finish. */
+       USER_KILL_STATE_FLUSHING,
        /* Wait for a while for the user connections to actually die. Note that
           only at this stage we can be sure that all the directors know about
           the user move (although it could be earlier if we added a new