Skip to content

Expand the set of filesystems considered remote on Linux#10818

Merged
ridiculousfish merged 1 commit into
fish-shell:masterfrom
ridiculousfish:expand-remote-filesystems
Oct 28, 2024
Merged

Expand the set of filesystems considered remote on Linux#10818
ridiculousfish merged 1 commit into
fish-shell:masterfrom
ridiculousfish:expand-remote-filesystems

Conversation

@ridiculousfish

Copy link
Copy Markdown
Member

Some background: fish has some files which should be updated atomically: specifically the history file and the universal variables file. If two fish processes modified these in-place at the same time, then that could result in interleaved writes and corrupted files.

To prevent this, fish uses the write-to-adjacent-file-then-rename to atomically swap in a new file (history is slightly more complicated than this, for performance, but this remains true). This avoids corruption.

However if two fish processes attempt this at the same time, then one process will win the race and the data from the other process will be lost. To prevent this, fish attempts to take an (advisory) lock on the target file before beginning this process. This prevents data loss because only one fish instance can replace the target file at once. (fish checks to ensure it's locked the right file).

However some filesystems, particularly remote file systems, may have locks which hang for a long time, preventing the user from using their shell. This is far more serious than data loss, which is not catastrophic: losing a history item or variable is not a major deal. So fish just attempts to skip locks on remote filesystems.

Unfortunately Linux does not have a good API for checking if a filesystem is remote: the best you can do is check the file system's magic number against a hard-coded list. Today, the list is NFS_SUPER_MAGIC, SMB_SUPER_MAGIC, SMB2_MAGIC_NUMBER, and CIFS_MAGIC_NUMBER.

Expand it to AFS_SUPER_MAGIC, CODA_SUPER_MAGIC, NCP_SUPER_MAGIC, NFS_SUPER_MAGIC, OCFS2_SUPER_MAGIC, SMB_SUPER_MAGIC, SMB2_MAGIC_NUMBER, CIFS_MAGIC_NUMBER, V9FS_MAGIC which is believed to be exhaustive.

ALSO include FUSE_SUPER_MAGIC: if the user's home directory is some FUSE filesystem, that's kind of sus and the fewer tricks we try to pull, the better.

Some background: fish has some files which should be updated atomically:
specifically the history file and the universal variables file. If two fish
processes modified these in-place at the same time, then that could result
in interleaved writes and corrupted files.

To prevent this, fish uses the write-to-adjacent-file-then-rename to
atomically swap in a new file (history is slightly more complicated than
this, for performance, but this remains true). This avoids corruption.

However if two fish processes attempt this at the same time, then one
process will win the race and the data from the other process will be lost.
To prevent this, fish attempts to take an (advisory) lock on the target
file before beginning this process. This prevents data loss because only
one fish instance can replace the target file at once. (fish checks to
ensure it's locked the right file).

However some filesystems, particularly remote file systems, may have locks
which hang for a long time, preventing the user from using their shell.
This is far more serious than data loss, which is not catastrophic: losing
a history item or variable is not a major deal. So fish just attempts to
skip locks on remote filesystems.

Unfortunately Linux does not have a good API for checking if a filesystem
is remote: the best you can do is check the file system's magic number
against a hard-coded list. Today, the list is NFS_SUPER_MAGIC,
SMB_SUPER_MAGIC, SMB2_MAGIC_NUMBER, and CIFS_MAGIC_NUMBER.

Expand it to AFS_SUPER_MAGIC, CODA_SUPER_MAGIC, NCP_SUPER_MAGIC,
NFS_SUPER_MAGIC, OCFS2_SUPER_MAGIC, SMB_SUPER_MAGIC, SMB2_MAGIC_NUMBER,
CIFS_MAGIC_NUMBER, V9FS_MAGIC which is believed to be exhaustive.

ALSO include FUSE_SUPER_MAGIC: if the user's home directory is some FUSE
filesystem, that's kind of sus and the fewer tricks we try to pull, the
better.
@ridiculousfish ridiculousfish merged commit e322d3a into fish-shell:master Oct 28, 2024
ridiculousfish added a commit to ridiculousfish/fish-shell that referenced this pull request Oct 28, 2024
@krobelus

Copy link
Copy Markdown
Contributor

Looks like the list used by df --local contains some more though I'm not sure if any of these are relevant.
Going forward we should try to add this info to Linux.
As a hacky alternative, maybe we can treat Linux file systems as remote if our mmap() fails with ENODEV? That sounds like it could work. The Debian man page says

   ENODEV The underlying filesystem of the specified file does not support memory mapping.

@zanchey zanchey added this to the fish 4.0 milestone Oct 30, 2024
@ridiculousfish

Copy link
Copy Markdown
Member Author

Great suggestions, implemented in 23941ea and in 344b072

@krobelus

krobelus commented Nov 3, 2024

Copy link
Copy Markdown
Contributor

23941ea

the problem is that in load_old_if_needed() we attempt to take a (shared) lock before ever calling mmap(),
so we might still hang (albeit unlikely). Not yet sure what's the best way around that, maybe call mmap() + munmap() specifically for that..

@ridiculousfish

Copy link
Copy Markdown
Member Author

Hmm good point...I'm inclined to do nothing now since we've had this same design for many releases.

@ridiculousfish ridiculousfish deleted the expand-remote-filesystems branch December 14, 2024 19:19
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Dec 15, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants