dovecot-2.2: dict: Various reference counting and other fixes re...
dovecot at dovecot.org
dovecot at dovecot.org
Thu Sep 3 17:56:19 UTC 2015
details: http://hg.dovecot.org/dovecot-2.2/rev/bed46aaaa769
changeset: 19081:bed46aaaa769
user: Timo Sirainen <tss at iki.fi>
date: Thu Sep 03 20:54:27 2015 +0300
description:
dict: Various reference counting and other fixes related to using freed memory.
diffstat:
src/dict/dict-commands.c | 52 +++++++++++++++++-----------------
src/dict/dict-commands.h | 1 -
src/dict/dict-connection.c | 67 ++++++++++++++++++++++++++++++++++-----------
src/dict/dict-connection.h | 6 ++++
4 files changed, 82 insertions(+), 44 deletions(-)
diffs (296 lines):
diff -r f116e41d868d -r bed46aaaa769 src/dict/dict-commands.c
--- a/src/dict/dict-commands.c Thu Sep 03 19:59:16 2015 +0300
+++ b/src/dict/dict-commands.c Thu Sep 03 20:54:27 2015 +0300
@@ -27,7 +27,7 @@
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 @@
{
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 @@
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 @@
{
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 @@
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 @@
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 @@
}
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 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 @@
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 @@
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 -r f116e41d868d -r bed46aaaa769 src/dict/dict-commands.h
--- a/src/dict/dict-commands.h Thu Sep 03 19:59:16 2015 +0300
+++ b/src/dict/dict-commands.h Thu Sep 03 20:54:27 2015 +0300
@@ -6,6 +6,5 @@
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 -r f116e41d868d -r bed46aaaa769 src/dict/dict-connection.c
--- a/src/dict/dict-connection.c Thu Sep 03 19:59:16 2015 +0300
+++ b/src/dict/dict-connection.c Thu Sep 03 20:54:27 2015 +0300
@@ -157,7 +157,7 @@
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 *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 @@
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;
- /* deinit dict before anything else, so any pending dict operations
- are aborted and their callbacks called. */
+ 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);
+ }
+ }
+
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 -r f116e41d868d -r bed46aaaa769 src/dict/dict-connection.h
--- a/src/dict/dict-connection.h Thu Sep 03 19:59:16 2015 +0300
+++ b/src/dict/dict-connection.h Thu Sep 03 20:54:27 2015 +0300
@@ -12,6 +12,7 @@
struct dict_connection {
struct dict_connection *prev, *next;
struct dict_server *server;
+ int refcount;
char *username;
char *name;
@@ -28,11 +29,16 @@
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);
More information about the dovecot-cvs
mailing list