c460ee
From adeec3d5d85baad8b50d203f34a47ad5360d7cd7 Mon Sep 17 00:00:00 2001
c460ee
From: karthik-us <ksubrahm@redhat.com>
c460ee
Date: Mon, 7 Jun 2021 18:36:11 +0530
c460ee
Subject: [PATCH 582/584] protocol/client: Fix lock memory leak
c460ee
c460ee
Problem-1:
c460ee
When an overlapping lock is issued the merged lock is not assigned the
c460ee
owner. When flush is issued on the fd, this particular lock is not freed
c460ee
leading to memory leak
c460ee
c460ee
Fix-1:
c460ee
Assign the owner while merging the locks.
c460ee
c460ee
Problem-2:
c460ee
On fd-destroy lock structs could be present in fdctx. For some reason
c460ee
with flock -x command and closing of the bash fd, it leads to this code
c460ee
path. Which leaks the lock structs.
c460ee
c460ee
Fix-2:
c460ee
When fdctx is being destroyed in client, make sure to cleanup any lock
c460ee
structs.
c460ee
c460ee
> Upstream patch: https://github.com/gluster/glusterfs/pull/2338/commits/926402f639471d2664bf00c6692221ba297c525f
c460ee
> fixes: gluster#2337
c460ee
> Change-Id: I298124213ce5a1cf2b1f1756d5e8a9745d9c0a1c
c460ee
> Signed-off-by: Pranith Kumar K <pranith.karampuri@phonepe.com>
c460ee
c460ee
BUG: 1689375
c460ee
Change-Id: I298124213ce5a1cf2b1f1756d5e8a9745d9c0a1c
c460ee
Signed-off-by: karthik-us <ksubrahm@redhat.com>
c460ee
Reviewed-on: https://code.engineering.redhat.com/gerrit/c/rhs-glusterfs/+/245603
c460ee
Tested-by: RHGS Build Bot <nigelb@redhat.com>
c460ee
Reviewed-by: Ravishankar Narayanankutty <ravishankar@redhat.com>
c460ee
Reviewed-by: Sunil Kumar Heggodu Gopala Acharya <sheggodu@redhat.com>
c460ee
---
c460ee
 tests/bugs/client/issue-2337-lock-mem-leak.c | 52 ++++++++++++++++++
c460ee
 tests/bugs/client/issue-2337-lock-mem-leak.t | 42 ++++++++++++++
c460ee
 tests/bugs/replicate/do-not-reopen-fd.t      | 65 ++++++++++++++--------
c460ee
 tests/volume.rc                              |  8 +++
c460ee
 xlators/protocol/client/src/client-helpers.c | 10 ++++
c460ee
 xlators/protocol/client/src/client-lk.c      | 82 ++++++++++++++++++----------
c460ee
 xlators/protocol/client/src/client.h         |  8 ++-
c460ee
 7 files changed, 213 insertions(+), 54 deletions(-)
c460ee
 create mode 100644 tests/bugs/client/issue-2337-lock-mem-leak.c
c460ee
 create mode 100644 tests/bugs/client/issue-2337-lock-mem-leak.t
c460ee
c460ee
diff --git a/tests/bugs/client/issue-2337-lock-mem-leak.c b/tests/bugs/client/issue-2337-lock-mem-leak.c
c460ee
new file mode 100644
c460ee
index 0000000..d4e02a7
c460ee
--- /dev/null
c460ee
+++ b/tests/bugs/client/issue-2337-lock-mem-leak.c
c460ee
@@ -0,0 +1,52 @@
c460ee
+#include <sys/file.h>
c460ee
+#include <stdio.h>
c460ee
+#include <string.h>
c460ee
+#include <errno.h>
c460ee
+#include <sys/types.h>
c460ee
+#include <sys/stat.h>
c460ee
+#include <fcntl.h>
c460ee
+
c460ee
+int
c460ee
+main(int argc, char *argv[])
c460ee
+{
c460ee
+    int fd = -1;
c460ee
+    char *filename = NULL;
c460ee
+    struct flock lock = {
c460ee
+        0,
c460ee
+    };
c460ee
+    int i = 0;
c460ee
+    int ret = -1;
c460ee
+
c460ee
+    if (argc != 2) {
c460ee
+        fprintf(stderr, "Usage: %s <filename> ", argv[0]);
c460ee
+        goto out;
c460ee
+    }
c460ee
+
c460ee
+    filename = argv[1];
c460ee
+
c460ee
+    fd = open(filename, O_RDWR | O_CREAT, 0);
c460ee
+    if (fd < 0) {
c460ee
+        fprintf(stderr, "open (%s) failed (%s)\n", filename, strerror(errno));
c460ee
+        goto out;
c460ee
+    }
c460ee
+
c460ee
+    lock.l_type = F_WRLCK;
c460ee
+    lock.l_whence = SEEK_SET;
c460ee
+    lock.l_len = 2;
c460ee
+
c460ee
+    while (i < 100) {
c460ee
+        lock.l_start = i;
c460ee
+        ret = fcntl(fd, F_SETLK, &lock);
c460ee
+        if (ret < 0) {
c460ee
+            fprintf(stderr, "fcntl setlk failed (%s)\n", strerror(errno));
c460ee
+            goto out;
c460ee
+        }
c460ee
+
c460ee
+        i++;
c460ee
+    }
c460ee
+
c460ee
+    ret = 0;
c460ee
+
c460ee
+out:
c460ee
+    return ret;
c460ee
+}
c460ee
diff --git a/tests/bugs/client/issue-2337-lock-mem-leak.t b/tests/bugs/client/issue-2337-lock-mem-leak.t
c460ee
new file mode 100644
c460ee
index 0000000..64132a2
c460ee
--- /dev/null
c460ee
+++ b/tests/bugs/client/issue-2337-lock-mem-leak.t
c460ee
@@ -0,0 +1,42 @@
c460ee
+#!/bin/bash
c460ee
+
c460ee
+#Test that lock fop is not leaking any memory for overlapping regions
c460ee
+. $(dirname $0)/../../include.rc
c460ee
+. $(dirname $0)/../../volume.rc
c460ee
+. $(dirname $0)/../../fileio.rc
c460ee
+
c460ee
+cleanup;
c460ee
+
c460ee
+LOCK_TEST=$(dirname $0)/issue-2337-lock-mem-leak
c460ee
+build_tester $(dirname $0)/issue-2337-lock-mem-leak.c -o ${LOCK_TEST}
c460ee
+
c460ee
+TEST glusterd
c460ee
+TEST pidof glusterd
c460ee
+TEST $CLI volume create $V0 $H0:$B0/${V0}1
c460ee
+#Guard against flush-behind
c460ee
+TEST $CLI volume set $V0 performance.write-behind off
c460ee
+TEST $CLI volume start $V0
c460ee
+TEST $GFS --volfile-id=/$V0 --volfile-server=$H0 $M0
c460ee
+
c460ee
+TEST touch $M0/a
c460ee
+TEST fd1=`fd_available`
c460ee
+TEST fd_open $fd1 'w' $M0/a
c460ee
+TEST flock -x $fd1
c460ee
+statedump=$(generate_mount_statedump $V0 $M0)
c460ee
+EXPECT_NOT "^nostatedump$" echo $statedump
c460ee
+#Making sure no one changes this mem-tracker name
c460ee
+TEST grep gf_client_mt_clnt_lock_t $statedump
c460ee
+TEST fd_close $fd1
c460ee
+
c460ee
+statedump=$(generate_mount_statedump $V0 $M0)
c460ee
+EXPECT_NOT "^nostatedump$" echo $statedump
c460ee
+TEST ! grep gf_client_mt_clnt_lock_t $statedump
c460ee
+
c460ee
+TEST ${LOCK_TEST} $M0/a
c460ee
+
c460ee
+statedump=$(generate_mount_statedump $V0 $M0)
c460ee
+EXPECT_NOT "^nostatedump$" echo $statedump
c460ee
+TEST ! grep gf_client_mt_clnt_lock_t $statedump
c460ee
+TEST cleanup_mount_statedump $V0
c460ee
+TEST rm ${LOCK_TEST}
c460ee
+cleanup
c460ee
diff --git a/tests/bugs/replicate/do-not-reopen-fd.t b/tests/bugs/replicate/do-not-reopen-fd.t
c460ee
index 76d8e70..13b5218 100644
c460ee
--- a/tests/bugs/replicate/do-not-reopen-fd.t
c460ee
+++ b/tests/bugs/replicate/do-not-reopen-fd.t
c460ee
@@ -45,13 +45,17 @@ EXPECT "data-2" cat $B0/${V0}2/a
c460ee
 gfid_a=$(gf_get_gfid_xattr $B0/${V0}0/a)
c460ee
 gfid_str_a=$(gf_gfid_xattr_to_str $gfid_a)
c460ee
 
c460ee
-EXPECT "N" gf_check_file_opened_in_brick $V0 $H0 $B0/${V0}0 $gfid_str_a
c460ee
-EXPECT "Y" gf_check_file_opened_in_brick $V0 $H0 $B0/${V0}1 $gfid_str_a
c460ee
-EXPECT "Y" gf_check_file_opened_in_brick $V0 $H0 $B0/${V0}2 $gfid_str_a
c460ee
+EXPECT "^0$" gf_open_file_count_in_brick $V0 $H0 $B0/${V0}0 $gfid_str_a
c460ee
+EXPECT "^1$" gf_open_file_count_in_brick $V0 $H0 $B0/${V0}1 $gfid_str_a
c460ee
+EXPECT "^1$" gf_open_file_count_in_brick $V0 $H0 $B0/${V0}2 $gfid_str_a
c460ee
 
c460ee
 TEST fd2=`fd_available`
c460ee
 TEST fd_open $fd2 'rw' $M1/a
c460ee
 
c460ee
+EXPECT "^1$" gf_open_file_count_in_brick $V0 $H0 $B0/${V0}0 $gfid_str_a
c460ee
+EXPECT "^2$" gf_open_file_count_in_brick $V0 $H0 $B0/${V0}1 $gfid_str_a
c460ee
+EXPECT "^2$" gf_open_file_count_in_brick $V0 $H0 $B0/${V0}2 $gfid_str_a
c460ee
+
c460ee
 # Kill 2nd brick and try writing to the file. The write should fail due to
c460ee
 # quorum failure.
c460ee
 TEST kill_brick $V0 $H0 $B0/${V0}1
c460ee
@@ -66,6 +70,9 @@ EXPECT_WITHIN $PROCESS_UP_TIMEOUT "^1$" brick_up_status $V0 $H0 $B0/${V0}1
c460ee
 EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" afr_child_up_status_meta $M0 $V0-replicate-0 1
c460ee
 TEST ! fd_write $fd1 "data-4"
c460ee
 TEST ! fd_cat $fd1
c460ee
+EXPECT "^1$" gf_open_file_count_in_brick $V0 $H0 $B0/${V0}0 $gfid_str_a
c460ee
+EXPECT "^1$" gf_open_file_count_in_brick $V0 $H0 $B0/${V0}1 $gfid_str_a
c460ee
+EXPECT "^2$" gf_open_file_count_in_brick $V0 $H0 $B0/${V0}2 $gfid_str_a
c460ee
 
c460ee
 # Enable heal and check the files will have same content on all the bricks after
c460ee
 # the heal is completed.
c460ee
@@ -89,7 +96,9 @@ TEST ! fd_write $fd1 "data-5"
c460ee
 
c460ee
 # At this point only one brick will have the lock. Try taking the lock again on
c460ee
 # the bad fd, which should also fail with EBADFD.
c460ee
-TEST ! flock -x $fd1
c460ee
+# TODO: At the moment quorum failure in lk leads to unlock on the bricks where
c460ee
+# lock succeeds. This will change lock state on 3rd brick, commenting for now
c460ee
+#TEST ! flock -x $fd1
c460ee
 
c460ee
 # Kill the only brick that is having lock and try taking lock on another client
c460ee
 # which should succeed.
c460ee
@@ -97,15 +106,25 @@ TEST kill_brick $V0 $H0 $B0/${V0}2
c460ee
 EXPECT_WITHIN $PROCESS_DOWN_TIMEOUT "0" afr_child_up_status_meta $M0 $V0-replicate-0 2
c460ee
 TEST flock -x $fd2
c460ee
 TEST fd_write $fd2 "data-6"
c460ee
+EXPECT "^1$" gf_open_file_count_in_brick $V0 $H0 $B0/${V0}0 $gfid_str_a
c460ee
+EXPECT "^1$" gf_open_file_count_in_brick $V0 $H0 $B0/${V0}1 $gfid_str_a
c460ee
+
c460ee
 
c460ee
 # Bring the brick up and try writing & reading on the old fd, which should still
c460ee
 # fail and operations on the 2nd fd should succeed.
c460ee
 TEST $CLI volume start $V0 force
c460ee
 EXPECT_WITHIN $PROCESS_UP_TIMEOUT "^1$" brick_up_status $V0 $H0 $B0/${V0}2
c460ee
 EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" afr_child_up_status_meta $M0 $V0-replicate-0 2
c460ee
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" afr_child_up_status_meta $M1 $V0-replicate-0 2
c460ee
+EXPECT "^1$" gf_open_file_count_in_brick $V0 $H0 $B0/${V0}0 $gfid_str_a
c460ee
+EXPECT "^1$" gf_open_file_count_in_brick $V0 $H0 $B0/${V0}1 $gfid_str_a
c460ee
+EXPECT "^1$" gf_open_file_count_in_brick $V0 $H0 $B0/${V0}2 $gfid_str_a
c460ee
 TEST ! fd_write $fd1 "data-7"
c460ee
 
c460ee
 TEST ! fd_cat $fd1
c460ee
+EXPECT "^1$" gf_open_file_count_in_brick $V0 $H0 $B0/${V0}0 $gfid_str_a
c460ee
+EXPECT "^1$" gf_open_file_count_in_brick $V0 $H0 $B0/${V0}1 $gfid_str_a
c460ee
+EXPECT "^1$" gf_open_file_count_in_brick $V0 $H0 $B0/${V0}2 $gfid_str_a
c460ee
 TEST fd_cat $fd2
c460ee
 
c460ee
 # Close both the fds which will release the locks and then re-open and take lock
c460ee
@@ -113,17 +132,15 @@ TEST fd_cat $fd2
c460ee
 TEST fd_close $fd1
c460ee
 TEST fd_close $fd2
c460ee
 
c460ee
-TEST ! ls /proc/$$/fd/$fd1
c460ee
-TEST ! ls /proc/$$/fd/$fd2
c460ee
-EXPECT_WITHIN $REOPEN_TIMEOUT "N" gf_check_file_opened_in_brick $V0 $H0 $B0/${V0}0 $gfid_str_a
c460ee
-EXPECT_WITHIN $REOPEN_TIMEOUT "N" gf_check_file_opened_in_brick $V0 $H0 $B0/${V0}1 $gfid_str_a
c460ee
-EXPECT_WITHIN $REOPEN_TIMEOUT "N" gf_check_file_opened_in_brick $V0 $H0 $B0/${V0}2 $gfid_str_a
c460ee
+EXPECT_WITHIN $REOPEN_TIMEOUT "^0$" gf_open_file_count_in_brick $V0 $H0 $B0/${V0}0 $gfid_str_a
c460ee
+EXPECT_WITHIN $REOPEN_TIMEOUT "^0$" gf_open_file_count_in_brick $V0 $H0 $B0/${V0}1 $gfid_str_a
c460ee
+EXPECT_WITHIN $REOPEN_TIMEOUT "^0$" gf_open_file_count_in_brick $V0 $H0 $B0/${V0}2 $gfid_str_a
c460ee
 
c460ee
 TEST fd1=`fd_available`
c460ee
 TEST fd_open $fd1 'rw' $M0/a
c460ee
-EXPECT_WITHIN $REOPEN_TIMEOUT "Y" gf_check_file_opened_in_brick $V0 $H0 $B0/${V0}0 $gfid_str_a
c460ee
-EXPECT_WITHIN $REOPEN_TIMEOUT "Y" gf_check_file_opened_in_brick $V0 $H0 $B0/${V0}1 $gfid_str_a
c460ee
-EXPECT_WITHIN $REOPEN_TIMEOUT "Y" gf_check_file_opened_in_brick $V0 $H0 $B0/${V0}2 $gfid_str_a
c460ee
+EXPECT_WITHIN $REOPEN_TIMEOUT "^1$" gf_open_file_count_in_brick $V0 $H0 $B0/${V0}0 $gfid_str_a
c460ee
+EXPECT_WITHIN $REOPEN_TIMEOUT "^1$" gf_open_file_count_in_brick $V0 $H0 $B0/${V0}1 $gfid_str_a
c460ee
+EXPECT_WITHIN $REOPEN_TIMEOUT "^1$" gf_open_file_count_in_brick $V0 $H0 $B0/${V0}2 $gfid_str_a
c460ee
 
c460ee
 TEST flock -x $fd1
c460ee
 TEST fd_write $fd1 "data-8"
c460ee
@@ -134,6 +151,10 @@ EXPECT "data-8" head -n 1 $B0/${V0}1/a
c460ee
 EXPECT "data-8" head -n 1 $B0/${V0}2/a
c460ee
 
c460ee
 TEST fd_close $fd1
c460ee
+EXPECT_WITHIN $REOPEN_TIMEOUT "^0$" gf_open_file_count_in_brick $V0 $H0 $B0/${V0}0 $gfid_str_a
c460ee
+EXPECT_WITHIN $REOPEN_TIMEOUT "^0$" gf_open_file_count_in_brick $V0 $H0 $B0/${V0}1 $gfid_str_a
c460ee
+EXPECT_WITHIN $REOPEN_TIMEOUT "^0$" gf_open_file_count_in_brick $V0 $H0 $B0/${V0}2 $gfid_str_a
c460ee
+
c460ee
 
c460ee
 # Heal the volume
c460ee
 TEST $CLI volume heal $V0 enable
c460ee
@@ -152,9 +173,9 @@ EXPECT_WITHIN $PROCESS_DOWN_TIMEOUT "0" afr_child_up_status_meta $M0 $V0-replica
c460ee
 TEST fd1=`fd_available`
c460ee
 TEST fd_open $fd1 'rw' $M0/a
c460ee
 
c460ee
-EXPECT "N" gf_check_file_opened_in_brick $V0 $H0 $B0/${V0}0 $gfid_str_a
c460ee
-EXPECT "Y" gf_check_file_opened_in_brick $V0 $H0 $B0/${V0}1 $gfid_str_a
c460ee
-EXPECT "Y" gf_check_file_opened_in_brick $V0 $H0 $B0/${V0}2 $gfid_str_a
c460ee
+EXPECT "^0$" gf_open_file_count_in_brick $V0 $H0 $B0/${V0}0 $gfid_str_a
c460ee
+EXPECT "^1$" gf_open_file_count_in_brick $V0 $H0 $B0/${V0}1 $gfid_str_a
c460ee
+EXPECT "^1$" gf_open_file_count_in_brick $V0 $H0 $B0/${V0}2 $gfid_str_a
c460ee
 
c460ee
 # Restart the brick and then write. Now fd should get re-opened and write should
c460ee
 # succeed on the previously down brick as well since there are no locks held on
c460ee
@@ -163,7 +184,7 @@ TEST $CLI volume start $V0 force
c460ee
 EXPECT_WITHIN $PROCESS_UP_TIMEOUT "^1$" brick_up_status $V0 $H0 $B0/${V0}0
c460ee
 EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" afr_child_up_status_meta $M0 $V0-replicate-0 0
c460ee
 TEST fd_write $fd1 "data-10"
c460ee
-EXPECT "Y" gf_check_file_opened_in_brick $V0 $H0 $B0/${V0}0 $gfid_str_a
c460ee
+EXPECT "^1$" gf_open_file_count_in_brick $V0 $H0 $B0/${V0}0 $gfid_str_a
c460ee
 
c460ee
 EXPECT "data-10" head -n 1 $B0/${V0}0/a
c460ee
 EXPECT "data-10" head -n 1 $B0/${V0}1/a
c460ee
@@ -177,9 +198,9 @@ TEST fd1=`fd_available`
c460ee
 TEST fd_open $fd1 'rw' $M0/a
c460ee
 TEST flock -x $fd1
c460ee
 
c460ee
-EXPECT "N" gf_check_file_opened_in_brick $V0 $H0 $B0/${V0}0 $gfid_str_a
c460ee
-EXPECT "Y" gf_check_file_opened_in_brick $V0 $H0 $B0/${V0}1 $gfid_str_a
c460ee
-EXPECT "Y" gf_check_file_opened_in_brick $V0 $H0 $B0/${V0}2 $gfid_str_a
c460ee
+EXPECT "^0$" gf_open_file_count_in_brick $V0 $H0 $B0/${V0}0 $gfid_str_a
c460ee
+EXPECT "^1$" gf_open_file_count_in_brick $V0 $H0 $B0/${V0}1 $gfid_str_a
c460ee
+EXPECT "^1$" gf_open_file_count_in_brick $V0 $H0 $B0/${V0}2 $gfid_str_a
c460ee
 
c460ee
 # Kill & restart another brick so that it will return EBADFD
c460ee
 TEST kill_brick $V0 $H0 $B0/${V0}1
c460ee
@@ -194,9 +215,9 @@ EXPECT_WITHIN $PROCESS_UP_TIMEOUT "^1$" brick_up_status $V0 $H0 $B0/${V0}1
c460ee
 EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" afr_child_up_status_meta $M0 $V0-replicate-0 0
c460ee
 EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" afr_child_up_status_meta $M0 $V0-replicate-0 1
c460ee
 TEST ! fd_write $fd1 "data-11"
c460ee
-EXPECT "N" gf_check_file_opened_in_brick $V0 $H0 $B0/${V0}0 $gfid_str_a
c460ee
-EXPECT "N" gf_check_file_opened_in_brick $V0 $H0 $B0/${V0}1 $gfid_str_a
c460ee
-EXPECT "Y" gf_check_file_opened_in_brick $V0 $H0 $B0/${V0}2 $gfid_str_a
c460ee
+EXPECT "^0$" gf_open_file_count_in_brick $V0 $H0 $B0/${V0}0 $gfid_str_a
c460ee
+EXPECT "^0$" gf_open_file_count_in_brick $V0 $H0 $B0/${V0}1 $gfid_str_a
c460ee
+EXPECT "^1$" gf_open_file_count_in_brick $V0 $H0 $B0/${V0}2 $gfid_str_a
c460ee
 
c460ee
 EXPECT "data-10" head -n 1 $B0/${V0}0/a
c460ee
 EXPECT "data-10" head -n 1 $B0/${V0}1/a
c460ee
diff --git a/tests/volume.rc b/tests/volume.rc
c460ee
index f5dd0b1..17c3835 100644
c460ee
--- a/tests/volume.rc
c460ee
+++ b/tests/volume.rc
c460ee
@@ -407,6 +407,14 @@ function gf_check_file_opened_in_brick {
c460ee
         fi
c460ee
 }
c460ee
 
c460ee
+function gf_open_file_count_in_brick {
c460ee
+        vol=$1
c460ee
+        host=$2
c460ee
+        brick=$3
c460ee
+        realpath=$4
c460ee
+        ls -l /proc/$(get_brick_pid $vol $host $brick)/fd | grep "${realpath}$" | wc -l
c460ee
+}
c460ee
+
c460ee
 function gf_get_gfid_backend_file_path {
c460ee
         brickpath=$1
c460ee
         filepath_in_brick=$2
c460ee
diff --git a/xlators/protocol/client/src/client-helpers.c b/xlators/protocol/client/src/client-helpers.c
c460ee
index 48b6448..a80f303 100644
c460ee
--- a/xlators/protocol/client/src/client-helpers.c
c460ee
+++ b/xlators/protocol/client/src/client-helpers.c
c460ee
@@ -3156,11 +3156,14 @@ client_fdctx_destroy(xlator_t *this, clnt_fd_ctx_t *fdctx)
c460ee
     int32_t ret = -1;
c460ee
     char parent_down = 0;
c460ee
     fd_lk_ctx_t *lk_ctx = NULL;
c460ee
+    gf_lkowner_t null_owner = {0};
c460ee
+    struct list_head deleted_list;
c460ee
 
c460ee
     GF_VALIDATE_OR_GOTO("client", this, out);
c460ee
     GF_VALIDATE_OR_GOTO(this->name, fdctx, out);
c460ee
 
c460ee
     conf = (clnt_conf_t *)this->private;
c460ee
+    INIT_LIST_HEAD(&deleted_list);
c460ee
 
c460ee
     if (fdctx->remote_fd == -1) {
c460ee
         gf_msg_debug(this->name, 0, "not a valid fd");
c460ee
@@ -3174,6 +3177,13 @@ client_fdctx_destroy(xlator_t *this, clnt_fd_ctx_t *fdctx)
c460ee
     pthread_mutex_unlock(&conf->lock);
c460ee
     lk_ctx = fdctx->lk_ctx;
c460ee
     fdctx->lk_ctx = NULL;
c460ee
+    pthread_spin_lock(&conf->fd_lock);
c460ee
+    {
c460ee
+        __delete_granted_locks_owner_from_fdctx(fdctx, &null_owner,
c460ee
+                                                &deleted_list);
c460ee
+    }
c460ee
+    pthread_spin_unlock(&conf->fd_lock);
c460ee
+    destroy_client_locks_from_list(&deleted_list);
c460ee
 
c460ee
     if (lk_ctx)
c460ee
         fd_lk_ctx_unref(lk_ctx);
c460ee
diff --git a/xlators/protocol/client/src/client-lk.c b/xlators/protocol/client/src/client-lk.c
c460ee
index c1fb055..cb4e894 100644
c460ee
--- a/xlators/protocol/client/src/client-lk.c
c460ee
+++ b/xlators/protocol/client/src/client-lk.c
c460ee
@@ -253,6 +253,7 @@ __insert_and_merge(clnt_fd_ctx_t *fdctx, client_posix_lock_t *lock)
c460ee
                 sum = add_locks(lock, conf);
c460ee
 
c460ee
                 sum->fd = lock->fd;
c460ee
+                sum->owner = conf->owner;
c460ee
 
c460ee
                 __delete_client_lock(conf);
c460ee
                 __destroy_client_lock(conf);
c460ee
@@ -320,56 +321,77 @@ destroy_client_lock(client_posix_lock_t *lock)
c460ee
     GF_FREE(lock);
c460ee
 }
c460ee
 
c460ee
-int32_t
c460ee
-delete_granted_locks_owner(fd_t *fd, gf_lkowner_t *owner)
c460ee
+void
c460ee
+destroy_client_locks_from_list(struct list_head *deleted)
c460ee
 {
c460ee
-    clnt_fd_ctx_t *fdctx = NULL;
c460ee
     client_posix_lock_t *lock = NULL;
c460ee
     client_posix_lock_t *tmp = NULL;
c460ee
-    xlator_t *this = NULL;
c460ee
-    clnt_conf_t *conf = NULL;
c460ee
-
c460ee
-    struct list_head delete_list;
c460ee
-    int ret = 0;
c460ee
+    xlator_t *this = THIS;
c460ee
     int count = 0;
c460ee
 
c460ee
-    INIT_LIST_HEAD(&delete_list);
c460ee
-    this = THIS;
c460ee
-    conf = this->private;
c460ee
+    list_for_each_entry_safe(lock, tmp, deleted, list)
c460ee
+    {
c460ee
+        list_del_init(&lock->list);
c460ee
+        destroy_client_lock(lock);
c460ee
+        count++;
c460ee
+    }
c460ee
 
c460ee
-    pthread_spin_lock(&conf->fd_lock);
c460ee
+    /* FIXME: Need to actually print the locks instead of count */
c460ee
+    gf_msg_trace(this->name, 0, "Number of locks cleared=%d", count);
c460ee
+}
c460ee
 
c460ee
-    fdctx = this_fd_get_ctx(fd, this);
c460ee
-    if (!fdctx) {
c460ee
-        pthread_spin_unlock(&conf->fd_lock);
c460ee
+void
c460ee
+__delete_granted_locks_owner_from_fdctx(clnt_fd_ctx_t *fdctx,
c460ee
+                                        gf_lkowner_t *owner,
c460ee
+                                        struct list_head *deleted)
c460ee
+{
c460ee
+    client_posix_lock_t *lock = NULL;
c460ee
+    client_posix_lock_t *tmp = NULL;
c460ee
 
c460ee
-        gf_msg(this->name, GF_LOG_WARNING, EINVAL, PC_MSG_FD_CTX_INVALID,
c460ee
-               "fdctx not valid");
c460ee
-        ret = -1;
c460ee
-        goto out;
c460ee
+    gf_boolean_t is_null_lkowner = _gf_false;
c460ee
+
c460ee
+    if (is_lk_owner_null(owner)) {
c460ee
+        is_null_lkowner = _gf_true;
c460ee
     }
c460ee
 
c460ee
     list_for_each_entry_safe(lock, tmp, &fdctx->lock_list, list)
c460ee
     {
c460ee
-        if (is_same_lkowner(&lock->owner, owner)) {
c460ee
+        if (is_null_lkowner || is_same_lkowner(&lock->owner, owner)) {
c460ee
             list_del_init(&lock->list);
c460ee
-            list_add_tail(&lock->list, &delete_list);
c460ee
-            count++;
c460ee
+            list_add_tail(&lock->list, deleted);
c460ee
         }
c460ee
     }
c460ee
+}
c460ee
 
c460ee
-    pthread_spin_unlock(&conf->fd_lock);
c460ee
+int32_t
c460ee
+delete_granted_locks_owner(fd_t *fd, gf_lkowner_t *owner)
c460ee
+{
c460ee
+    clnt_fd_ctx_t *fdctx = NULL;
c460ee
+    xlator_t *this = NULL;
c460ee
+    clnt_conf_t *conf = NULL;
c460ee
+    int ret = 0;
c460ee
+    struct list_head deleted_locks;
c460ee
 
c460ee
-    if (!list_empty(&delete_list)) {
c460ee
-        list_for_each_entry_safe(lock, tmp, &delete_list, list)
c460ee
-        {
c460ee
-            list_del_init(&lock->list);
c460ee
-            destroy_client_lock(lock);
c460ee
+    this = THIS;
c460ee
+    conf = this->private;
c460ee
+    INIT_LIST_HEAD(&deleted_locks);
c460ee
+
c460ee
+    pthread_spin_lock(&conf->fd_lock);
c460ee
+    {
c460ee
+        fdctx = this_fd_get_ctx(fd, this);
c460ee
+        if (!fdctx) {
c460ee
+            pthread_spin_unlock(&conf->fd_lock);
c460ee
+
c460ee
+            gf_smsg(this->name, GF_LOG_WARNING, EINVAL, PC_MSG_FD_CTX_INVALID,
c460ee
+                    NULL);
c460ee
+            ret = -1;
c460ee
+            goto out;
c460ee
         }
c460ee
+        __delete_granted_locks_owner_from_fdctx(fdctx, owner, &deleted_locks);
c460ee
     }
c460ee
+    pthread_spin_unlock(&conf->fd_lock);
c460ee
 
c460ee
-    /* FIXME: Need to actually print the locks instead of count */
c460ee
-    gf_msg_trace(this->name, 0, "Number of locks cleared=%d", count);
c460ee
+    destroy_client_locks_from_list(&deleted_locks);
c460ee
 
c460ee
 out:
c460ee
     return ret;
c460ee
diff --git a/xlators/protocol/client/src/client.h b/xlators/protocol/client/src/client.h
c460ee
index 2a50625..f952aea 100644
c460ee
--- a/xlators/protocol/client/src/client.h
c460ee
+++ b/xlators/protocol/client/src/client.h
c460ee
@@ -406,8 +406,12 @@ int
c460ee
 client_attempt_lock_recovery(xlator_t *this, clnt_fd_ctx_t *fdctx);
c460ee
 int32_t
c460ee
 delete_granted_locks_owner(fd_t *fd, gf_lkowner_t *owner);
c460ee
-int32_t
c460ee
-delete_granted_locks_fd(clnt_fd_ctx_t *fdctx);
c460ee
+void
c460ee
+__delete_granted_locks_owner_from_fdctx(clnt_fd_ctx_t *fdctx,
c460ee
+                                        gf_lkowner_t *owner,
c460ee
+                                        struct list_head *deleted);
c460ee
+void
c460ee
+destroy_client_locks_from_list(struct list_head *deleted);
c460ee
 int32_t
c460ee
 client_cmd_to_gf_cmd(int32_t cmd, int32_t *gf_cmd);
c460ee
 void
c460ee
-- 
c460ee
1.8.3.1
c460ee