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 f7fc5544d26dde4a75dd43343714c5ee1e398f47..07958f30b5e543d01c4c79c1ac964e1a191ed082 100644 (file)
--- 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);
 
                /*