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.

diff --git a/lib/fuse.c b/lib/fuse.c
index a7feced66ed7787ef00fe6d77c1f692105dcca77..6d5df233ae55cec185932c22b73f9b19482ba5be 100644 (file)
--- 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 1f5cb534d3e68cc42968f930506041ef21d14fd2..3d74b9ae0a194841df579b844d32106563d7bc0e 100644 (file)
--- 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 (file)
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 f0aa63db40ba79bb91e0ea38e832df420ce367ee..958e63307eadc45ca0d8d7a111567fe42e5f0b62 100755 (executable)
--- 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)