From dcbf54581717d10cf82cdce9bc08415ba1e1d19b Mon Sep 17 00:00:00 2001
From: Xavi Hernandez <xhernandez@redhat.com>
Date: Tue, 12 May 2020 23:54:54 +0200
Subject: open-behind: rewrite of internal logic

There was a critical flaw in the previous implementation of open-behind.

When an open is done in the background, it's necessary to take a
reference on the fd_t object because once we "fake" the open answer,
the fd could be destroyed. However as long as there's a reference,
the release function won't be called. So, if the application closes
the file descriptor without having actually opened it, there will
always remain at least 1 reference, causing a leak.

To avoid this problem, the previous implementation didn't take a
reference on the fd_t, so there were races where the fd could be
destroyed while it was still in use.

To fix this, I've implemented a new xlator cbk that gets called from
fuse when the application closes a file descriptor.

The whole logic of handling background opens have been simplified and
it's more efficient now. Only if the fop needs to be delayed until an
open completes, a stub is created. Otherwise no memory allocations are
needed.

Correctly handling the close request while the open is still pending
has added a bit of complexity, but overall normal operation is simpler.

Change-Id: I6376a5491368e0e1c283cc452849032636261592
Fixes: #1225
Signed-off-by: Xavi Hernandez <xhernandez@redhat.com>
---
 libglusterfs/src/fd.c               | 26 ++++++++++++++++++++++++++
 libglusterfs/src/glusterfs/fd.h     |  3 +++
 libglusterfs/src/glusterfs/xlator.h |  4 ++++
 libglusterfs/src/libglusterfs.sym   |  1 +
 4 files changed, 34 insertions(+)

(limited to 'libglusterfs/src')

diff --git a/libglusterfs/src/fd.c b/libglusterfs/src/fd.c
index e0767a7e61f..62606e91164 100644
--- a/libglusterfs/src/fd.c
+++ b/libglusterfs/src/fd.c
@@ -501,6 +501,32 @@ out:
     return;
 }
 
+void
+fd_close(fd_t *fd)
+{
+    xlator_t *xl, *old_THIS;
+
+    old_THIS = THIS;
+
+    for (xl = fd->inode->table->xl->graph->first; xl != NULL; xl = xl->next) {
+        if (!xl->call_cleanup) {
+            THIS = xl;
+
+            if (IA_ISDIR(fd->inode->ia_type)) {
+                if (xl->cbks->fdclosedir != NULL) {
+                    xl->cbks->fdclosedir(xl, fd);
+                }
+            } else {
+                if (xl->cbks->fdclose != NULL) {
+                    xl->cbks->fdclose(xl, fd);
+                }
+            }
+        }
+    }
+
+    THIS = old_THIS;
+}
+
 void
 fd_unref(fd_t *fd)
 {
diff --git a/libglusterfs/src/glusterfs/fd.h b/libglusterfs/src/glusterfs/fd.h
index 28906d34e4d..3ffaaa60504 100644
--- a/libglusterfs/src/glusterfs/fd.h
+++ b/libglusterfs/src/glusterfs/fd.h
@@ -106,6 +106,9 @@ fd_ref(fd_t *fd);
 void
 fd_unref(fd_t *fd);
 
+void
+fd_close(fd_t *fd);
+
 fd_t *
 fd_create(struct _inode *inode, pid_t pid);
 
diff --git a/libglusterfs/src/glusterfs/xlator.h b/libglusterfs/src/glusterfs/xlator.h
index 42cbdc1ac93..4f188808649 100644
--- a/libglusterfs/src/glusterfs/xlator.h
+++ b/libglusterfs/src/glusterfs/xlator.h
@@ -700,6 +700,8 @@ typedef size_t (*cbk_inodectx_size_t)(xlator_t *this, inode_t *inode);
 
 typedef size_t (*cbk_fdctx_size_t)(xlator_t *this, fd_t *fd);
 
+typedef void (*cbk_fdclose_t)(xlator_t *this, fd_t *fd);
+
 struct xlator_cbks {
     cbk_forget_t forget;
     cbk_release_t release;
@@ -710,6 +712,8 @@ struct xlator_cbks {
     cbk_ictxmerge_t ictxmerge;
     cbk_inodectx_size_t ictxsize;
     cbk_fdctx_size_t fdctxsize;
+    cbk_fdclose_t fdclose;
+    cbk_fdclose_t fdclosedir;
 };
 
 typedef int32_t (*dumpop_priv_t)(xlator_t *this);
diff --git a/libglusterfs/src/libglusterfs.sym b/libglusterfs/src/libglusterfs.sym
index 0de3836efc5..26a11e36410 100644
--- a/libglusterfs/src/libglusterfs.sym
+++ b/libglusterfs/src/libglusterfs.sym
@@ -451,6 +451,7 @@ gf_event_unregister_close
 fd_anonymous
 fd_anonymous_with_flags
 fd_bind
+fd_close
 fd_create
 fd_create_uint64
 __fd_ctx_del
-- 
cgit