From f46e648f568c38b57ada874cea87e561e3a235c7 Mon Sep 17 00:00:00 2001 From: Kevin McCarthy Date: Sun, 13 Nov 2016 20:02:35 -0800 Subject: Compress: pull the lock/unlock operations into the open,close,sync operations. Some operations, such as open_append and sync, need an exclusive lock across a longer period than a single compress/decompress. Remove it from the execute_command and pull into the outer callers. Store lock information inside compress_info. Sync and check_mailbox need more fixes, which will be addressed in subsequent patches. --- compress.c | 123 ++++++++++++++++++++++++++++++++++++------------------------- 1 file changed, 73 insertions(+), 50 deletions(-) (limited to 'compress.c') diff --git a/compress.c b/compress.c index a588a0c7..681fffcf 100644 --- a/compress.c +++ b/compress.c @@ -49,13 +49,14 @@ typedef struct const char *open; /* open-hook command */ off_t size; /* size of the compressed file */ struct mx_ops *child_ops; /* callbacks of de-compressed file */ + int locked; /* if realpath is locked */ + FILE *lockfp; /* fp used for locking */ } COMPRESS_INFO; /** - * lock_mailbox - Try to lock a mailbox (exclusively) + * lock_realpath - Try to lock the ctx->realpath * @ctx: Mailbox to lock - * @fp: File pointer to the mailbox file * @excl: Lock exclusively? * * Try to (exclusively) lock the mailbox. If we succeed, then we mark the @@ -67,19 +68,35 @@ typedef struct * 0: Error (can't lock the file) */ static int -lock_mailbox (CONTEXT *ctx, FILE *fp, int excl) +lock_realpath (CONTEXT *ctx, int excl) { - if (!ctx || !fp) + if (!ctx) return 0; - int r = mx_lock_file (ctx->realpath, fileno (fp), excl, 1, 1); + COMPRESS_INFO *ci = ctx->compress_info; + if (!ci) + return 0; - if (r == 0) + if (ci->locked) + return 1; + + if (excl) + ci->lockfp = fopen (ctx->realpath, "a"); + else + ci->lockfp = fopen (ctx->realpath, "r"); + if (!ci->lockfp) { - ctx->locked = 1; + mutt_perror (ctx->realpath); + return 0; } + + int r = mx_lock_file (ctx->realpath, fileno (ci->lockfp), excl, 1, 1); + + if (r == 0) + ci->locked = 1; else if (excl == 0) { + safe_fclose (&ci->lockfp); ctx->readonly = 1; return 1; } @@ -88,25 +105,28 @@ lock_mailbox (CONTEXT *ctx, FILE *fp, int excl) } /** - * unlock_mailbox - Unlock a mailbox + * unlock_realpath - Unlock the ctx->realpath * @ctx: Mailbox to unlock - * @fp: File pointer to mailbox file * * Unlock a mailbox previously locked by lock_mailbox(). */ static void -unlock_mailbox (CONTEXT *ctx, FILE *fp) +unlock_realpath (CONTEXT *ctx) { - if (!ctx || !fp) + if (!ctx) + return; + + COMPRESS_INFO *ci = ctx->compress_info; + if (!ci) return; - if (!ctx->locked) + if (!ci->locked) return; - fflush (fp); + mx_unlock_file (ctx->realpath, fileno (ci->lockfp), 1); - mx_unlock_file (ctx->realpath, fileno (fp), 1); - ctx->locked = 0; + ci->locked = 0; + safe_fclose (&ci->lockfp); } /** @@ -270,6 +290,8 @@ mutt_free_compress_info (CONTEXT *ctx) FREE (&ci->close); FREE (&ci->append); + unlock_realpath (ctx); + FREE (&ctx->compress_info); } @@ -345,7 +367,6 @@ expand_command_str (const CONTEXT *ctx, const char *cmd, char *buf, int buflen) * execute_command - Run a system command * @ctx: Mailbox to work with * @command: Command string to execute - * @create_file: Should the tmp file be created? * @progress: Message to show the user * * Run the supplied command, taking care of all the Mutt requirements, @@ -356,11 +377,9 @@ expand_command_str (const CONTEXT *ctx, const char *cmd, char *buf, int buflen) * 0: Failure */ static int -execute_command (CONTEXT *ctx, const char *command, int create_file, const char *progress) +execute_command (CONTEXT *ctx, const char *command, const char *progress) { - FILE *fp; - int rc = 0; - int unlock = 0; + int rc = 1; char sys_cmd[HUGE_STRING]; if (!ctx || !command || !progress) @@ -369,25 +388,7 @@ execute_command (CONTEXT *ctx, const char *command, int create_file, const char if (!ctx->quiet) mutt_message (progress, ctx->realpath); - if (create_file) - fp = fopen (ctx->realpath, "a"); - else - fp = fopen (ctx->realpath, "r"); - if (!fp) - { - mutt_perror (ctx->realpath); - return 0; - } - mutt_block_signals(); - /* If we're creating the file, lock it exclusively */ - if (!lock_mailbox (ctx, fp, create_file)) - { - mutt_error (_("Unable to lock mailbox!")); - goto cleanup; - } - - unlock = 1; endwin(); fflush (stdout); @@ -395,18 +396,12 @@ execute_command (CONTEXT *ctx, const char *command, int create_file, const char if (mutt_system (sys_cmd) != 0) { + rc = 0; mutt_any_key_to_continue (NULL); mutt_error (_("Error executing: %s\n"), sys_cmd); - goto cleanup; } - rc = 1; - -cleanup: - if (unlock) - unlock_mailbox (ctx, fp); mutt_unblock_signals(); - safe_fclose (&fp); return rc; } @@ -438,10 +433,18 @@ open_mailbox (CONTEXT *ctx) goto or_fail; store_size (ctx); - int rc = execute_command (ctx, ci->open, 0, _("Decompressing %s")); + if (!lock_realpath (ctx, 0)) + { + mutt_error (_("Unable to lock mailbox!")); + goto or_fail; + } + + int rc = execute_command (ctx, ci->open, _("Decompressing %s")); if (rc == 0) goto or_fail; + unlock_realpath (ctx); + ctx->magic = mx_get_magic (ctx->path); if (ctx->magic == 0) { @@ -511,10 +514,18 @@ open_append_mailbox (CONTEXT *ctx, int flags) goto oa_fail2; } + /* Lock the realpath for the duration of the append. + * It will be unlocked in the close */ + if (!lock_realpath (ctx, 1)) + { + mutt_error (_("Unable to lock mailbox!")); + goto oa_fail2; + } + /* Open the existing mailbox, unless we are appending */ - if (!ci->append && (access (ctx->realpath, F_OK) == 0)) + if (!ci->append && (get_size (ctx->realpath) > 0)) { - int rc = execute_command (ctx, ci->open, 0, _("Decompressing %s")); + int rc = execute_command (ctx, ci->open, _("Decompressing %s")); if (rc == 0) { mutt_error (_("Compress command failed: %s"), ci->open); @@ -595,7 +606,7 @@ close_mailbox (CONTEXT *ctx) msg = _("Compressing %s..."); } - int rc = execute_command (ctx, append, 1, msg); + int rc = execute_command (ctx, append, msg); if (rc == 0) { mutt_any_key_to_continue (NULL); @@ -604,6 +615,8 @@ close_mailbox (CONTEXT *ctx) else remove (ctx->path); + unlock_realpath (ctx); + return 0; } @@ -829,10 +842,20 @@ mutt_comp_sync (CONTEXT *ctx) return -1; } - int rc = execute_command (ctx, ci->close, 1, _("Compressing %s")); + /* TODO: need to refactor sync so we can lock around the + * path sync as well as the compress operation */ + if (!lock_realpath (ctx, 1)) + { + mutt_error (_("Unable to lock mailbox!")); + return -1; + } + + int rc = execute_command (ctx, ci->close, _("Compressing %s")); if (rc == 0) return -1; + unlock_realpath (ctx); + store_size (ctx); return 0; -- cgit v1.2.3