dict: Various reference counting and other fixes related to using freed memory.

diff --git a/src/dict/dict-commands.c b/src/dict/dict-commands.c
index 2fb43fc98eb3c4fd637ee69b7bf25df60ae91b8b..5b73cc25fd2699dd5c7a01552754766397c38ffb 100644 (file)
--- a/src/dict/dict-commands.c
+++ b/src/dict/dict-commands.c
@@ -27,7 +27,7 @@ struct dict_connection_cmd {
        struct dict_iterate_context *iter;
        enum dict_iterate_flags iter_flags;
 
-       struct dict_connection_transaction *trans;
+       unsigned int trans_id;
 };
 
 static void dict_connection_cmd_output_more(struct dict_connection_cmd *cmd);
@@ -36,10 +36,10 @@ static void dict_connection_cmd_free(struct dict_connection_cmd *cmd)
 {
        if (cmd->iter != NULL)
                (void)dict_iterate_deinit(&cmd->iter);
-       array_delete(&cmd->conn->cmds, 0, 1);
        i_free(cmd->reply);
 
-       dict_connection_continue_input(cmd->conn);
+       if (dict_connection_unref(cmd->conn))
+               dict_connection_continue_input(cmd->conn);
        i_free(cmd);
 }
 
@@ -51,6 +51,7 @@ static void dict_connection_cmd_remove(struct dict_connection_cmd *cmd)
        cmds = array_get(&cmd->conn->cmds, &count);
        for (i = 0; i < count; i++) {
                if (cmds[i] == cmd) {
+                       array_delete(&cmd->conn->cmds, i, 1);
                        dict_connection_cmd_free(cmd);
                        return;
                }
@@ -62,6 +63,7 @@ static void dict_connection_cmds_flush(struct dict_connection *conn)
 {
        struct dict_connection_cmd *cmd, *const *first_cmdp;
 
+       dict_connection_ref(conn);
        while (array_count(&conn->cmds) > 0) {
                first_cmdp = array_idx(&conn->cmds, 0);
                cmd = *first_cmdp;
@@ -74,16 +76,7 @@ static void dict_connection_cmds_flush(struct dict_connection *conn)
                o_stream_nsend_str(conn->output, cmd->reply);
                dict_connection_cmd_remove(cmd);
        }
-}
-
-void dict_connection_cmds_free(struct dict_connection *conn)
-{
-       struct dict_connection_cmd *const *first_cmdp;
-
-       while (array_count(&conn->cmds) > 0) {
-               first_cmdp = array_idx(&conn->cmds, 0);
-               dict_connection_cmd_remove(*first_cmdp);
-       }
+       dict_connection_unref(conn);
 }
 
 static void
@@ -201,20 +194,20 @@ dict_connection_transaction_lookup(struct dict_connection *conn,
 
 static void
 dict_connection_transaction_array_remove(struct dict_connection *conn,
-                                        struct dict_connection_transaction *trans)
+                                        unsigned int id)
 {
        const struct dict_connection_transaction *transactions;
        unsigned int i, count;
 
-       i_assert(trans->ctx == NULL);
-
        transactions = array_get(&conn->transactions, &count);
        for (i = 0; i < count; i++) {
-               if (&transactions[i] == trans) {
+               if (transactions[i].id == id) {
+                       i_assert(transactions[i].ctx == NULL);
                        array_delete(&conn->transactions, i, 1);
-                       break;
+                       return;
                }
        }
+       i_unreached();
 }
 
 static int cmd_begin(struct dict_connection_cmd *cmd, const char *line)
@@ -279,11 +272,11 @@ cmd_commit_finish(struct dict_connection_cmd *cmd, int ret, bool async)
        }
        if (async) {
                cmd->reply = i_strdup_printf("%c%c%u\n",
-                       DICT_PROTOCOL_REPLY_ASYNC_COMMIT, chr, cmd->trans->id);
+                       DICT_PROTOCOL_REPLY_ASYNC_COMMIT, chr, cmd->trans_id);
        } else {
-               cmd->reply = i_strdup_printf("%c%u\n", chr, cmd->trans->id);
+               cmd->reply = i_strdup_printf("%c%u\n", chr, cmd->trans_id);
        }
-       dict_connection_transaction_array_remove(cmd->conn, cmd->trans);
+       dict_connection_transaction_array_remove(cmd->conn, cmd->trans_id);
        dict_connection_cmds_flush(cmd->conn);
 }
 
@@ -304,20 +297,26 @@ static void cmd_commit_callback_async(int ret, void *context)
 static int
 cmd_commit(struct dict_connection_cmd *cmd, const char *line)
 {
-       if (dict_connection_transaction_lookup_parse(cmd->conn, line, &cmd->trans) < 0)
+       struct dict_connection_transaction *trans;
+
+       if (dict_connection_transaction_lookup_parse(cmd->conn, line, &trans) < 0)
                return -1;
+       cmd->trans_id = trans->id;
 
-       dict_transaction_commit_async(&cmd->trans->ctx, cmd_commit_callback, cmd);
+       dict_transaction_commit_async(&trans->ctx, cmd_commit_callback, cmd);
        return 1;
 }
 
 static int
 cmd_commit_async(struct dict_connection_cmd *cmd, const char *line)
 {
-       if (dict_connection_transaction_lookup_parse(cmd->conn, line, &cmd->trans) < 0)
+       struct dict_connection_transaction *trans;
+
+       if (dict_connection_transaction_lookup_parse(cmd->conn, line, &trans) < 0)
                return -1;
+       cmd->trans_id = trans->id;
 
-       dict_transaction_commit_async(&cmd->trans->ctx, cmd_commit_callback_async, cmd);
+       dict_transaction_commit_async(&trans->ctx, cmd_commit_callback_async, cmd);
        return 1;
 }
 
@@ -329,7 +328,7 @@ static int cmd_rollback(struct dict_connection_cmd *cmd, const char *line)
                return -1;
 
        dict_transaction_rollback(&trans->ctx);
-       dict_connection_transaction_array_remove(cmd->conn, trans);
+       dict_connection_transaction_array_remove(cmd->conn, trans->id);
        return 0;
 }
 
@@ -451,6 +450,7 @@ int dict_command_input(struct dict_connection *conn, const char *line)
        cmd->conn = conn;
        cmd->cmd = cmd_func;
        array_append(&conn->cmds, &cmd, 1);
+       dict_connection_ref(conn);
        if ((ret = cmd_func->func(cmd, line + 1)) <= 0) {
                dict_connection_cmd_remove(cmd);
                return ret;

diff --git a/src/dict/dict-commands.h b/src/dict/dict-commands.h
index 6fe32b2de0f3ae3ec5e6a4b01ad44a521fba8892..03214c4e52081a3a0cf904cec22e64eddb4ce492 100644 (file)
--- a/src/dict/dict-commands.h
+++ b/src/dict/dict-commands.h
@@ -6,6 +6,5 @@ struct dict_connection;
 int dict_command_input(struct dict_connection *conn, const char *line);
 
 void dict_connection_cmds_output_more(struct dict_connection *conn);
-void dict_connection_cmds_free(struct dict_connection *conn);
 
 #endif

diff --git a/src/dict/dict-connection.c b/src/dict/dict-connection.c
index cd13cb643bfda072f4e76b1dcd8797cbf8773b9a..b2b3499d3c267ed7b6d7dcd7da7e2f6e5427402b 100644 (file)
--- a/src/dict/dict-connection.c
+++ b/src/dict/dict-connection.c
@@ -157,7 +157,7 @@ static void dict_connection_input(struct dict_connection *conn)
 
 void dict_connection_continue_input(struct dict_connection *conn)
 {
-       if (conn->io != NULL)
+       if (conn->io != NULL || conn->destroyed)
                return;
 
        conn->io = io_add(conn->fd, IO_READ, dict_connection_input, conn);
@@ -183,6 +183,7 @@ struct dict_connection *dict_connection_create(int fd)
        struct dict_connection *conn;
 
        conn = i_new(struct dict_connection, 1);
+       conn->refcount = 1;
        conn->fd = fd;
        conn->input = i_stream_create_fd(fd, DICT_CLIENT_MAX_LINE_LENGTH,
                                         FALSE);
@@ -195,41 +196,73 @@ struct dict_connection *dict_connection_create(int fd)
        return conn;
 }
 
-void dict_connection_destroy(struct dict_connection *conn)
+void dict_connection_ref(struct dict_connection *conn)
+{
+       i_assert(conn->refcount > 0);
+       conn->refcount++;
+}
+
+bool dict_connection_unref(struct dict_connection *conn)
 {
        struct dict_connection_transaction *transaction;
 
-       DLLIST_REMOVE(&dict_connections, conn);
+       i_assert(conn->refcount > 0);
+       if (--conn->refcount > 0)
+               return TRUE;
+
+       i_assert(array_count(&conn->cmds) == 0);
+
+       /* we should have only transactions that haven't been committed or
+          rollbacked yet. close those before dict is deinitialized. */
+       if (array_is_created(&conn->transactions)) {
+               array_foreach_modifiable(&conn->transactions, transaction) {
+                       if (transaction->ctx != NULL)
+                               dict_transaction_rollback(&transaction->ctx);
+               }
+       }
 
-       /* deinit dict before anything else, so any pending dict operations
-          are aborted and their callbacks called. */
        if (conn->dict != NULL)
                dict_deinit(&conn->dict);
 
-       if (array_is_created(&conn->transactions)) {
-               array_foreach_modifiable(&conn->transactions, transaction)
-                       dict_transaction_rollback(&transaction->ctx);
+       if (array_is_created(&conn->transactions))
                array_free(&conn->transactions);
-       }
 
-       /* this may end up adding conn->io back, so keep this early */
-       dict_connection_cmds_free(conn);
+       i_stream_destroy(&conn->input);
+       o_stream_destroy(&conn->output);
+
        array_free(&conn->cmds);
+       i_free(conn->name);
+       i_free(conn->username);
+       i_free(conn);
+
+       master_service_client_connection_destroyed(master_service);
+       return FALSE;
+}
+
+void dict_connection_destroy(struct dict_connection *conn)
+{
+       conn->destroyed = TRUE;
+       DLLIST_REMOVE(&dict_connections, conn);
 
        if (conn->to_input != NULL)
                timeout_remove(&conn->to_input);
        if (conn->io != NULL)
                io_remove(&conn->io);
-       i_stream_destroy(&conn->input);
-       o_stream_destroy(&conn->output);
+       i_stream_close(conn->input);
+       o_stream_close(conn->output);
        if (close(conn->fd) < 0)
                i_error("close(dict client) failed: %m");
+       conn->fd = -1;
 
-       i_free(conn->name);
-       i_free(conn->username);
-       i_free(conn);
+       /* the connection is closed, but there may still be commands left
+          running. finish them, even if the calling client can't be notified
+          about whether they succeeded (clients may not even care).
 
-       master_service_client_connection_destroyed(master_service);
+          flush the command output here in case we were waiting on iteration
+          output. */
+       dict_connection_cmds_output_more(conn);
+
+       dict_connection_unref(conn);
 }
 
 void dict_connections_destroy_all(void)

diff --git a/src/dict/dict-connection.h b/src/dict/dict-connection.h
index 3d437d7e4997d384a6a040d8cee243c53b3f60f1..7ab6cef45fc2dfff0f4bc8f3c9cdd9e00399248e 100644 (file)
--- a/src/dict/dict-connection.h
+++ b/src/dict/dict-connection.h
@@ -12,6 +12,7 @@ struct dict_connection_transaction {
 struct dict_connection {
        struct dict_connection *prev, *next;
        struct dict_server *server;
+       int refcount;
 
        char *username;
        char *name;
@@ -28,11 +29,16 @@ struct dict_connection {
           array is fast enough */
        ARRAY(struct dict_connection_transaction) transactions;
        ARRAY(struct dict_connection_cmd *) cmds;
+
+       unsigned int destroyed:1;
 };
 
 struct dict_connection *dict_connection_create(int fd);
 void dict_connection_destroy(struct dict_connection *conn);
 
+void dict_connection_ref(struct dict_connection *conn);
+bool dict_connection_unref(struct dict_connection *conn);
+
 void dict_connection_continue_input(struct dict_connection *conn);
 
 void dict_connections_destroy_all(void);