From: Stefan Metzmacher <metze@samba.org>
Date: Mon, 27 Jun 2022 13:39:18 +0000 (+0000)
Subject: s3:g_lock: avoid calling g_lock_store() from g_lock_cleanup_dead()
X-Git-Tag: ldb-2.6.1~23
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=b865bb28abedd18d5db0ae3ea31908c75ee1129b;p=thirdparty%2Fsamba.git

s3:g_lock: avoid calling g_lock_store() from g_lock_cleanup_dead()

This matches the behavior of g_lock_cleanup_shared(), which also
only operates on the in memory struct g_lock.

We do a g_lock_store() later during g_lock_trylock() anyway
when we make any progress.

In the case we where a pending exclusive lock holder
we now force a g_lock_store() if g_lock_cleanup_dead()
removed the dead blocker.

This will be useful for the following changes...

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
---

diff --git a/source3/lib/g_lock.c b/source3/lib/g_lock.c
index f7fc5544d26..07958f30b5e 100644
--- a/source3/lib/g_lock.c
+++ b/source3/lib/g_lock.c
@@ -242,18 +242,15 @@ struct g_lock_ctx *g_lock_ctx_init(TALLOC_CTX *mem_ctx,
 	return ctx;
 }
 
-static NTSTATUS g_lock_cleanup_dead(
-	struct db_record *rec,
+static void g_lock_cleanup_dead(
 	struct g_lock *lck,
 	struct server_id *dead_blocker)
 {
-	bool modified = false;
 	bool exclusive_died;
-	NTSTATUS status = NT_STATUS_OK;
 	struct server_id_buf tmp;
 
 	if (dead_blocker == NULL) {
-		return NT_STATUS_OK;
+		return;
 	}
 
 	exclusive_died = server_id_equal(dead_blocker, &lck->exclusive);
@@ -262,7 +259,6 @@ static NTSTATUS g_lock_cleanup_dead(
 		DBG_DEBUG("Exclusive holder %s died\n",
 			  server_id_str_buf(lck->exclusive, &tmp));
 		lck->exclusive.pid = 0;
-		modified = true;
 	}
 
 	if (lck->num_shared != 0) {
@@ -276,19 +272,8 @@ static NTSTATUS g_lock_cleanup_dead(
 			DBG_DEBUG("Shared holder %s died\n",
 				  server_id_str_buf(shared, &tmp));
 			g_lock_del_shared(lck, 0);
-			modified = true;
-		}
-	}
-
-	if (modified) {
-		status = g_lock_store(rec, lck, NULL, NULL, 0);
-		if (!NT_STATUS_IS_OK(status)) {
-			DBG_DEBUG("g_lock_store() failed: %s\n",
-				  nt_errstr(status));
 		}
 	}
-
-	return status;
 }
 
 static ssize_t g_lock_find_shared(
@@ -369,6 +354,7 @@ static NTSTATUS g_lock_trylock(
 	enum g_lock_type type = req_state->type;
 	bool retry = req_state->retry;
 	struct g_lock lck = { .exclusive.pid = 0 };
+	size_t orig_num_shared;
 	struct server_id_buf tmp;
 	NTSTATUS status;
 	bool ok;
@@ -378,13 +364,9 @@ static NTSTATUS g_lock_trylock(
 		DBG_DEBUG("g_lock_parse failed\n");
 		return NT_STATUS_INTERNAL_DB_CORRUPTION;
 	}
+	orig_num_shared = lck.num_shared;
 
-	status = g_lock_cleanup_dead(rec, &lck, state->dead_blocker);
-	if (!NT_STATUS_IS_OK(status)) {
-		DBG_DEBUG("g_lock_cleanup_dead() failed: %s\n",
-			  nt_errstr(status));
-		return status;
-	}
+	g_lock_cleanup_dead(&lck, state->dead_blocker);
 
 	if (lck.exclusive.pid != 0) {
 		bool self_exclusive = server_id_equal(&self, &lck.exclusive);
@@ -468,6 +450,15 @@ static NTSTATUS g_lock_trylock(
 			return NT_STATUS_LOCK_NOT_GRANTED;
 		}
 
+		if (orig_num_shared != lck.num_shared) {
+			status = g_lock_store(rec, &lck, NULL, NULL, 0);
+			if (!NT_STATUS_IS_OK(status)) {
+				DBG_DEBUG("g_lock_store() failed: %s\n",
+					  nt_errstr(status));
+				return status;
+			}
+		}
+
 		talloc_set_destructor(req_state, NULL);
 
 		/*