Fix issue #746. (#782)

Added a secondary check in fuse_lib_unlink() after hide_node()
to check again under a lock if the (now hidden) file is still open.
If not then delete it.

This should synchronise fuse_lib_unlink() with fuse_lib_release(),
when nullpath_ok is set.

4 files changed, 187 insertions, 0 deletions

diff --git a/lib/fuse.c b/lib/fuse.c
index a7feced..6d5df23 100644
--- a/lib/fuse.c
+++ b/lib/fuse.c
@@ -2967,6 +2967,20 @@ static void fuse_lib_unlink(fuse_req_t req, fuse_ino_t parent,
 		fuse_prepare_interrupt(f, req, &d);
 		if (!f->conf.hard_remove && is_open(f, parent, name)) {
 			err = hide_node(f, path, parent, name);
+			if (!err) {
+				/* we have hidden the node so now check again under a lock in case it is not used any more */
+				if (!is_open(f, parent, wnode->name)) {
+					char *unlinkpath;
+
+					/* get the hidden file path, to unlink it */
+					if (try_get_path(f, wnode->nodeid, NULL, &unlinkpath, NULL, false) == 0) {
+						err = fuse_fs_unlink(f->fs, unlinkpath);
+						if (!err)
+							remove_node(f, parent, wnode->name);
+						free(unlinkpath);
+					}
+				}
+			}
 		} else {
 			err = fuse_fs_unlink(f->fs, path);
 			if (!err)
diff --git a/test/meson.build b/test/meson.build
index 1f5cb53..3d74b9a 100644
--- a/test/meson.build
+++ b/test/meson.build
@@ -13,6 +13,9 @@ td += executable('test_syscalls', 'test_syscalls.c',
 td += executable('readdir_inode', 'readdir_inode.c',
                  include_directories: include_dirs,
                  install: false)
+td += executable('release_unlink_race', 'release_unlink_race.c',
+                 dependencies: [ libfuse_dep ],
+                 install: false)
 
 test_scripts = [ 'conftest.py', 'pytest.ini', 'test_examples.py',
                  'util.py', 'test_ctests.py', 'test_custom_io.py' ]
diff --git a/test/release_unlink_race.c b/test/release_unlink_race.c
new file mode 100644
index 0000000..2edb200
--- /dev/null
+++ b/test/release_unlink_race.c
@@ -0,0 +1,111 @@
+/*
+  This program can be distributed under the terms of the GNU GPLv2.
+  See the file COPYING.
+*/
+
+#define FUSE_USE_VERSION 31
+
+#define _GNU_SOURCE
+
+#include <fuse.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <errno.h>
+
+static void *xmp_init(struct fuse_conn_info *conn,
+		      struct fuse_config *cfg)
+{
+	(void) conn;
+
+	cfg->use_ino = 1;
+	cfg->nullpath_ok = 1;
+	cfg->entry_timeout = 0;
+	cfg->attr_timeout = 0;
+	cfg->negative_timeout = 0;
+
+	return NULL;
+}
+
+static int xmp_getattr(const char *path, struct stat *stbuf,
+			struct fuse_file_info *fi)
+{
+	int res;
+
+	(void) path;
+
+	if(fi)
+		res = fstat(fi->fh, stbuf);
+	else
+		res = lstat(path, stbuf);
+	if (res == -1)
+		return -errno;
+
+	return 0;
+}
+
+static int xmp_unlink(const char *path)
+{
+	int res;
+
+	res = unlink(path);
+	if (res == -1)
+		return -errno;
+
+	return 0;
+}
+
+static int xmp_rename(const char *from, const char *to, unsigned int flags)
+{
+	int res;
+
+	if (flags)
+		return -EINVAL;
+
+        if(!getenv("RELEASEUNLINKRACE_DELAY_DISABLE")) usleep(100000);
+
+	res = rename(from, to);
+	if (res == -1)
+		return -errno;
+
+	return 0;
+}
+
+static int xmp_create(const char *path, mode_t mode, struct fuse_file_info *fi)
+{
+	int fd;
+
+	fd = open(path, fi->flags, mode);
+	if (fd == -1)
+		return -errno;
+
+	fi->fh = fd;
+	return 0;
+}
+
+static int xmp_release(const char *path, struct fuse_file_info *fi)
+{
+	(void) path;
+
+        if(!getenv("RELEASEUNLINKRACE_DELAY_DISABLE")) usleep(100000);
+
+	close(fi->fh);
+
+	return 0;
+}
+
+static const struct fuse_operations xmp_oper = {
+	.init           = xmp_init,
+	.getattr	= xmp_getattr,
+	.unlink		= xmp_unlink,
+	.rename		= xmp_rename,
+	.create		= xmp_create,
+	.release	= xmp_release,
+};
+
+int main(int argc, char *argv[])
+{
+	umask(0);
+	return fuse_main(argc, argv, &xmp_oper, NULL);
+}
diff --git a/test/test_examples.py b/test/test_examples.py
index f0aa63d..958e633 100755
--- a/test/test_examples.py
+++ b/test/test_examples.py
@@ -442,6 +442,65 @@ def test_cuse(output_checker):
     finally:
         mount_process.terminate()
 
+def test_release_unlink_race(tmpdir, output_checker):
+    """test case for Issue #746
+
+    If RELEASE and UNLINK opcodes are sent back to back, and fuse_fs_release()
+    and fuse_fs_rename() are slow to execute, UNLINK will run while RELEASE is
+    still executing. UNLINK will try to rename the file and, while the rename
+    is happening, the RELEASE will finish executing. As a result, RELEASE will
+    not detect in time that UNLINK has happened, and UNLINK will not detect in
+    time that RELEASE has happened.
+
+
+    NOTE: This is triggered only when nullpath_ok is set.
+
+    If it is NOT SET then get_path_nullok() called by fuse_lib_release() will
+    call get_path_common() and lock the path, and then the fuse_lib_unlink()
+    will wait for the path to be unlocked before executing and thus synchronise
+    with fuse_lib_release().
+
+    If it is SET then get_path_nullok() will just set the path to null and
+    return without locking anything and thus allowing fuse_lib_unlink() to
+    eventually execute unimpeded while fuse_lib_release() is still running.
+    """
+
+    fuse_mountpoint = str(tmpdir)
+
+    fuse_binary_command = base_cmdline + \
+        [ pjoin(basename, 'test', 'release_unlink_race'),
+        "-f", fuse_mountpoint]
+
+    fuse_process = subprocess.Popen(fuse_binary_command,
+                                   stdout=output_checker.fd,
+                                   stderr=output_checker.fd)
+
+    try:
+        wait_for_mount(fuse_process, fuse_mountpoint)
+
+        temp_dir = tempfile.TemporaryDirectory(dir="/tmp/")
+        temp_dir_path = temp_dir.name
+
+        fuse_temp_file, fuse_temp_file_path = tempfile.mkstemp(dir=(fuse_mountpoint + temp_dir_path))
+
+        os.close(fuse_temp_file)
+        os.unlink(fuse_temp_file_path)
+
+        # needed for slow CI/CD pipelines for unlink OP to complete processing
+        safe_sleep(3)
+
+        assert os.listdir(temp_dir_path) == []
+    
+    except:
+        temp_dir.cleanup()
+        cleanup(fuse_process, fuse_mountpoint)
+        raise
+
+    else:
+        temp_dir.cleanup()
+        umount(fuse_process, fuse_mountpoint)
+
+
 @contextmanager
 def os_open(name, flags):
     fd = os.open(name, flags)