New save/load mutex strategy

A complete save or reload operation is made up of several cooperating lower
level function calls. In stead of protecting the lower level read or write calls
on the data files, the entire save or load operation is protected in order to
ensure its integrity.

Thus mutex protection has been moved from the level:

	io_load_data(), io_merge_data(), new_data(),

to two functions at the higher level:

	io_save_cal(), io_reload_data().

The protection includes pre- and post-hooks.

The function io_load_data() needs no protection when calcurse starts up
(threads not yet started) or runs in non-interactive mode (no threads involved).

Signed-off-by: Lars Henriksen <LarsHenriksen@get2net.dk>
Signed-off-by: Lukas Fleischer <lfleischer@calcurse.org>

1 files changed, 28 insertions, 39 deletions

diff --git a/src/io.c b/src/io.c
index 9cfae0f..72b2bba 100644
--- a/src/io.c
+++ b/src/io.c
@@ -371,10 +371,8 @@ static void io_merge_data(void)
 	asprintf(&path_apts_new, "%s%s", path_apts, new_ext);
 	asprintf(&path_todo_new, "%s%s", path_todo, new_ext);
 
-	io_mutex_lock();
 	io_save_apts(path_apts_new);
 	io_save_todo(path_todo_new);
-	io_mutex_unlock();
 
 	/*
 	 * We do not directly write to the data files here; however, the
@@ -457,15 +455,13 @@ static int new_data()
 	char sha1_new[SHA1_DIGESTLEN * 2 + 1];
 	int ret = NONEW;
 
-	io_mutex_lock();
-
 	if (io_compute_hash(path_apts, sha1_new)) {
 		if (strncmp(sha1_new, apts_sha1, SHA1_DIGESTLEN * 2) != 0) {
 			ret |= APTS;
 		}
 	} else {
 		ret = NOKNOW;
-		goto cleanup;
+		goto exit;
 	}
 
 	if (io_compute_hash(path_todo, sha1_new)) {
@@ -474,11 +470,9 @@ static int new_data()
 		}
 	} else {
 		ret = NOKNOW;
-		goto cleanup;
+		goto exit;
 	}
-
-cleanup:
-	io_mutex_unlock();
+   exit:
 	return ret;
 }
 
@@ -493,46 +487,43 @@ cleanup:
  */
 int io_save_cal(enum save_type s_t)
 {
-	int ret;
+	int ret, new;
 
 	if (read_only)
 		return IO_SAVE_CANCEL;
 
-	if ((ret = new_data()) == NOKNOW)
-		return IO_SAVE_ERROR;
-
-	if (ret) { /* New data */
-		if (s_t == periodic)
-			return IO_SAVE_CANCEL;
+	io_mutex_lock();
+	if ((new = new_data()) == NOKNOW) {
+		ret = IO_SAVE_ERROR;
+		goto cleanup;
+	}
+	if (new) { /* New data */
+		if (s_t == periodic) {
+			ret = IO_SAVE_CANCEL;
+			goto cleanup;
+		}
 		/* Interactively decide what to do. */
 		if ((ret = resolve_save_conflict()))
-			return ret;
+			goto cleanup;
 	} else /* No new data */
-		if (!io_get_modified())
-			return IO_SAVE_NOOP;
+		if (!io_get_modified()) {
+			ret = IO_SAVE_NOOP;
+			goto cleanup;
+		}
 
 	ret = IO_SAVE_CTINUE;
-
 	run_hook("pre-save");
-	io_mutex_lock();
-
-	if (!io_save_todo(path_todo)) {
-		ret = IO_SAVE_ERROR;
-		goto cleanup;
-	}
-	if (!io_save_apts(path_apts)) {
+	if (io_save_todo(path_todo) &&
+	    io_save_apts(path_apts)) {
+		io_compute_hash(path_apts, apts_sha1);
+		io_compute_hash(path_todo, todo_sha1);
+		io_unset_modified();
+	} else
 		ret = IO_SAVE_ERROR;
-		goto cleanup;
-	}
-
-	io_unset_modified();
-
-	io_compute_hash(path_apts, apts_sha1);
-	io_compute_hash(path_todo, todo_sha1);
+	run_hook("post-save");
 
 cleanup:
 	io_mutex_unlock();
-	run_hook("post-save");
 	return ret;
 }
 
@@ -833,8 +824,6 @@ int io_load_data(struct item_filter *filter, int force)
 	else
 		force = new_data();
 
-	io_mutex_lock();
-
 	if (force & APTS) {
 		apoint_llist_free();
 		event_llist_free();
@@ -854,7 +843,6 @@ int io_load_data(struct item_filter *filter, int force)
 
 	io_unset_modified();
 
-	io_mutex_unlock();
 	run_hook("post-load");
 	return force;
 }
@@ -868,6 +856,7 @@ int io_reload_data(void)
 	int load = NOFORCE;
 	int ret = IO_RELOAD_LOAD;
 
+	io_mutex_lock();
 	if (io_get_modified()) {
 		const char *msg_um_prefix =
 				_("Screen data have changed and will be lost:");
@@ -900,8 +889,8 @@ int io_reload_data(void)
 	load = io_load_data(NULL, load);
 	if (load == NONEW)
 		ret = IO_RELOAD_NOOP;
-
 cleanup:
+	io_mutex_unlock();
 	mem_free(msg_um_asktype);
 	return ret;
 }