From 661998e2ccd772ad92a9d4a75cb712692a8c94b3 Mon Sep 17 00:00:00 2001 From: Timo Sirainen Date: Fri, 6 May 2016 15:14:02 +0300 Subject: [PATCH] lib-dict: dict_transaction_commit*() returns now error string --- src/dict/dict-commands.c | 32 ++-- src/doveadm/doveadm-dict.c | 15 +- src/lib-dict-extra/dict-fs.c | 20 +- src/lib-dict/dict-client.c | 182 ++++++++++++------- src/lib-dict/dict-file.c | 45 +++-- src/lib-dict/dict-memcached-ascii.c | 68 +++---- src/lib-dict/dict-redis.c | 109 ++++++----- src/lib-dict/dict-sql.c | 82 +++++---- src/lib-dict/dict.c | 28 ++- src/lib-dict/dict.h | 12 +- src/lib-fs/fs-dict.c | 10 +- src/lib-storage/index/index-transaction.c | 11 +- src/plugins/acl/acl-lookup-dict.c | 4 +- src/plugins/expire/doveadm-expire.c | 4 +- src/plugins/expire/expire-plugin.c | 5 +- src/plugins/last-login/last-login-plugin.c | 4 +- src/plugins/quota-clone/quota-clone-plugin.c | 5 +- src/plugins/quota/quota-dict.c | 10 +- src/plugins/quota/quota.c | 5 +- 19 files changed, 398 insertions(+), 253 deletions(-) diff --git a/src/dict/dict-commands.c b/src/dict/dict-commands.c index 712a73e320..471faf1111 100644 --- a/src/dict/dict-commands.c +++ b/src/dict/dict-commands.c @@ -261,11 +261,13 @@ dict_connection_transaction_lookup_parse(struct dict_connection *conn, } static void -cmd_commit_finish(struct dict_connection_cmd *cmd, int ret, bool async) +cmd_commit_finish(struct dict_connection_cmd *cmd, + const struct dict_commit_result *result, bool async) { + string_t *str = t_str_new(64); char chr; - switch (ret) { + switch (result->ret) { case 1: chr = DICT_PROTOCOL_REPLY_OK; break; @@ -273,31 +275,39 @@ cmd_commit_finish(struct dict_connection_cmd *cmd, int ret, bool async) chr = DICT_PROTOCOL_REPLY_NOTFOUND; break; default: + i_assert(result->error != NULL); chr = DICT_PROTOCOL_REPLY_FAIL; break; } - if (async) { - cmd->reply = i_strdup_printf("%c%c%u\n", - DICT_PROTOCOL_REPLY_ASYNC_COMMIT, chr, cmd->trans_id); - } else { - cmd->reply = i_strdup_printf("%c%u\n", chr, cmd->trans_id); + if (async) + str_append_c(str, DICT_PROTOCOL_REPLY_ASYNC_COMMIT); + str_append_c(str, chr); + str_printfa(str, "%c%u", chr, cmd->trans_id); + if (chr == DICT_PROTOCOL_REPLY_FAIL) { + str_append_c(str, '\t'); + str_append_tabescaped(str, result->error); } + str_append_c(str, '\n'); + cmd->reply = i_strdup(str_c(str)); + dict_connection_transaction_array_remove(cmd->conn, cmd->trans_id); dict_connection_cmds_flush(cmd->conn); } -static void cmd_commit_callback(int ret, void *context) +static void cmd_commit_callback(const struct dict_commit_result *result, + void *context) { struct dict_connection_cmd *cmd = context; - cmd_commit_finish(cmd, ret, FALSE); + cmd_commit_finish(cmd, result, FALSE); } -static void cmd_commit_callback_async(int ret, void *context) +static void cmd_commit_callback_async(const struct dict_commit_result *result, + void *context) { struct dict_connection_cmd *cmd = context; - cmd_commit_finish(cmd, ret, TRUE); + cmd_commit_finish(cmd, result, TRUE); } static int diff --git a/src/doveadm/doveadm-dict.c b/src/doveadm/doveadm-dict.c index 29310568ac..cb9e24021d 100644 --- a/src/doveadm/doveadm-dict.c +++ b/src/doveadm/doveadm-dict.c @@ -121,14 +121,15 @@ static void cmd_dict_set(int argc, char *argv[]) { struct dict *dict; struct dict_transaction_context *trans; + const char *error; if (cmd_dict_init(&argc, &argv, 2, 0, cmd_dict_set, &dict) < 0) return; trans = dict_transaction_begin(dict); dict_set(trans, argv[0], argv[1]); - if (dict_transaction_commit(&trans) <= 0) { - i_error("dict_transaction_commit() failed"); + if (dict_transaction_commit(&trans, &error) <= 0) { + i_error("dict_transaction_commit() failed: %s", error); doveadm_exit_code = EX_TEMPFAIL; } dict_deinit(&dict); @@ -138,14 +139,15 @@ static void cmd_dict_unset(int argc, char *argv[]) { struct dict *dict; struct dict_transaction_context *trans; + const char *error; if (cmd_dict_init(&argc, &argv, 1, 0, cmd_dict_unset, &dict) < 0) return; trans = dict_transaction_begin(dict); dict_unset(trans, argv[0]); - if (dict_transaction_commit(&trans) <= 0) { - i_error("dict_transaction_commit() failed"); + if (dict_transaction_commit(&trans, &error) <= 0) { + i_error("dict_transaction_commit() failed: %s", error); doveadm_exit_code = EX_TEMPFAIL; } dict_deinit(&dict); @@ -155,6 +157,7 @@ static void cmd_dict_inc(int argc, char *argv[]) { struct dict *dict; struct dict_transaction_context *trans; + const char *error; long long diff; int ret; @@ -170,9 +173,9 @@ static void cmd_dict_inc(int argc, char *argv[]) trans = dict_transaction_begin(dict); dict_atomic_inc(trans, argv[0], diff); - ret = dict_transaction_commit(&trans); + ret = dict_transaction_commit(&trans, &error); if (ret < 0) { - i_error("dict_transaction_commit() failed"); + i_error("dict_transaction_commit() failed: %s", error); doveadm_exit_code = EX_TEMPFAIL; } else if (ret == 0) { i_error("%s doesn't exist", argv[0]); diff --git a/src/lib-dict-extra/dict-fs.c b/src/lib-dict-extra/dict-fs.c index bf1595f92c..7b5ff9ca92 100644 --- a/src/lib-dict-extra/dict-fs.c +++ b/src/lib-dict-extra/dict-fs.c @@ -219,7 +219,8 @@ fs_dict_transaction_init(struct dict *_dict) return &ctx->ctx; } -static int fs_dict_write_changes(struct dict_transaction_memory_context *ctx) +static int fs_dict_write_changes(struct dict_transaction_memory_context *ctx, + const char **error_r) { struct fs_dict *dict = (struct fs_dict *)ctx->ctx.dict; struct fs_file *file; @@ -234,7 +235,8 @@ static int fs_dict_write_changes(struct dict_transaction_memory_context *ctx) file = fs_file_init(dict->fs, key, FS_OPEN_MODE_REPLACE); if (fs_write(file, change->value.str, strlen(change->value.str)) < 0) { - i_error("fs_write(%s) failed: %s", key, + *error_r = t_strdup_printf( + "fs_write(%s) failed: %s", key, fs_file_last_error(file)); ret = -1; } @@ -243,7 +245,8 @@ static int fs_dict_write_changes(struct dict_transaction_memory_context *ctx) case DICT_CHANGE_TYPE_UNSET: file = fs_file_init(dict->fs, key, FS_OPEN_MODE_READONLY); if (fs_delete(file) < 0) { - i_error("fs_delete(%s) failed: %s", key, + *error_r = t_strdup_printf( + "fs_delete(%s) failed: %s", key, fs_file_last_error(file)); ret = -1; } @@ -266,15 +269,14 @@ fs_dict_transaction_commit(struct dict_transaction_context *_ctx, { struct dict_transaction_memory_context *ctx = (struct dict_transaction_memory_context *)_ctx; - int ret; + struct dict_commit_result result = { .ret = 1 }; - if (fs_dict_write_changes(ctx) < 0) - ret = -1; - else - ret = 1; + + if (fs_dict_write_changes(ctx, &result.error) < 0) + result.ret = -1; pool_unref(&ctx->pool); - callback(ret, context); + callback(&result, context); } struct dict dict_driver_fs = { diff --git a/src/lib-dict/dict-client.c b/src/lib-dict/dict-client.c index bf9a370c2d..f2374b1948 100644 --- a/src/lib-dict/dict-client.c +++ b/src/lib-dict/dict-client.c @@ -69,17 +69,18 @@ struct client_dict_transaction_context { dict_transaction_commit_callback_t *callback; void *context; + char *error; + unsigned int id; unsigned int connect_counter; - unsigned int failed:1; unsigned int sent_begin:1; unsigned int async:1; unsigned int committed:1; }; static int client_dict_connect(struct client_dict *dict, const char **error_r); -static void client_dict_disconnect(struct client_dict *dict); +static void client_dict_disconnect(struct client_dict *dict, const char *reason); const char *dict_client_escape(const char *src) { @@ -171,15 +172,15 @@ static int client_dict_send_query(struct client_dict *dict, const char *query, if (o_stream_send_str(dict->output, query) < 0 || o_stream_flush(dict->output) < 0) { /* Send failed */ + *error_r = t_strdup_printf("write(%s) failed: %s", + dict->path, o_stream_get_error(dict->output)); if (!dict->handshaked) { /* we're trying to send hello, don't try to reconnect */ - *error_r = t_strdup_printf("write(%s) failed: %s", - dict->path, o_stream_get_error(dict->output)); return -1; } /* Reconnect and try again. */ - client_dict_disconnect(dict); + client_dict_disconnect(dict, *error_r); if (client_dict_connect(dict, error_r) < 0) return -1; @@ -187,7 +188,7 @@ static int client_dict_send_query(struct client_dict *dict, const char *query, o_stream_flush(dict->output) < 0) { *error_r = t_strdup_printf("write(%s) failed: %s", dict->path, o_stream_get_error(dict->output)); - client_dict_disconnect(dict); + client_dict_disconnect(dict, *error_r); return -1; } } @@ -195,41 +196,43 @@ static int client_dict_send_query(struct client_dict *dict, const char *query, } static int -client_dict_transaction_send_begin(struct client_dict_transaction_context *ctx) +client_dict_transaction_send_begin(struct client_dict_transaction_context *ctx, + const char **error_r) { struct client_dict *dict = (struct client_dict *)ctx->ctx.dict; - const char *query, *error; + const char *query; - if (ctx->failed) - return -1; + i_assert(ctx->error == NULL); query = t_strdup_printf("%c%u\n", DICT_PROTOCOL_CMD_BEGIN, ctx->id); - if (client_dict_send_query(dict, query, &error) < 0) { - i_error("%s", error); - ctx->failed = TRUE; + if (client_dict_send_query(dict, query, error_r) < 0) return -1; - } ctx->connect_counter = dict->connect_counter; return 0; } -static int ATTR_NOWARN_UNUSED_RESULT +static int client_dict_send_transaction_query(struct client_dict_transaction_context *ctx, - const char *query) + const char *query, const char **error_r) { struct client_dict *dict = (struct client_dict *)ctx->ctx.dict; + i_assert(ctx->error == NULL); + if (!ctx->sent_begin) { - if (client_dict_transaction_send_begin(ctx) < 0) + if (client_dict_transaction_send_begin(ctx, error_r) < 0) return -1; ctx->sent_begin = TRUE; } - if (ctx->connect_counter != dict->connect_counter || ctx->failed) + if (ctx->connect_counter != dict->connect_counter) { + *error_r = "Reconnected to dict-server - transaction lost"; return -1; + } if (dict->output == NULL) { /* not connected, this'll fail */ + *error_r = "Disconnected from dict-server"; return -1; } @@ -237,13 +240,26 @@ client_dict_send_transaction_query(struct client_dict_transaction_context *ctx, o_stream_flush(dict->output) < 0) { /* Send failed. Our transactions have died, so don't even try to re-send the command */ - ctx->failed = TRUE; - client_dict_disconnect(dict); + *error_r = t_strdup_printf("write(%s) failed: %s", + dict->path, o_stream_get_error(dict->output)); + client_dict_disconnect(dict, *error_r); return -1; } return 0; } +static void +client_dict_try_send_transaction_query(struct client_dict_transaction_context *ctx, + const char *query) +{ + const char *error; + + if (ctx->error != NULL) + return; + if (client_dict_send_transaction_query(ctx, query, &error) < 0) + ctx->error = i_strdup(error); +} + static struct client_dict_transaction_context * client_dict_transaction_find(struct client_dict *dict, unsigned int id) { @@ -257,8 +273,8 @@ client_dict_transaction_find(struct client_dict *dict, unsigned int id) } static void -client_dict_finish_transaction(struct client_dict *dict, - unsigned int id, int ret) +client_dict_finish_transaction(struct client_dict *dict, unsigned int id, + const struct dict_commit_result *result) { struct client_dict_transaction_context *ctx; @@ -267,9 +283,17 @@ client_dict_finish_transaction(struct client_dict *dict, i_error("dict-client: Unknown transaction id %u", id); return; } - ctx->failed = TRUE; - if (!ctx->committed) + if (!ctx->committed) { + /* transaction isn't committed yet, but we disconnected from + dict. mark it as failed so commit will fail later. */ + if (result->ret >= 0) { + i_error("dict-client: Received transaction reply before it was committed"); + return; + } + if (ctx->error == NULL) + ctx->error = i_strdup(result->error); return; + } /* the callback may call the dict code again, so remove this transaction before calling it */ @@ -281,7 +305,7 @@ client_dict_finish_transaction(struct client_dict *dict, DLLIST_REMOVE(&dict->transactions, ctx); if (ctx->callback != NULL) - ctx->callback(ret, ctx->context); + ctx->callback(result, ctx->context); i_free(ctx); } @@ -347,26 +371,35 @@ client_dict_read_one_line_real(struct client_dict *dict, char **line_r, } } if (*line == DICT_PROTOCOL_REPLY_ASYNC_COMMIT) { + struct dict_commit_result result; + + memset(&result, 0, sizeof(result)); switch (line[1]) { case DICT_PROTOCOL_REPLY_OK: - ret = 1; + result.ret = 1; break; case DICT_PROTOCOL_REPLY_NOTFOUND: - ret = 0; + result.ret = 0; break; - case DICT_PROTOCOL_REPLY_FAIL: - ret = -1; + case DICT_PROTOCOL_REPLY_FAIL: { + const char *error = strchr(line+2, '\t'); + + result.ret = -1; + result.error = t_strdup_printf( + "dict-server returned failure: %s", + error != NULL ? dict_client_unescape(error) : ""); break; + } default: *error_r = t_strdup_printf( "dict-client: Invalid async commit line: %s", line); return -1; } - if (str_to_uint(line+2, &id) < 0) { + if (str_to_uint(t_strcut(line+2, '\t'), &id) < 0) { *error_r = t_strdup_printf("dict-client: Invalid ID"); return -1; } - client_dict_finish_transaction(dict, id, ret); + client_dict_finish_transaction(dict, id, &result); return 0; } if (dict->iter_replies_skip > 0) { @@ -392,7 +425,7 @@ static int client_dict_read_one_line(struct client_dict *dict, char **line_r, int ret; if ((ret = client_dict_read_one_line_real(dict, line_r, error_r)) < 0) - client_dict_disconnect(dict); + client_dict_disconnect(dict, *error_r); return ret; } @@ -405,7 +438,7 @@ static bool client_dict_is_finished(struct client_dict *dict) static void client_dict_timeout(struct client_dict *dict) { if (client_dict_is_finished(dict)) - client_dict_disconnect(dict); + client_dict_disconnect(dict, "Idle disconnection"); } static void client_dict_add_timeout(struct client_dict *dict) @@ -468,7 +501,7 @@ static int client_dict_connect(struct client_dict *dict, const char **error_r) dict->value_type, dict->username, dict->uri); if (client_dict_send_query(dict, query, error_r) < 0) { dict->last_failed_connect = ioloop_time; - client_dict_disconnect(dict); + client_dict_disconnect(dict, *error_r); return -1; } @@ -476,9 +509,10 @@ static int client_dict_connect(struct client_dict *dict, const char **error_r) return 0; } -static void client_dict_disconnect(struct client_dict *dict) +static void client_dict_disconnect(struct client_dict *dict, const char *reason) { struct client_dict_transaction_context *ctx, *next; + struct dict_commit_result result = { -1, reason }; dict->connect_counter++; dict->handshaked = FALSE; @@ -488,7 +522,7 @@ static void client_dict_disconnect(struct client_dict *dict) for (ctx = dict->transactions; ctx != NULL; ctx = next) { next = ctx->next; if (ctx->async) - client_dict_finish_transaction(dict, ctx->id, -1); + client_dict_finish_transaction(dict, ctx->id, &result); } if (dict->to_idle != NULL) @@ -567,7 +601,7 @@ static void client_dict_deinit(struct dict *_dict) { struct client_dict *dict = (struct client_dict *)_dict; - client_dict_disconnect(dict); + client_dict_disconnect(dict, "Deinit"); i_assert(dict->transactions == NULL); pool_unref(&dict->pool); } @@ -586,8 +620,10 @@ static void client_dict_wait(struct dict *_dict) } if (ret > 0) { - i_error("dict-client: Unexpected reply waiting waiting for async commits: %s", line); - client_dict_disconnect(dict); + const char *reason = t_strdup_printf( + "dict-client: Unexpected reply waiting waiting for async commits: %s", line); + i_error("%s", reason); + client_dict_disconnect(dict, reason); break; } } @@ -626,7 +662,7 @@ static int client_dict_lookup(struct dict *_dict, pool_t pool, const char *key, default: *error_r = t_strdup_printf( "dict-client: Invalid lookup '%s' reply: %s", key, line); - client_dict_disconnect(dict); + client_dict_disconnect(dict, *error_r); return -1; } } @@ -763,8 +799,10 @@ static void dict_async_input(struct client_dict *dict) i_error("%s", error); io_remove(&dict->io); } else if (ret > 0) { - i_error("dict-client: Unexpected reply waiting waiting for async commits: %s", line); - client_dict_disconnect(dict); + const char *reason = t_strdup_printf( + "dict-client: Unexpected reply waiting waiting for async commits: %s", line); + i_error("%s", reason); + client_dict_disconnect(dict, reason); } } @@ -778,19 +816,23 @@ client_dict_transaction_commit(struct dict_transaction_context *_ctx, (struct client_dict_transaction_context *)_ctx; struct client_dict *dict = (struct client_dict *)_ctx->dict; unsigned int id; - int ret = ctx->failed ? -1 : 1; + struct dict_commit_result result; + + memset(&result, 0, sizeof(result)); + result.ret = ctx->error != NULL ? -1 : 1; + result.error = t_strdup(ctx->error); ctx->committed = TRUE; - if (ctx->sent_begin && !ctx->failed) { - const char *query, *error; + if (ctx->sent_begin && ctx->error == NULL) { + const char *query; char *line; query = t_strdup_printf("%c%u\n", !async ? DICT_PROTOCOL_CMD_COMMIT : DICT_PROTOCOL_CMD_COMMIT_ASYNC, ctx->id); - if (client_dict_send_transaction_query(ctx, query) < 0) - ret = -1; + if (client_dict_send_transaction_query(ctx, query, &result.error) < 0) + result.ret = -1; else if (async) { ctx->callback = callback; ctx->context = context; @@ -802,38 +844,46 @@ client_dict_transaction_commit(struct dict_transaction_context *_ctx, return; } else { /* sync commit, read reply */ - if (client_dict_read_line(dict, &line, &error) < 0) { - i_error("%s", error); - ret = -1; + if (client_dict_read_line(dict, &line, &result.error) < 0) { + result.ret = -1; } else switch (*line) { case DICT_PROTOCOL_REPLY_OK: - ret = 1; + result.ret = 1; break; case DICT_PROTOCOL_REPLY_NOTFOUND: - ret = 0; + result.ret = 0; break; - case DICT_PROTOCOL_REPLY_FAIL: - ret = -1; + case DICT_PROTOCOL_REPLY_FAIL: { + const char *error = strchr(line+1, '\t'); + + result.ret = -1; + result.error = t_strdup_printf( + "dict-server returned failure: %s", + error != NULL ? dict_client_unescape(error) : ""); break; + } default: - i_error("dict-client: Invalid commit reply: %s", line); - client_dict_disconnect(dict); + result.ret = -1; + result.error = t_strdup_printf( + "dict-client: Invalid commit reply: %s", line); + client_dict_disconnect(dict, result.error); line = NULL; - ret = -1; break; } if (line != NULL && - (str_to_uint(line+1, &id) < 0 || ctx->id != id)) { - i_error("dict-client: Invalid commit reply, " + (str_to_uint(t_strcut(line+1, '\t'), &id) < 0 || ctx->id != id)) { + result.ret = -1; + result.error = t_strdup_printf( + "dict-client: Invalid commit reply, " "expected id=%u: %s", ctx->id, line); - client_dict_disconnect(dict); - ret = -1; + client_dict_disconnect(dict, result.error); } } } DLLIST_REMOVE(&dict->transactions, ctx); - callback(ret, context); + callback(&result, context); + i_free(ctx->error); i_free(ctx); client_dict_add_timeout(dict); @@ -851,7 +901,7 @@ client_dict_transaction_rollback(struct dict_transaction_context *_ctx) query = t_strdup_printf("%c%u\n", DICT_PROTOCOL_CMD_ROLLBACK, ctx->id); - client_dict_send_transaction_query(ctx, query); + client_dict_try_send_transaction_query(ctx, query); } DLLIST_REMOVE(&dict->transactions, ctx); @@ -871,7 +921,7 @@ static void client_dict_set(struct dict_transaction_context *_ctx, DICT_PROTOCOL_CMD_SET, ctx->id, dict_client_escape(key), dict_client_escape(value)); - client_dict_send_transaction_query(ctx, query); + client_dict_try_send_transaction_query(ctx, query); } static void client_dict_unset(struct dict_transaction_context *_ctx, @@ -884,7 +934,7 @@ static void client_dict_unset(struct dict_transaction_context *_ctx, query = t_strdup_printf("%c%u\t%s\n", DICT_PROTOCOL_CMD_UNSET, ctx->id, dict_client_escape(key)); - client_dict_send_transaction_query(ctx, query); + client_dict_try_send_transaction_query(ctx, query); } static void client_dict_atomic_inc(struct dict_transaction_context *_ctx, @@ -897,7 +947,7 @@ static void client_dict_atomic_inc(struct dict_transaction_context *_ctx, query = t_strdup_printf("%c%u\t%s\t%lld\n", DICT_PROTOCOL_CMD_ATOMIC_INC, ctx->id, dict_client_escape(key), diff); - client_dict_send_transaction_query(ctx, query); + client_dict_try_send_transaction_query(ctx, query); } struct dict dict_driver_client = { diff --git a/src/lib-dict/dict-file.c b/src/lib-dict/dict-file.c index 615d2935eb..113975ee89 100644 --- a/src/lib-dict/dict-file.c +++ b/src/lib-dict/dict-file.c @@ -507,8 +507,9 @@ file_dict_lock(struct file_dict *dict, struct file_lock **lock_r, return ret < 0 ? -1 : 0; } -static int file_dict_write_changes(struct dict_transaction_memory_context *ctx, - bool *atomic_inc_not_found_r) +static int +file_dict_write_changes(struct dict_transaction_memory_context *ctx, + bool *atomic_inc_not_found_r, const char **error_r) { struct file_dict *dict = (struct file_dict *)ctx->ctx.dict; struct dotlock *dotlock = NULL; @@ -516,7 +517,6 @@ static int file_dict_write_changes(struct dict_transaction_memory_context *ctx, const char *temp_path = NULL; struct hash_iterate_context *iter; struct ostream *output; - const char *error; char *key, *value; string_t *str; int fd = -1; @@ -526,15 +526,13 @@ static int file_dict_write_changes(struct dict_transaction_memory_context *ctx, switch (dict->lock_method) { case FILE_LOCK_METHOD_FCNTL: case FILE_LOCK_METHOD_FLOCK: - if (file_dict_lock(dict, &lock, &error) < 0) { - i_error("%s", error); + if (file_dict_lock(dict, &lock, error_r) < 0) return -1; - } temp_path = t_strdup_printf("%s.tmp", dict->path); fd = creat(temp_path, 0600); if (fd == -1) { - i_error("file dict commit: creat(%s) failed: %m", - temp_path); + *error_r = t_strdup_printf( + "dict-file: creat(%s) failed: %m", temp_path); return -1; } break; @@ -542,15 +540,14 @@ static int file_dict_write_changes(struct dict_transaction_memory_context *ctx, fd = file_dotlock_open(&file_dict_dotlock_settings, dict->path, 0, &dotlock); if (fd == -1 && errno == ENOENT) { - if (file_dict_mkdir(dict, &error) < 0) { - i_error("%s", error); + if (file_dict_mkdir(dict, error_r) < 0) return -1; - } fd = file_dotlock_open(&file_dict_dotlock_settings, dict->path, 0, &dotlock); } if (fd == -1) { - i_error("file dict commit: file_dotlock_open(%s) failed: %m", + *error_r = t_strdup_printf( + "dict-file: file_dotlock_open(%s) failed: %m", dict->path); return -1; } @@ -559,8 +556,7 @@ static int file_dict_write_changes(struct dict_transaction_memory_context *ctx, } /* refresh once more now that we're locked */ - if (file_dict_refresh(dict, &error) < 0) { - i_error("%s", error); + if (file_dict_refresh(dict, error_r) < 0) { if (dotlock != NULL) file_dotlock_delete(&dotlock); else { @@ -593,7 +589,8 @@ static int file_dict_write_changes(struct dict_transaction_memory_context *ctx, hash_table_iterate_deinit(&iter); if (o_stream_nfinish(output) < 0) { - i_error("write(%s) failed: %m", temp_path); + *error_r = t_strdup_printf("write(%s) failed: %s", temp_path, + o_stream_get_error(output)); o_stream_destroy(&output); i_close_fd(&fd); return -1; @@ -603,13 +600,14 @@ static int file_dict_write_changes(struct dict_transaction_memory_context *ctx, if (dotlock != NULL) { if (file_dotlock_replace(&dotlock, DOTLOCK_REPLACE_FLAG_DONT_CLOSE_FD) < 0) { + *error_r = t_strdup_printf("file_dotlock_replace() failed: %m"); i_close_fd(&fd); return -1; } } else { if (rename(temp_path, dict->path) < 0) { - i_error("rename(%s, %s) failed: %m", - temp_path, dict->path); + *error_r = t_strdup_printf("rename(%s, %s) failed: %m", + temp_path, dict->path); file_unlock(&lock); i_close_fd(&fd); return -1; @@ -631,18 +629,19 @@ file_dict_transaction_commit(struct dict_transaction_context *_ctx, { struct dict_transaction_memory_context *ctx = (struct dict_transaction_memory_context *)_ctx; + struct dict_commit_result result; bool atomic_inc_not_found; - int ret; - if (file_dict_write_changes(ctx, &atomic_inc_not_found) < 0) - ret = -1; + memset(&result, 0, sizeof(result)); + if (file_dict_write_changes(ctx, &atomic_inc_not_found, &result.error) < 0) + result.ret = -1; else if (atomic_inc_not_found) - ret = 0; + result.ret = 0; else - ret = 1; + result.ret = 1; pool_unref(&ctx->pool); - callback(ret, context); + callback(&result, context); } struct dict dict_driver_file = { diff --git a/src/lib-dict/dict-memcached-ascii.c b/src/lib-dict/dict-memcached-ascii.c index 3d1e4571a1..886d44a369 100644 --- a/src/lib-dict/dict-memcached-ascii.c +++ b/src/lib-dict/dict-memcached-ascii.c @@ -68,7 +68,8 @@ static struct connection_list *memcached_ascii_connections; static void memcached_ascii_callback(struct memcached_ascii_dict *dict, - const struct memcached_ascii_dict_reply *reply, int ret) + const struct memcached_ascii_dict_reply *reply, + const struct dict_commit_result *result) { if (reply->callback != NULL) { if (dict->prev_ioloop != NULL) { @@ -77,29 +78,38 @@ memcached_ascii_callback(struct memcached_ascii_dict *dict, or timeouts. */ current_ioloop = dict->prev_ioloop; } - reply->callback(ret, reply->context); + reply->callback(result, reply->context); if (dict->prev_ioloop != NULL) current_ioloop = dict->ioloop; } } -static void memcached_ascii_conn_destroy(struct connection *_conn) +static void +memcached_ascii_disconnected(struct memcached_ascii_connection *conn, + const char *reason) { - struct memcached_ascii_connection *conn = - (struct memcached_ascii_connection *)_conn; + const struct dict_commit_result result = { -1, reason }; const struct memcached_ascii_dict_reply *reply; - connection_disconnect(_conn); + connection_disconnect(&conn->conn); if (conn->dict->ioloop != NULL) io_loop_stop(conn->dict->ioloop); array_foreach(&conn->dict->replies, reply) - memcached_ascii_callback(conn->dict, reply, -1); + memcached_ascii_callback(conn->dict, reply, &result); array_clear(&conn->dict->replies); array_clear(&conn->dict->input_states); conn->reply_bytes_left = 0; } +static void memcached_ascii_conn_destroy(struct connection *_conn) +{ + struct memcached_ascii_connection *conn = + (struct memcached_ascii_connection *)_conn; + + memcached_ascii_disconnected(conn, connection_disconnect_reason(_conn)); +} + static bool memcached_ascii_input_value(struct memcached_ascii_connection *conn) { const unsigned char *data; @@ -191,6 +201,7 @@ static int memcached_ascii_input_reply_read(struct memcached_ascii_dict *dict, static int memcached_ascii_input_reply(struct memcached_ascii_dict *dict, const char **error_r) { + const struct dict_commit_result result = { 1, NULL }; struct memcached_ascii_dict_reply *replies; unsigned int count; int ret; @@ -204,7 +215,7 @@ static int memcached_ascii_input_reply(struct memcached_ascii_dict *dict, i_assert(count > 0); i_assert(replies[0].reply_count > 0); if (--replies[0].reply_count == 0) { - memcached_ascii_callback(dict, &replies[0], 1); + memcached_ascii_callback(dict, &replies[0], &result); array_delete(&dict->replies, 0, 1); } return 1; @@ -221,17 +232,16 @@ static void memcached_ascii_conn_input(struct connection *_conn) case 0: return; case -1: - memcached_ascii_conn_destroy(_conn); + memcached_ascii_disconnected(conn, + i_stream_get_disconnect_reason(_conn->input)); return; default: break; } while ((ret = memcached_ascii_input_reply(conn->dict, &error)) > 0) ; - if (ret < 0) { - i_error("%s", error); - memcached_ascii_conn_destroy(_conn); - } + if (ret < 0) + memcached_ascii_disconnected(conn, error); io_loop_stop(conn->dict->ioloop); } @@ -261,9 +271,10 @@ static int memcached_ascii_input_wait(struct memcached_ascii_dict *dict, static void memcached_ascii_input_timeout(struct memcached_ascii_dict *dict) { - i_error("memcached_ascii: Request timed out in %u.%03u secs", + const char *reason = t_strdup_printf( + "memcached_ascii: Request timed out in %u.%03u secs", dict->timeout_msecs/1000, dict->timeout_msecs%1000); - memcached_ascii_conn_destroy(&dict->conn.conn); + memcached_ascii_disconnected(&dict->conn, reason); } static int memcached_ascii_wait_replies(struct memcached_ascii_dict *dict, @@ -278,7 +289,7 @@ static int memcached_ascii_wait_replies(struct memcached_ascii_dict *dict, if ((ret = memcached_ascii_input_reply(dict, error_r)) != 0) { if (ret < 0) - memcached_ascii_conn_destroy(&dict->conn.conn); + memcached_ascii_disconnected(&dict->conn, *error_r); break; } ret = memcached_ascii_input_wait(dict, error_r); @@ -580,18 +591,16 @@ memcached_send_change(struct dict_memcached_ascii_commit_ctx *ctx, } static int -memcached_ascii_transaction_send(struct dict_memcached_ascii_commit_ctx *ctx) +memcached_ascii_transaction_send(struct dict_memcached_ascii_commit_ctx *ctx, + const char **error_r) { struct memcached_ascii_dict *dict = ctx->dict; struct memcached_ascii_dict_reply *reply; const struct dict_transaction_memory_change *changes; unsigned int i, count, old_state_count; - const char *error; - if (memcached_ascii_connect(dict, &error) < 0) { - i_error("%s", error); + if (memcached_ascii_connect(dict, error_r) < 0) return -1; - } old_state_count = array_count(&dict->input_states); changes = array_get(&ctx->memctx->changes, &count); @@ -621,8 +630,7 @@ memcached_ascii_transaction_commit(struct dict_transaction_context *_ctx, struct memcached_ascii_dict *dict = (struct memcached_ascii_dict *)_ctx->dict; struct dict_memcached_ascii_commit_ctx commit_ctx; - const char *error; - int ret = 1; + struct dict_commit_result result = { 1, NULL }; if (_ctx->changed) { memset(&commit_ctx, 0, sizeof(commit_ctx)); @@ -632,22 +640,20 @@ memcached_ascii_transaction_commit(struct dict_transaction_context *_ctx, commit_ctx.context = context; commit_ctx.str = str_new(default_pool, 128); - ret = memcached_ascii_transaction_send(&commit_ctx); + result.ret = memcached_ascii_transaction_send(&commit_ctx, &result.error); str_free(&commit_ctx.str); - if (async && ret == 0) { + if (async && result.ret == 0) { pool_unref(&ctx->pool); return; } - if (ret == 0) { - if (memcached_ascii_wait(dict, &error) < 0) { - i_error("%s", error); - ret = -1; - } + if (result.ret == 0) { + if (memcached_ascii_wait(dict, &result.error) < 0) + result.ret = -1; } } - callback(ret, context); + callback(&result, context); pool_unref(&ctx->pool); } diff --git a/src/lib-dict/dict-redis.c b/src/lib-dict/dict-redis.c index 2d38eb59c3..804808f96f 100644 --- a/src/lib-dict/dict-redis.c +++ b/src/lib-dict/dict-redis.c @@ -62,7 +62,7 @@ struct redis_dict { struct redis_dict_transaction_context { struct dict_transaction_context ctx; unsigned int cmd_count; - bool failed; + char *error; }; static struct connection_list *redis_connections; @@ -79,7 +79,8 @@ static void redis_input_state_remove(struct redis_dict *dict) } static void redis_callback(struct redis_dict *dict, - const struct redis_dict_reply *reply, int ret) + const struct redis_dict_reply *reply, + const struct dict_commit_result *result) { if (reply->callback != NULL) { if (dict->prev_ioloop != NULL) { @@ -88,23 +89,24 @@ static void redis_callback(struct redis_dict *dict, or timeouts. */ current_ioloop = dict->prev_ioloop; } - reply->callback(ret, reply->context); + reply->callback(result, reply->context); if (dict->prev_ioloop != NULL) current_ioloop = dict->ioloop; } } -static void redis_conn_destroy(struct connection *_conn) +static void +redis_disconnected(struct redis_connection *conn, const char *reason) { - struct redis_connection *conn = (struct redis_connection *)_conn; + const struct dict_commit_result result = { -1, reason }; const struct redis_dict_reply *reply; conn->dict->db_id_set = FALSE; conn->dict->connected = FALSE; - connection_disconnect(_conn); + connection_disconnect(&conn->conn); array_foreach(&conn->dict->replies, reply) - redis_callback(conn->dict, reply, -1); + redis_callback(conn->dict, reply, &result); array_clear(&conn->dict->replies); array_clear(&conn->dict->input_states); @@ -112,11 +114,19 @@ static void redis_conn_destroy(struct connection *_conn) io_loop_stop(conn->dict->ioloop); } +static void redis_conn_destroy(struct connection *_conn) +{ + struct redis_connection *conn = (struct redis_connection *)_conn; + + redis_disconnected(conn, connection_disconnect_reason(_conn)); +} + static void redis_dict_wait_timeout(struct redis_dict *dict) { - i_error("redis: Commit timed out in %u.%03u secs", + const char *reason = t_strdup_printf( + "redis: Commit timed out in %u.%03u secs", dict->timeout_msecs/1000, dict->timeout_msecs%1000); - redis_conn_destroy(&dict->conn.conn); + redis_disconnected(&dict->conn, reason); } static void redis_wait(struct redis_dict *dict) @@ -142,7 +152,7 @@ static void redis_wait(struct redis_dict *dict) dict->prev_ioloop = NULL; } -static int redis_input_get(struct redis_connection *conn) +static int redis_input_get(struct redis_connection *conn, const char **error_r) { const unsigned char *data; size_t size; @@ -162,8 +172,8 @@ static int redis_input_get(struct redis_connection *conn) return 1; } if (line[0] != '$' || str_to_uint(line+1, &conn->bytes_left) < 0) { - i_error("redis: Unexpected input (wanted $size): %s", - line); + *error_r = t_strdup_printf( + "redis: Unexpected input (wanted $size): %s", line); return -1; } conn->bytes_left += 2; /* include trailing CRLF */ @@ -190,7 +200,8 @@ static int redis_input_get(struct redis_connection *conn) return 1; } -static int redis_conn_input_more(struct redis_connection *conn) +static int +redis_conn_input_more(struct redis_connection *conn, const char **error_r) { struct redis_dict *dict = conn->dict; struct redis_dict_reply *reply; @@ -204,12 +215,13 @@ static int redis_conn_input_more(struct redis_connection *conn) line = i_stream_next_line(conn->conn.input); if (line == NULL) return 0; - i_error("redis: Unexpected input (expected nothing): %s", line); + *error_r = t_strdup_printf( + "redis: Unexpected input (expected nothing): %s", line); return -1; } state = states[0]; if (state == REDIS_INPUT_STATE_GET) - return redis_input_get(conn); + return redis_input_get(conn, error_r); line = i_stream_next_line(conn->conn.input); if (line == NULL) @@ -232,7 +244,8 @@ static int redis_conn_input_more(struct redis_connection *conn) reply = array_idx_modifiable(&dict->replies, 0); i_assert(reply->reply_count > 0); if (reply->reply_count != num_replies) { - i_error("redis: EXEC expected %u replies, not %u", + *error_r = t_strdup_printf( + "redis: EXEC expected %u replies, not %u", reply->reply_count, num_replies); return -1; } @@ -244,7 +257,8 @@ static int redis_conn_input_more(struct redis_connection *conn) reply = array_idx_modifiable(&dict->replies, 0); i_assert(reply->reply_count > 0); if (--reply->reply_count == 0) { - redis_callback(dict, reply, 1); + const struct dict_commit_result result = { 1, NULL }; + redis_callback(dict, reply, &result); array_delete(&dict->replies, 0, 1); /* if we're running in a dict-ioloop, we're handling a synchronous commit and need to stop now */ @@ -254,30 +268,29 @@ static int redis_conn_input_more(struct redis_connection *conn) } return 1; } - i_error("redis: Unexpected input (state=%d): %s", state, line); + *error_r = t_strdup_printf("redis: Unexpected input (state=%d): %s", state, line); return -1; } static void redis_conn_input(struct connection *_conn) { struct redis_connection *conn = (struct redis_connection *)_conn; + const char *error; int ret; switch (i_stream_read(_conn->input)) { case 0: return; case -1: - if (conn->dict->ioloop != NULL) - i_error("redis: Disconnected unexpectedly"); - redis_conn_destroy(_conn); + redis_disconnected(conn, i_stream_get_error(_conn->input)); return; default: break; } - while ((ret = redis_conn_input_more(conn)) > 0) ; + while ((ret = redis_conn_input_more(conn, &error)) > 0) ; if (ret < 0) - redis_conn_destroy(_conn); + redis_disconnected(conn, error); } static void redis_conn_connected(struct connection *_conn, bool success) @@ -448,9 +461,10 @@ static void redis_dict_deinit(struct dict *_dict) static void redis_dict_lookup_timeout(struct redis_dict *dict) { - i_error("redis: Lookup timed out in %u.%03u secs", + const char *reason = t_strdup_printf( + "redis: Lookup timed out in %u.%03u secs", dict->timeout_msecs/1000, dict->timeout_msecs%1000); - redis_conn_destroy(&dict->conn.conn); + redis_disconnected(&dict->conn, reason); } static const char * @@ -541,8 +555,8 @@ static int redis_dict_lookup(struct dict *_dict, pool_t pool, const char *key, if (!dict->conn.value_received) { /* we failed in some way. make sure we disconnect since the connection state isn't known anymore */ - redis_conn_destroy(&dict->conn.conn); *error_r = "redis: Communication failure"; + redis_disconnected(&dict->conn, *error_r); return -1; } if (dict->conn.value_not_found) @@ -587,15 +601,16 @@ redis_transaction_commit(struct dict_transaction_context *_ctx, bool async, struct redis_dict *dict = (struct redis_dict *)_ctx->dict; struct redis_dict_reply *reply; unsigned int i; - int ret = 1; + struct dict_commit_result result = { .ret = 1 }; i_assert(dict->transaction_open); dict->transaction_open = FALSE; - if (ctx->failed) { + if (ctx->error != NULL) { /* make sure we're disconnected */ - redis_conn_destroy(&dict->conn.conn); - ret = -1; + redis_disconnected(&dict->conn, ctx->error); + result.ret = -1; + result.error = ctx->error; } else if (_ctx->changed) { i_assert(ctx->cmd_count > 0); @@ -614,7 +629,8 @@ redis_transaction_commit(struct dict_transaction_context *_ctx, bool async, } redis_wait(dict); } - callback(ret, context); + callback(&result, context); + i_free(ctx->error); i_free(ctx); } @@ -628,9 +644,9 @@ static void redis_transaction_rollback(struct dict_transaction_context *_ctx) i_assert(dict->transaction_open); dict->transaction_open = FALSE; - if (ctx->failed) { + if (ctx->error != NULL) { /* make sure we're disconnected */ - redis_conn_destroy(&dict->conn.conn); + redis_disconnected(&dict->conn, ctx->error); } else if (_ctx->changed) { o_stream_nsend_str(dict->conn.conn.output, "*1\r\n$7\r\nDISCARD\r\n"); @@ -638,6 +654,7 @@ static void redis_transaction_rollback(struct dict_transaction_context *_ctx) reply->reply_count = 1; redis_input_state_add(dict, REDIS_INPUT_STATE_DISCARD); } + i_free(ctx->error); i_free(ctx); } @@ -645,11 +662,10 @@ static int redis_check_transaction(struct redis_dict_transaction_context *ctx) { struct redis_dict *dict = (struct redis_dict *)ctx->ctx.dict; - if (ctx->failed) + if (ctx->error != NULL) return -1; if (!dict->connected) { - /* disconnected during transaction */ - ctx->failed = TRUE; + ctx->error = i_strdup("Disconnected during transaction"); return -1; } if (ctx->ctx.changed) @@ -658,7 +674,8 @@ static int redis_check_transaction(struct redis_dict_transaction_context *ctx) redis_input_state_add(dict, REDIS_INPUT_STATE_MULTI); if (o_stream_send_str(dict->conn.conn.output, "*1\r\n$5\r\nMULTI\r\n") < 0) { - ctx->failed = TRUE; + ctx->error = i_strdup_printf("write() failed: %s", + o_stream_get_error(dict->conn.conn.output)); return -1; } return 0; @@ -700,8 +717,10 @@ static void redis_set(struct dict_transaction_context *_ctx, redis_input_state_add(dict, REDIS_INPUT_STATE_MULTI); ctx->cmd_count++; redis_append_expire(ctx, cmd, key); - if (o_stream_send(dict->conn.conn.output, str_data(cmd), str_len(cmd)) < 0) - ctx->failed = TRUE; + if (o_stream_send(dict->conn.conn.output, str_data(cmd), str_len(cmd)) < 0) { + ctx->error = i_strdup_printf("write() failed: %s", + o_stream_get_error(dict->conn.conn.output)); + } } static void redis_unset(struct dict_transaction_context *_ctx, @@ -718,8 +737,10 @@ static void redis_unset(struct dict_transaction_context *_ctx, key = redis_dict_get_full_key(dict, key); cmd = t_strdup_printf("*2\r\n$3\r\nDEL\r\n$%u\r\n%s\r\n", (unsigned int)strlen(key), key); - if (o_stream_send_str(dict->conn.conn.output, cmd) < 0) - ctx->failed = TRUE; + if (o_stream_send_str(dict->conn.conn.output, cmd) < 0) { + ctx->error = i_strdup_printf("write() failed: %s", + o_stream_get_error(dict->conn.conn.output)); + } redis_input_state_add(dict, REDIS_INPUT_STATE_MULTI); ctx->cmd_count++; } @@ -745,8 +766,10 @@ static void redis_atomic_inc(struct dict_transaction_context *_ctx, redis_input_state_add(dict, REDIS_INPUT_STATE_MULTI); ctx->cmd_count++; redis_append_expire(ctx, cmd, key); - if (o_stream_send(dict->conn.conn.output, str_data(cmd), str_len(cmd)) < 0) - ctx->failed = TRUE; + if (o_stream_send(dict->conn.conn.output, str_data(cmd), str_len(cmd)) < 0) { + ctx->error = i_strdup_printf("write() failed: %s", + o_stream_get_error(dict->conn.conn.output)); + } } struct dict dict_driver_redis = { diff --git a/src/lib-dict/dict-sql.c b/src/lib-dict/dict-sql.c index 2ba4179165..840e35492f 100644 --- a/src/lib-dict/dict-sql.c +++ b/src/lib-dict/dict-sql.c @@ -68,7 +68,7 @@ struct sql_dict_transaction_context { dict_transaction_commit_callback_t *async_callback; void *async_context; - unsigned int failed:1; + char *error; }; static struct sql_db_cache *dict_sql_db_cache; @@ -764,6 +764,7 @@ static void sql_dict_transaction_free(struct sql_dict_transaction_context *ctx) if (ctx->inc_row_pool != NULL) pool_unref(&ctx->inc_row_pool); i_free(ctx->prev_inc_key); + i_free(ctx->error); i_free(ctx); } @@ -784,17 +785,20 @@ static void sql_dict_transaction_commit_callback(const char *error, struct sql_dict_transaction_context *ctx) { - int ret; + struct dict_commit_result result; + memset(&result, 0, sizeof(result)); if (error == NULL) - ret = sql_dict_transaction_has_nonexistent(ctx) ? 0 : 1; + result.ret = sql_dict_transaction_has_nonexistent(ctx) ? 0 : 1; else { - i_error("sql dict: commit failed: %s", error); - ret = -1; + result.error = t_strdup_printf("sql dict: commit failed: %s", error); + result.ret = -1; } if (ctx->async_callback != NULL) - ctx->async_callback(ret, ctx->async_context); + ctx->async_callback(&result, ctx->async_context); + else if (result.ret < 0) + i_error("%s", result.error); sql_dict_transaction_free(ctx); } @@ -806,35 +810,39 @@ sql_dict_transaction_commit(struct dict_transaction_context *_ctx, bool async, struct sql_dict_transaction_context *ctx = (struct sql_dict_transaction_context *)_ctx; const char *error; - int ret = 1; + struct dict_commit_result result; + + memset(&result, 0, sizeof(result)); + result.ret = -1; + result.error = t_strdup(ctx->error); if (ctx->prev_inc_map != NULL) sql_dict_prev_inc_flush(ctx); - if (ctx->failed) { + if (ctx->error != NULL) { sql_transaction_rollback(&ctx->sql_ctx); - ret = -1; } else if (!_ctx->changed) { /* nothing changed, no need to commit */ sql_transaction_rollback(&ctx->sql_ctx); + result.ret = 1; } else if (async) { ctx->async_callback = callback; ctx->async_context = context; sql_transaction_commit(&ctx->sql_ctx, sql_dict_transaction_commit_callback, ctx); return; + } else if (sql_transaction_commit_s(&ctx->sql_ctx, &error) < 0) { + result.error = t_strdup_printf( + "sql dict: commit failed: %s", error); } else { - if (sql_transaction_commit_s(&ctx->sql_ctx, &error) < 0) { - i_error("sql dict: commit failed: %s", error); - ret = -1; - } else { - if (sql_dict_transaction_has_nonexistent(ctx)) - ret = 0; - } + if (sql_dict_transaction_has_nonexistent(ctx)) + result.ret = 0; + else + result.ret = 1; } sql_dict_transaction_free(ctx); - callback(ret, context); + callback(&result, context); } static void sql_dict_transaction_rollback(struct dict_transaction_context *_ctx) @@ -985,10 +993,13 @@ static void sql_dict_set(struct dict_transaction_context *_ctx, struct dict_sql_build_query_field field; const char *query, *error; + if (ctx->error != NULL) + return; + map = sql_dict_find_map(dict, key, &values); if (map == NULL) { - i_error("sql dict set: Invalid/unmapped key: %s", key); - ctx->failed = TRUE; + ctx->error = i_strdup_printf( + "dict-sql: Invalid/unmapped key: %s", key); return; } @@ -1006,9 +1017,8 @@ static void sql_dict_set(struct dict_transaction_context *_ctx, build.key1 = key[0]; if (sql_dict_set_query(&build, &query, &error) < 0) { - i_error("dict-sql: Failed to set %s=%s: %s", - key, value, error); - ctx->failed = TRUE; + ctx->error = i_strdup_printf("dict-sql: Failed to set %s=%s: %s", + key, value, error); } else { sql_update(ctx->sql_ctx, query); } @@ -1025,21 +1035,23 @@ static void sql_dict_unset(struct dict_transaction_context *_ctx, string_t *query = t_str_new(256); const char *error; + if (ctx->error != NULL) + return; + if (ctx->prev_inc_map != NULL) sql_dict_prev_inc_flush(ctx); map = sql_dict_find_map(dict, key, &values); if (map == NULL) { - i_error("sql dict unset: Invalid/unmapped key: %s", key); - ctx->failed = TRUE; + ctx->error = i_strdup_printf("dict-sql: Invalid/unmapped key: %s", key); return; } str_printfa(query, "DELETE FROM %s", map->table); if (sql_dict_where_build(dict, map, &values, key[0], SQL_DICT_RECURSE_NONE, query, &error) < 0) { - i_error("dict-sql: Failed to delete %s: %s", key, error); - ctx->failed = TRUE; + ctx->error = i_strdup_printf( + "dict-sql: Failed to delete %s: %s", key, error); } else { sql_update(ctx->sql_ctx, str_c(query)); } @@ -1071,6 +1083,9 @@ static void sql_dict_atomic_inc_real(struct sql_dict_transaction_context *ctx, struct dict_sql_build_query_field field; const char *query, *error; + if (ctx->error != NULL) + return; + map = sql_dict_find_map(dict, key, &values); i_assert(map != NULL); @@ -1086,8 +1101,8 @@ static void sql_dict_atomic_inc_real(struct sql_dict_transaction_context *ctx, build.inc = TRUE; if (sql_dict_update_query(&build, &query, &error) < 0) { - i_error("dict-sql: Failed to increase %s: %s", key, error); - ctx->failed = TRUE; + ctx->error = i_strdup_printf( + "dict-sql: Failed to increase %s: %s", key, error); } else { sql_update_get_rows(ctx->sql_ctx, query, sql_dict_next_inc_row(ctx)); @@ -1146,10 +1161,13 @@ static void sql_dict_atomic_inc(struct dict_transaction_context *_ctx, const struct dict_sql_map *map; ARRAY_TYPE(const_string) values; + if (ctx->error != NULL) + return; + map = sql_dict_find_map(dict, key, &values); if (map == NULL) { - i_error("sql dict atomic inc: Invalid/unmapped key: %s", key); - ctx->failed = TRUE; + ctx->error = i_strdup_printf( + "sql dict atomic inc: Invalid/unmapped key: %s", key); return; } @@ -1186,8 +1204,8 @@ static void sql_dict_atomic_inc(struct dict_transaction_context *_ctx, field->value = t_strdup_printf("%lld", diff); if (sql_dict_update_query(&build, &query, &error) < 0) { - i_error("dict-sql: Failed to increase %s: %s", key, error); - ctx->failed = TRUE; + ctx->error = i_strdup_printf( + "dict-sql: Failed to increase %s: %s", key, error); } else { sql_update_get_rows(ctx->sql_ctx, query, sql_dict_next_inc_row(ctx)); diff --git a/src/lib-dict/dict.c b/src/lib-dict/dict.c index e2ddff2e49..be3e822b42 100644 --- a/src/lib-dict/dict.c +++ b/src/lib-dict/dict.c @@ -187,21 +187,35 @@ struct dict_transaction_context *dict_transaction_begin(struct dict *dict) return dict->v.transaction_init(dict); } -static void dict_transaction_commit_sync_callback(int ret, void *context) +struct dict_commit_sync_result { + int ret; + char *error; +}; + +static void +dict_transaction_commit_sync_callback(const struct dict_commit_result *result, + void *context) { - int *ret_p = context; - *ret_p = ret; + struct dict_commit_sync_result *sync_result = context; + + sync_result->ret = result->ret; + sync_result->error = i_strdup(result->error); } -int dict_transaction_commit(struct dict_transaction_context **_ctx) +int dict_transaction_commit(struct dict_transaction_context **_ctx, + const char **error_r) { struct dict_transaction_context *ctx = *_ctx; - int ret; + struct dict_commit_sync_result result; *_ctx = NULL; + + memset(&result, 0, sizeof(result)); ctx->dict->v.transaction_commit(ctx, FALSE, - dict_transaction_commit_sync_callback, &ret); - return ret; + dict_transaction_commit_sync_callback, &result); + *error_r = t_strdup(result.error); + i_free(result.error); + return result.ret; } void dict_transaction_commit_async(struct dict_transaction_context **_ctx, diff --git a/src/lib-dict/dict.h b/src/lib-dict/dict.h index 406f8209d7..4553102e1b 100644 --- a/src/lib-dict/dict.h +++ b/src/lib-dict/dict.h @@ -44,10 +44,17 @@ struct dict_lookup_result { const char *error; }; +struct dict_commit_result { + int ret; + const char *error; +}; + typedef void dict_lookup_callback_t(const struct dict_lookup_result *result, void *context); typedef void dict_iterate_callback_t(void *context); -typedef void dict_transaction_commit_callback_t(int ret, void *context); +typedef void +dict_transaction_commit_callback_t(const struct dict_commit_result *result, + void *context); void dict_driver_register(struct dict *driver); void dict_driver_unregister(struct dict *driver); @@ -102,7 +109,8 @@ int dict_iterate_deinit(struct dict_iterate_context **ctx, const char **error_r) struct dict_transaction_context *dict_transaction_begin(struct dict *dict); /* Commit the transaction. Returns 1 if ok, 0 if dict_atomic_inc() was used on a nonexistent key, -1 if failed. */ -int dict_transaction_commit(struct dict_transaction_context **ctx); +int dict_transaction_commit(struct dict_transaction_context **ctx, + const char **error_r); /* Commit the transaction, but don't wait to see if it finishes successfully. If callback isn't NULL, it's called eventually. If it's not called by the time you want to deinitialize dict, call dict_flush() to wait for the diff --git a/src/lib-fs/fs-dict.c b/src/lib-fs/fs-dict.c index 9132fcaf85..4f0fdc389b 100644 --- a/src/lib-fs/fs-dict.c +++ b/src/lib-fs/fs-dict.c @@ -191,6 +191,7 @@ static int fs_dict_write_stream_finish(struct fs_file *_file, bool success) struct dict_fs_file *file = (struct dict_fs_file *)_file; struct dict_fs *fs = (struct dict_fs *)_file->fs; struct dict_transaction_context *trans; + const char *error; o_stream_destroy(&_file->output); if (!success) @@ -217,9 +218,9 @@ static int fs_dict_write_stream_finish(struct fs_file *_file, bool success) dict_set(trans, file->key, str_c(base64)); } } - if (dict_transaction_commit(&trans) < 0) { + if (dict_transaction_commit(&trans, &error) < 0) { errno = EIO; - fs_set_error(_file->fs, "Dict transaction commit failed"); + fs_set_error(_file->fs, "Dict transaction commit failed: %s", error); return -1; } return 1; @@ -242,12 +243,13 @@ static int fs_dict_delete(struct fs_file *_file) struct dict_fs_file *file = (struct dict_fs_file *)_file; struct dict_fs *fs = (struct dict_fs *)_file->fs; struct dict_transaction_context *trans; + const char *error; trans = dict_transaction_begin(fs->dict); dict_unset(trans, file->key); - if (dict_transaction_commit(&trans) < 0) { + if (dict_transaction_commit(&trans, &error) < 0) { errno = EIO; - fs_set_error(_file->fs, "Dict transaction commit failed"); + fs_set_error(_file->fs, "Dict transaction commit failed: %s", error); return -1; } return 0; diff --git a/src/lib-storage/index/index-transaction.c b/src/lib-storage/index/index-transaction.c index 4c7b42cc3b..d11ae410c3 100644 --- a/src/lib-storage/index/index-transaction.c +++ b/src/lib-storage/index/index-transaction.c @@ -26,20 +26,23 @@ index_transaction_index_commit(struct mail_index_transaction *index_trans, struct mailbox_transaction_context *t = MAIL_STORAGE_CONTEXT(index_trans); struct index_mailbox_sync_pvt_context *pvt_sync_ctx = NULL; + const char *error; int ret = 0; if (t->nontransactional_changes) t->changes->changed = TRUE; if (t->attr_pvt_trans != NULL) { - if (dict_transaction_commit(&t->attr_pvt_trans) < 0) { - mail_storage_set_internal_error(t->box->storage); + if (dict_transaction_commit(&t->attr_pvt_trans, &error) < 0) { + mail_storage_set_critical(t->box->storage, + "Dict private transaction commit failed: %s", error); ret = -1; } } if (t->attr_shared_trans != NULL) { - if (dict_transaction_commit(&t->attr_shared_trans) < 0) { - mail_storage_set_internal_error(t->box->storage); + if (dict_transaction_commit(&t->attr_shared_trans, &error) < 0) { + mail_storage_set_critical(t->box->storage, + "Dict shared transaction commit failed: %s", error); ret = -1; } } diff --git a/src/plugins/acl/acl-lookup-dict.c b/src/plugins/acl/acl-lookup-dict.c index 34e415cd47..d70701e861 100644 --- a/src/plugins/acl/acl-lookup-dict.c +++ b/src/plugins/acl/acl-lookup-dict.c @@ -217,8 +217,8 @@ acl_lookup_dict_rebuild_update(struct acl_lookup_dict *dict, oldi++; } } - if (dict_transaction_commit(&dt) < 0) { - i_error("acl: dict commit failed"); + if (dict_transaction_commit(&dt, &error) < 0) { + i_error("acl: dict commit failed: %s", error); return -1; } return 0; diff --git a/src/plugins/expire/doveadm-expire.c b/src/plugins/expire/doveadm-expire.c index 51e0dd84b3..6ce0d4d563 100644 --- a/src/plugins/expire/doveadm-expire.c +++ b/src/plugins/expire/doveadm-expire.c @@ -358,8 +358,8 @@ static void doveadm_expire_mail_cmd_deinit(struct doveadm_mail_cmd_context *ctx) if (dict_iterate_deinit(&ectx->iter, &error) < 0) i_error("expire: Dictionary iteration failed: %s", error); } - if (dict_transaction_commit(&ectx->trans) < 0) - i_error("expire: Dictionary commit failed"); + if (dict_transaction_commit(&ectx->trans, &error) < 0) + i_error("expire: Dictionary commit failed: %s", error); dict_deinit(&ectx->dict); hash_table_destroy(&ectx->user_states); diff --git a/src/plugins/expire/expire-plugin.c b/src/plugins/expire/expire-plugin.c index b1fc082719..0a3b823813 100644 --- a/src/plugins/expire/expire-plugin.c +++ b/src/plugins/expire/expire-plugin.c @@ -158,11 +158,12 @@ expire_update(struct mailbox *box, const char *key, time_t timestamp) struct dict_transaction_context *dctx; struct mail_index_transaction *trans; struct expire_mail_index_header hdr; + const char *error; dctx = dict_transaction_begin(euser->db); dict_set(dctx, key, dec2str(timestamp)); - if (dict_transaction_commit(&dctx) < 0) - i_error("expire: dict commit failed"); + if (dict_transaction_commit(&dctx, &error) < 0) + i_error("expire: dict commit failed: %s", error); else if (euser->expire_cache) { memset(&hdr, 0, sizeof(hdr)); hdr.timestamp = timestamp; diff --git a/src/plugins/last-login/last-login-plugin.c b/src/plugins/last-login/last-login-plugin.c index e091dc8f11..0673794046 100644 --- a/src/plugins/last-login/last-login-plugin.c +++ b/src/plugins/last-login/last-login-plugin.c @@ -47,7 +47,9 @@ static void last_login_user_deinit(struct mail_user *user) luser->module_ctx.super.deinit(user); } -static void last_login_dict_commit(int ret ATTR_UNUSED, void *context) +static void +last_login_dict_commit(const struct dict_commit_result *result ATTR_UNUSED, + void *context) { struct mail_user *user = context; struct last_login_user *luser = LAST_LOGIN_USER_CONTEXT(user); diff --git a/src/plugins/quota-clone/quota-clone-plugin.c b/src/plugins/quota-clone/quota-clone-plugin.c index c666793927..f4f6bd333f 100644 --- a/src/plugins/quota-clone/quota-clone-plugin.c +++ b/src/plugins/quota-clone/quota-clone-plugin.c @@ -41,6 +41,7 @@ static void quota_clone_flush(struct mailbox *box) struct quota_root_iter *iter; struct quota_root *root; uint64_t bytes_value, count_value, limit; + const char *error; /* we'll clone the first quota root */ iter = quota_root_iter_init(box); @@ -70,8 +71,8 @@ static void quota_clone_flush(struct mailbox *box) t_strdup_printf("%llu", (unsigned long long)bytes_value)); dict_set(trans, DICT_QUOTA_CLONE_COUNT_PATH, t_strdup_printf("%llu", (unsigned long long)count_value)); - if (dict_transaction_commit(&trans) < 0) - i_error("quota_clone_plugin: Failed to commit dict update"); + if (dict_transaction_commit(&trans, &error) < 0) + i_error("quota_clone_plugin: Failed to commit dict update: %s", error); else qbox->quota_changed = FALSE; } diff --git a/src/plugins/quota/quota-dict.c b/src/plugins/quota/quota-dict.c index 3fdfe113c4..987611ad62 100644 --- a/src/plugins/quota/quota-dict.c +++ b/src/plugins/quota/quota-dict.c @@ -198,16 +198,18 @@ static void dict_quota_recalc_timeout(struct dict_quota_root *root) (void)dict_quota_count(root, TRUE, &value); } -static void dict_quota_update_callback(int ret, void *context) +static void dict_quota_update_callback(const struct dict_commit_result *result, + void *context) { struct dict_quota_root *root = context; - if (ret == 0) { + if (result->ret == 0) { /* row doesn't exist, need to recalculate it */ if (root->to_update == NULL) root->to_update = timeout_add_short(0, dict_quota_recalc_timeout, root); - } else if (ret < 0) { - i_error("dict quota: Quota update failed, it's now desynced"); + } else if (result->ret < 0) { + i_error("dict quota: Quota update failed: %s " + "- Quota is now desynced", result->error); } } diff --git a/src/plugins/quota/quota.c b/src/plugins/quota/quota.c index 47101a3324..23782690a7 100644 --- a/src/plugins/quota/quota.c +++ b/src/plugins/quota/quota.c @@ -709,7 +709,7 @@ int quota_set_resource(struct quota_root *root, const char *name, uint64_t value, const char **error_r) { struct dict_transaction_context *trans; - const char *key; + const char *key, *error; if (root->set->limit_set == NULL) { *error_r = MAIL_ERRSTR_NO_PERMISSION; @@ -742,7 +742,8 @@ int quota_set_resource(struct quota_root *root, const char *name, trans = dict_transaction_begin(root->limit_set_dict); key = t_strdup_printf(QUOTA_LIMIT_SET_PATH"%s", key); dict_set(trans, key, dec2str(value)); - if (dict_transaction_commit(&trans) < 0) { + if (dict_transaction_commit(&trans, &error) < 0) { + i_error("dict_transaction_commit() failed: %s", error); *error_r = "Internal quota limit update error"; return -1; }