From 5e2f89a1c73e3b29e35621ade0f5e16a0db9b82a Mon Sep 17 00:00:00 2001 From: Thales Antunes de Oliveira Barretto Date: Tue, 15 Sep 2026 20:27:03 -0300 Subject: [PATCH 1/2] gfapi: deliver fdclose before dropping an fd in glfs_fd_destroy() fd_close() (added with the open-behind rewrite, db95388706) notifies the xlators of an fd's graph that the application's handle is closing; open-behind uses it to cancel a deferred open and drop the fd reference and the open stub it keeps for it. Only the FUSE bridge ever calls it. gfapi releases its fds with a bare fd_unref(), so with performance.open-behind on (the default volume setting) the first open of an inode in a gfapi process that is closed without an fd fop reaching open-behind -- glfs_open() followed by glfs_close(), or a small-file read served by quick-read in between; glfs_close()'s flush is answered locally -- leaves the fd_t alive with open-behind's two references, pinning its inode, until the file is opened again through that graph (which triggers the stale open and releases it) or the process exits. open-behind's open_count for the inode never comes back down either, so later opens of that file are all sent synchronously. Aleksey Vasenev reported the open_count symptom against 10.1 in #3977 ("caching don't work in nfs-ganesha") and proposed the two-line fd_close() in glfs_close() (#3978); the PR got no review and the stale bot closed it. This change puts the call in glfs_fd_destroy() instead, the single point where every gfapi handle releases its fd (glfs_close, the error paths, a fini-time drain), and delivers it once per fd_t: a glfs_dup()ed handle shares the fd_t with its siblings, so only the last glfd left on fs->openfds sends the fdclose. Reproducible with any gfapi program: open N existing files O_RDONLY and close them without reading; a statedump (glfs_sysrq) shows the fd_t pool active-count = N and N pinned inodes; 0 with this change. Fixes: #3977 Reported-by: Aleksey Vasenev Signed-off-by: Thales Antunes de Oliveira Barretto --- api/src/glfs.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/api/src/glfs.c b/api/src/glfs.c index 1a4bfc14343..c4e744d09a0 100644 --- a/api/src/glfs.c +++ b/api/src/glfs.c @@ -674,16 +674,38 @@ pub_glfs_from_glfd(struct glfs_fd *glfd) static void glfs_fd_destroy(struct glfs_fd *glfd) { + struct glfs_fd *other = NULL; + gf_boolean_t shared = _gf_false; + if (!glfd) return; glfs_lock(glfd->fs, _gf_true); { list_del_init(&glfd->openfds); + + /* glfs_dup() makes several glfds share one fd_t; fdclose must be + * delivered once, when the last of them goes away. */ + if (glfd->fd) { + list_for_each_entry(other, &glfd->fs->openfds, openfds) + { + if (other->fd == glfd->fd) { + shared = _gf_true; + break; + } + } + } } glfs_unlock(glfd->fs); if (glfd->fd) { + /* The application's handle is closing: tell the xlators of the + * graph the fd belongs to before dropping the last gfapi + * reference, exactly as fuse_release() does. open-behind relies + * on this to cancel a still deferred open and release the fd and + * stub references it holds for it. */ + if (!shared) + fd_close(glfd->fd); fd_unref(glfd->fd); glfd->fd = NULL; } From 0ef754adc4f1da568782867524fafab2c5df2fdd Mon Sep 17 00:00:00 2001 From: Thales Antunes de Oliveira Barretto Date: Tue, 15 Sep 2026 20:54:45 -0300 Subject: [PATCH 2/2] tests: gfapi open+close without I/O must not leak with open-behind on Regression test for the fdclose delivery in glfs_fd_destroy(): a small gfapi program opens N existing files read-only, closes them without any I/O, then opens N more, glfs_dup()s each handle and closes both in alternating order (reading through the dup after the original was closed: a premature fdclose would leave open-behind unable to serve it), and takes a statedump through glfs_sysrq(); the test expects the gfapi xlator's gf_common_mt_fd_ctx allocation count in that dump to be 0 (open-behind's deferred opens were cancelled by the closes, once per fd_t). That count exists on every build; the fd_t mem-pool does not on tcmalloc builds. The files are created through a FUSE mount first so that the gfapi process sees their inodes for the first time on open. Updates: #3977 Signed-off-by: Thales Antunes de Oliveira Barretto --- tests/basic/gfapi/open-behind-close-leak.c | 136 +++++++++++++++++++++ tests/basic/gfapi/open-behind-close-leak.t | 59 +++++++++ 2 files changed, 195 insertions(+) create mode 100644 tests/basic/gfapi/open-behind-close-leak.c create mode 100644 tests/basic/gfapi/open-behind-close-leak.t diff --git a/tests/basic/gfapi/open-behind-close-leak.c b/tests/basic/gfapi/open-behind-close-leak.c new file mode 100644 index 00000000000..49ec943f2e5 --- /dev/null +++ b/tests/basic/gfapi/open-behind-close-leak.c @@ -0,0 +1,136 @@ +/* + * Open N existing files (/f1../fN) read-only through gfapi and close them + * without any I/O; then, on N further files (/f(N+1)../f2N), open, glfs_dup() + * the handle and close both, alternating the order and reading through the + * surviving dup after the original was closed. Finally take a statedump of the + * process. With performance.open-behind on the deferred opens must have been + * cancelled exactly once, by the last handle: the gfapi xlator's live fd count + * in the dump must be 0 and every read through a dup must succeed. + * + * usage: open-behind-close-leak + */ + +#include +#include +#include +#include +#include +#include + +#include + +int +main(int argc, char *argv[]) +{ + glfs_t *fs = NULL; + glfs_fd_t *fd = NULL; + glfs_fd_t *dupfd = NULL; + FILE *pf = NULL; + char path[64]; + char buf[1]; + int n = 0, i = 0, ret = 0; + + if (argc != 6) { + fprintf(stderr, "usage: %s \n", + argv[0]); + return 1; + } + n = atoi(argv[3]); + + fs = glfs_new(argv[2]); + if (!fs) { + fprintf(stderr, "glfs_new: %s\n", strerror(errno)); + return 1; + } + ret = glfs_set_volfile_server(fs, "tcp", argv[1], 24007); + if (ret) { + fprintf(stderr, "glfs_set_volfile_server: %s\n", strerror(errno)); + return 1; + } + ret = glfs_set_logging(fs, argv[4], 7); + if (ret) { + fprintf(stderr, "glfs_set_logging: %s\n", strerror(errno)); + return 1; + } + ret = glfs_init(fs); + if (ret) { + fprintf(stderr, "glfs_init: %s\n", strerror(errno)); + return 1; + } + + for (i = 1; i <= n; i++) { + snprintf(path, sizeof(path), "/f%d", i); + fd = glfs_open(fs, path, O_RDONLY); + if (!fd) { + fprintf(stderr, "glfs_open(%s): %s\n", path, strerror(errno)); + return 1; + } + ret = glfs_close(fd); + if (ret) { + fprintf(stderr, "glfs_close(%s): %s\n", path, strerror(errno)); + return 1; + } + } + + /* glfs_dup(): the two handles share one fd_t; the fdclose must be + * delivered once, by whichever handle goes last. */ + for (i = n + 1; i <= 2 * n; i++) { + snprintf(path, sizeof(path), "/f%d", i); + fd = glfs_open(fs, path, O_RDONLY); + if (!fd) { + fprintf(stderr, "glfs_open(%s): %s\n", path, strerror(errno)); + return 1; + } + dupfd = glfs_dup(fd); + if (!dupfd) { + fprintf(stderr, "glfs_dup(%s): %s\n", path, strerror(errno)); + return 1; + } + if (i % 2) { + /* original first, then read through the dup: a premature + * fdclose would leave open-behind unable to serve the read */ + ret = glfs_close(fd); + if (ret) { + fprintf(stderr, "glfs_close(%s): %s\n", path, strerror(errno)); + return 1; + } + if (glfs_pread(dupfd, buf, 1, 0, 0, NULL) != 1) { + fprintf(stderr, + "glfs_pread through dup of %s after closing " + "the original: %s\n", + path, strerror(errno)); + return 1; + } + ret = glfs_close(dupfd); + } else { + ret = glfs_close(dupfd); + if (ret) { + fprintf(stderr, "glfs_close(dup %s): %s\n", path, + strerror(errno)); + return 1; + } + ret = glfs_close(fd); + } + if (ret) { + fprintf(stderr, "glfs_close(%s): %s\n", path, strerror(errno)); + return 1; + } + } + + pf = fopen(argv[5], "w"); + if (!pf) { + fprintf(stderr, "fopen(%s): %s\n", argv[5], strerror(errno)); + return 1; + } + fprintf(pf, "%d\n", (int)getpid()); + fclose(pf); + + ret = glfs_sysrq(fs, GLFS_SYSRQ_STATEDUMP); + if (ret) { + fprintf(stderr, "glfs_sysrq: %s\n", strerror(errno)); + return 1; + } + + glfs_fini(fs); + return 0; +} diff --git a/tests/basic/gfapi/open-behind-close-leak.t b/tests/basic/gfapi/open-behind-close-leak.t new file mode 100644 index 00000000000..b076d791371 --- /dev/null +++ b/tests/basic/gfapi/open-behind-close-leak.t @@ -0,0 +1,59 @@ +#!/bin/bash +# +# gfapi released its fds without delivering fdclose to the graph, so with +# performance.open-behind on a glfs_open() followed by glfs_close() with no +# I/O in between left the fd_t alive (open-behind's deferred-open references) +# and pinned its inode for the life of the process. glfs_dup()ed handles share +# the fd_t, so the fdclose must come from the last handle only: the tester +# also opens, dups and closes N more files in both orders, reading through the +# dup after the original was closed. + +. $(dirname $0)/../../include.rc +. $(dirname $0)/../../volume.rc + +cleanup; + +N=3 +tester=$(dirname $0)/open-behind-close-leak +logfile=$LOGDIR/open-behind-close-leak.log +pidfile=$B0/open-behind-close-leak.pid + +# Live fd_t objects created by the gfapi process: every fd_t owns exactly one +# gf_common_mt_fd_ctx array, accounted to the gfapi master xlator. This works on +# every build; the fd_t mem-pool count does not exist on tcmalloc builds. +function gfapi_live_fd_count { + if [ ! -f "$1" ]; then + echo "no-statedump" + return + fi + awk -F= '/^\[mount\/api\.gfapi - usage-type gf_common_mt_fd_ctx memusage\]$/ {f=1} + f && /^num_allocs=/ {print $2; f=0; e=1} END {if (!e) print 0}' "$1" +} + +TEST glusterd +TEST pidof glusterd +TEST $CLI volume create $V0 $H0:$B0/${V0}0 +TEST $CLI volume set $V0 performance.open-behind on +TEST $CLI volume start $V0 + +# the files must exist before the gfapi process first sees their inodes; +# f1..fN for the plain pass, f(N+1)..f2N for the dup pass +TEST $GFS --volfile-id=/$V0 --volfile-server=$H0 $M0 +for i in $(seq 1 $((2 * N))); do + echo data > $M0/f$i +done +EXPECT_WITHIN $UMOUNT_TIMEOUT "Y" force_umount $M0 + +TEST build_tester $tester.c -lgfapi +rm -f $pidfile +TEST $tester $H0 $V0 $N $logfile $pidfile +pid=$(cat $pidfile 2>/dev/null) +TEST [ -n "$pid" ] +dump=$(ls $statedumpdir/glusterdump.$pid.dump.* 2>/dev/null | head -1) +TEST [ -n "$dump" ] + +EXPECT "0" gfapi_live_fd_count "$dump" + +rm -f $dump $pidfile +cleanup_tester $tester +cleanup;