Incorrect file close in api

[Ratio2]

version 10.1

I think we must call fd_close in glfs_close. Otherwise it leads to indefinitely increase open_count in open-behind and caching don't work in nfs-ganesha.

[stale]

Thank you for your contributions.
Noticed that this issue is not having any activity in last ~6 months! We are marking this issue as stale because it has not had recent activity.
It will be closed in 2 weeks if no one responds with a comment here.

[ThalesBarretto]

related to #4666 #4667 #4668 #4669

[ThalesBarretto]

@Ratio2 i did face this from the samba client side (the api contract expects the client to close the open fds before closing the session but this is not documented or enforced in the api itself). But i agree with you and i would appreciate your view on the related issues and pull requests since they aim to address this design issue.

[ThalesBarretto]

@Ratio2 you were right in 2023, and the effect is larger than the open_count accounting: the fd itself leaks.

I measured it on current devel (a482a8578a, a stock ./configure build) with performance.open-behind on
(the default): a gfapi program that glfs_open()s N existing files read-only and glfs_close()s them without any
I/O leaves N fd_t objects alive, each pinning its inode. In the process statedump (which on a default tcmalloc
build has no fd_t mem-pool, so the per-xlator memory accounting is the count to look at):

N=10:   [mount/api.gfapi - usage-type gf_common_mt_fd_ctx memusage]  num_allocs=10    (one per live fd_t)
N=1000:                                                               num_allocs=1000
with performance.open-behind off, or with the fix below: the section is absent (0)

gfapi statedumps carry no inode table (separate small patch coming for that); with it, each of those inodes shows
fd-count=1 active-fd-count=1 ref=2 ref_by_xl:.vol4490-open-behind=1 after the application closed everything,
and active_size goes 11 → 1 with the fix.

The syncop_flush in glfs_close() does not help — open-behind
answers a flush on a still-deferred fd locally (OB_POST_FLUSH → OB_STATE_OPEN_PENDING) without triggering
the open, so its two references (ob_inode->first_fd and the open stub) survive both fd_unref()s. The fd is
released only when the same file is opened again through that graph (10 → 9 after one reopen), otherwise never.
Any fd fop that reaches open-behind (fstat, writev, fsync, lk, a readv not served by quick-read) triggers
the open and the close then works normally, which is why it hides well: it is the open-then-close-without-I/O
handles that leak. glfs_creat is not affected (the create is synchronous).

fd_close() has exactly one caller in the whole tree, fuse_release(), and that one only covers the base fd — the
FUSE bridge has the same gap for fds migrated across a graph switch (new issue: #4836).

PR #4840 revives #3978 with the call in glfs_fd_destroy() rather than glfs_close(): that is the single point
where every gfapi handle releases its fd (close, the error paths, a fini-time drain like #4669), and it delivers
the fdclose once per fd_t — a glfs_dup()ed handle shares the fd_t with its siblings, so only the last glfd
sends it (with the glfs_close() placement the second close of a dup pair fails with EBADFD: the first close cancels the
deferred open, and the second's flush reaches the brick with no remote fd — measured). It comes with a regression test (tests/basic/gfapi/open-behind-close-leak.t: open+close without I/O,
statedump, expect 0). A/B on the reproduction above: 10 → 0 and 1000 → 0 on stock builds; with fds held across a graph switch (debug builds): 5 → 0.

If you still have the NFS-Ganesha setup where you saw the caching symptom, a run with the patch would be a
valuable second data point.