fuse: release newfd when fd migration fails after fd_create() - #4842
Draft
ThalesBarretto wants to merge 2 commits into
Draft
ThalesBarretto wants to merge 2 commits into
ThalesBarretto wants to merge 2 commits into
Conversation
fuse_migrate_fd_open() creates the new-graph fd with fd_create() and hands it over to basefd_ctx->activefd only at the very end. The two exits in between -- fuse_fd_ctx_check_n_create() failing and the syncop_open()/syncop_opendir() on the new graph failing -- jump to out:, which only wipes the loc. newfd, the fuse fd ctx just created for it, and the inode and lk_ctx references it carries are never released, so every failed reopen during a graph switch leaks one fd_t + one gf_fuse_mt_fd_ctx_t and pins the new-graph inode for the life of the process (the application already gets EBADF on that handle, the leak is the part it cannot see). The GF_VALIDATE_OR_GOTO() exits also left ret at the inode_path() length, so fuse_migrate_fd() was told the migration succeeded although activefd was never set. Release newfd on every failure after fd_create(), destroying its fuse fd ctx explicitly first: fd_destroy() reaches the ctx through the .release cbk for regular files only, no .releasedir cbk is registered. Make every early exit past the lookup a failure (ret = -1). Reproducible with performance.open-behind off (with it on the reopen is answered locally and cannot fail): keep files open across a 'volume set', make the reopen fail (RLIMIT_NOFILE clamp on the brick, or chattr +i on the brick file with an O_RDWR handle), close everything and take a statedump: fd_t active-count and gf_fuse_mt_fd_ctx_t num_allocs equal the number of failed reopens; 0 with this change. Present since fd migration was introduced (33d9df7, 2012). Fixes: gluster#4841 Signed-off-by: Thales Antunes de Oliveira Barretto <thales.barretto.git@gmail.com>
Regression test for the error path of fuse_migrate_fd_open(): keep five files open O_RDWR on a FUSE mount with open-behind off, make the brick copies immutable (chattr +i) so the O_RDWR reopen on the new graph fails with EPERM, switch graphs (poking the mount, since FUSE switches on the next request), close the now-dead handles and expect the client statedump to show fd_t active-count 0 and no gf_fuse_mt_fd_ctx_t allocations. Updates: gluster#4841 Signed-off-by: Thales Antunes de Oliveira Barretto <thales.barretto.git@gmail.com>
ThalesBarretto
marked this pull request as draft
September 22, 2026 10:00
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fuse: release newfd when fd migration fails after fd_create()
Problem
fuse_migrate_fd_open()creates the new-graph fd withfd_create()and hands it over tobasefd_ctx->activefdonly at the very end. The two exits in between —fuse_fd_ctx_check_n_create()failing and the
syncop_open()/syncop_opendir()on the new graph failing — jump toout:, which onlywipes the loc.
newfd, the fuse fd ctx just created for it, and the inode andlk_ctxreferences it carriesare never released, so every failed reopen during a graph switch leaks one
fd_t+ onegf_fuse_mt_fd_ctx_tand pins the new-graph inode for the life of the process. The application alreadygets
EBADFon that handle (migration_failed); the leak is the part it cannot see.The
GF_VALIDATE_OR_GOTO()exits also leftretat theinode_path()length, sofuse_migrate_fd()wastold the migration succeeded although
activefdwas never set.Fix
ret = -1before the validates).out:, whenret < 0 && newfd, destroy newfd's fuse fd ctx explicitly andfd_unref()it.The explicit
fuse_fd_ctx_destroy()is needed for directories:fd_destroy()reaches the ctx throughthe
.releasecbk for regular files only, no.releasedircbk is registered. For regular files it is anidempotent duplicate (
fd_ctx_del_ptr()returns NULL the second time).Reproduction and A/B
Stock builds of devel
a482a8578aand of this branch (default./configure: tcmalloc, no mem-pools, no debug).With
performance.open-behind off(with it on the reopen is answered locally and cannot fail): files held openO_RDWR on a FUSE mount,
chattr +ion their brick copies so the O_RDWR reopen fails EPERM, avolume set, the mountpoked with
stat(FUSE switches graphs on the next request), everything closed, client statedump. On a tcmallocbuild there is no
fd_tmem-pool, so the count is the fuse xlator's memory accounting: onegf_common_mt_fd_ctxarray per live
fd_t.gf_common_mt_fd_ctx)gf_fuse_mt_fd_ctx_tactive_size(the leaked fd was never bound, hence
fd-count=0; the inode stays active on the referencefd_create()took.)(*) the remaining ctx belongs to the one successfully migrated directory: fuse registers no
.releasedircbk, so
fd_destroy()never frees a directory fd's fuse ctx — the gap #4490 closes, not this change. Verified onstock arms carrying #4490: devel + #4490 still leaks the 9 failed newfds (9 fds / 9 ctxs, the migrated directory's
ctx now freed); this change + #4490 gives 0 / 0.
Test
tests/bugs/fuse/fd-migration-reopen-failure-leak.t: five O_RDWR handles,chattr +ion the brick copies,one graph switch, close, expects the fuse xlator's
gf_common_mt_fd_ctxallocation count (live fds; exists onevery build, unlike the
fd_tmem-pool) and itsgf_fuse_mt_fd_ctx_tcount to be 0 in the mount statedump. Failson stock devel on exactly those two assertions (
Got "5" instead of "0"), passes with the change.Present since
33d9df7a93(2012); also in release-11.Fixes: #4841