protocol/client: Fix lock memory leak (#2338)

Problem-1:
When an overlapping lock is issued the merged lock is not assigned the
owner. When flush is issued on the fd, this particular lock is not freed
leading to memory leak

Fix-1:
Assign the owner while merging the locks.

Problem-2:
On fd-destroy lock structs could be present in fdctx. For some reason
with flock -x command and closing of the bash fd, it leads to this code
path. Which leaks the lock structs.

Fix-2:
When fdctx is being destroyed in client, make sure to cleanup any lock
structs.

fixes: #2337
Change-Id: I298124213ce5a1cf2b1f1756d5e8a9745d9c0a1c
Signed-off-by: Pranith Kumar K <pranith.karampuri@phonepe.com>

3 files changed, 68 insertions, 33 deletions

diff --git a/xlators/protocol/client/src/client-helpers.c b/xlators/protocol/client/src/client-helpers.c
index 23e2a57bce..6ae8bbd1fb 100644
--- a/xlators/protocol/client/src/client-helpers.c
+++ b/xlators/protocol/client/src/client-helpers.c
@@ -859,11 +859,14 @@ client_fdctx_destroy(xlator_t *this, clnt_fd_ctx_t *fdctx)
     int32_t ret = -1;
     char parent_down = 0;
     fd_lk_ctx_t *lk_ctx = NULL;
+    gf_lkowner_t null_owner = {0};
+    struct list_head deleted_list;
 
     GF_VALIDATE_OR_GOTO("client", this, out);
     GF_VALIDATE_OR_GOTO(this->name, fdctx, out);
 
     conf = (clnt_conf_t *)this->private;
+    INIT_LIST_HEAD(&deleted_list);
 
     if (fdctx->remote_fd == -1) {
         gf_msg_debug(this->name, 0, "not a valid fd");
@@ -877,6 +880,13 @@ client_fdctx_destroy(xlator_t *this, clnt_fd_ctx_t *fdctx)
     pthread_mutex_unlock(&conf->lock);
     lk_ctx = fdctx->lk_ctx;
     fdctx->lk_ctx = NULL;
+    pthread_spin_lock(&conf->fd_lock);
+    {
+        __delete_granted_locks_owner_from_fdctx(fdctx, &null_owner,
+                                                &deleted_list);
+    }
+    pthread_spin_unlock(&conf->fd_lock);
+    destroy_client_locks_from_list(&deleted_list);
 
     if (lk_ctx)
         fd_lk_ctx_unref(lk_ctx);
diff --git a/xlators/protocol/client/src/client-lk.c b/xlators/protocol/client/src/client-lk.c
index 795839734c..c0e3e2df20 100644
--- a/xlators/protocol/client/src/client-lk.c
+++ b/xlators/protocol/client/src/client-lk.c
@@ -249,6 +249,7 @@ __insert_and_merge(clnt_fd_ctx_t *fdctx, client_posix_lock_t *lock)
                 sum = add_locks(lock, conf);
 
                 sum->fd = lock->fd;
+                sum->owner = conf->owner;
 
                 __delete_client_lock(conf);
                 __destroy_client_lock(conf);
@@ -316,57 +317,77 @@ destroy_client_lock(client_posix_lock_t *lock)
     GF_FREE(lock);
 }
 
-int32_t
-delete_granted_locks_owner(fd_t *fd, gf_lkowner_t *owner)
+void
+destroy_client_locks_from_list(struct list_head *deleted)
 {
-    clnt_fd_ctx_t *fdctx = NULL;
     client_posix_lock_t *lock = NULL;
     client_posix_lock_t *tmp = NULL;
-    xlator_t *this = NULL;
-    clnt_conf_t *conf = NULL;
-
-    struct list_head delete_list;
-    int ret = 0;
+    xlator_t *this = THIS;
     int count = 0;
 
-    INIT_LIST_HEAD(&delete_list);
-    this = THIS;
-    conf = this->private;
+    list_for_each_entry_safe(lock, tmp, deleted, list)
+    {
+        list_del_init(&lock->list);
+        destroy_client_lock(lock);
+        count++;
+    }
 
-    pthread_spin_lock(&conf->fd_lock);
+    /* FIXME: Need to actually print the locks instead of count */
+    gf_msg_trace(this->name, 0, "Number of locks cleared=%d", count);
+}
 
-    fdctx = this_fd_get_ctx(fd, this);
-    if (!fdctx) {
-        pthread_spin_unlock(&conf->fd_lock);
+void
+__delete_granted_locks_owner_from_fdctx(clnt_fd_ctx_t *fdctx,
+                                        gf_lkowner_t *owner,
+                                        struct list_head *deleted)
+{
+    client_posix_lock_t *lock = NULL;
+    client_posix_lock_t *tmp = NULL;
 
-        gf_smsg(this->name, GF_LOG_WARNING, EINVAL, PC_MSG_FD_CTX_INVALID,
-                NULL);
-        ret = -1;
-        goto out;
+    gf_boolean_t is_null_lkowner = _gf_false;
+
+    if (is_lk_owner_null(owner)) {
+        is_null_lkowner = _gf_true;
     }
 
     list_for_each_entry_safe(lock, tmp, &fdctx->lock_list, list)
     {
-        if (is_same_lkowner(&lock->owner, owner)) {
+        if (is_null_lkowner || is_same_lkowner(&lock->owner, owner)) {
             list_del_init(&lock->list);
-            list_add_tail(&lock->list, &delete_list);
-            count++;
+            list_add_tail(&lock->list, deleted);
         }
     }
+}
 
-    pthread_spin_unlock(&conf->fd_lock);
+int32_t
+delete_granted_locks_owner(fd_t *fd, gf_lkowner_t *owner)
+{
+    clnt_fd_ctx_t *fdctx = NULL;
+    xlator_t *this = NULL;
+    clnt_conf_t *conf = NULL;
+    int ret = 0;
+    struct list_head deleted_locks;
 
-    if (!list_empty(&delete_list)) {
-        list_for_each_entry_safe(lock, tmp, &delete_list, list)
-        {
-            list_del_init(&lock->list);
-            destroy_client_lock(lock);
+    this = THIS;
+    conf = this->private;
+    INIT_LIST_HEAD(&deleted_locks);
+
+    pthread_spin_lock(&conf->fd_lock);
+    {
+        fdctx = this_fd_get_ctx(fd, this);
+        if (!fdctx) {
+            pthread_spin_unlock(&conf->fd_lock);
+
+            gf_smsg(this->name, GF_LOG_WARNING, EINVAL, PC_MSG_FD_CTX_INVALID,
+                    NULL);
+            ret = -1;
+            goto out;
         }
+        __delete_granted_locks_owner_from_fdctx(fdctx, owner, &deleted_locks);
     }
+    pthread_spin_unlock(&conf->fd_lock);
 
-    /* FIXME: Need to actually print the locks instead of count */
-    gf_msg_trace(this->name, 0, "Number of locks cleared=%d", count);
-
+    destroy_client_locks_from_list(&deleted_locks);
 out:
     return ret;
 }
diff --git a/xlators/protocol/client/src/client.h b/xlators/protocol/client/src/client.h
index 4b4e079764..b9cbf64fd4 100644
--- a/xlators/protocol/client/src/client.h
+++ b/xlators/protocol/client/src/client.h
@@ -307,8 +307,12 @@ int
 client_attempt_lock_recovery(xlator_t *this, clnt_fd_ctx_t *fdctx);
 int32_t
 delete_granted_locks_owner(fd_t *fd, gf_lkowner_t *owner);
-int32_t
-delete_granted_locks_fd(clnt_fd_ctx_t *fdctx);
+void
+__delete_granted_locks_owner_from_fdctx(clnt_fd_ctx_t *fdctx,
+                                        gf_lkowner_t *owner,
+                                        struct list_head *deleted);
+void
+destroy_client_locks_from_list(struct list_head *deleted);
 int32_t
 client_cmd_to_gf_cmd(int32_t cmd, int32_t *gf_cmd);
 void