git » libjio » commit 16be747

Nowadays, we unlock operations as we commit them. However, this could lead to potential issues if we have an error in the middle of a commit when a partially-overlapping transaction gets in the way.

Nowadays, we unlock operations as we commit them. However, this could lead to potential issues if we have an error in the middle of a commit when a partially-overlapping transaction gets in the way.

Nowadays, we unlock operations as we commit them. However, this could lead to
potential issues if we have an error in the middle of a commit when a
partially-overlapping transaction gets in the way.

The problem is that rollback gets done without locking because it assumes the
operations are already locked (by the caller commit), but in the case of
partially applied transactions they won't be, so it allows another transaction
to step into, leaving things in an inconsistent state.

This also leads to recovery and ordering problems.

The patch fixes this by doing all unlock at once, after the transaction has
been decided.

diff --git a/trans.c b/trans.c
index 00e9b34..f1ba9c7 100644
--- a/trans.c
+++ b/trans.c
@@ -355,10 +355,6 @@ ssize_t jtrans_commit(struct jtrans *ts)
 	written = 0;
 	for (op = ts->op; op != NULL; op = op->next) {
 		rv = spwrite(ts->fs->fd, op->buf, op->len, op->offset);
-
-		plockf(ts->fs->fd, F_UNLOCK, op->offset, op->len);
-		op->locked = 0;
-
 		if (rv != op->len)
 			goto rollback_exit;
 
@@ -417,9 +413,17 @@ unlink_exit:
 	}
 
 	close(fd);
-	for (op = ts->op; op != NULL; op = op->next) {
-		if (op->locked)
-			plockf(ts->fs->fd, F_UNLOCK, op->offset, op->len);
+
+	/* always unlock everything at the end; otherwise we could have
+	 * half-overlapping transactions applying simultaneously, and if
+	 * anything goes wrong it's possible to break consistency */
+	if (!(ts->flags & J_NOLOCK)) {
+		for (op = ts->op; op != NULL; op = op->next) {
+			if (op->locked) {
+				plockf(ts->fs->fd, F_UNLOCK,
+						op->offset, op->len);
+			}
+		}
 	}
 
 exit: