Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Copy tasks between some different file-systems (remote) hang with (os error 95) #1775

Closed
ppenguin opened this issue Oct 13, 2024 · 18 comments · Fixed by #1805 or #1817
Closed

Copy tasks between some different file-systems (remote) hang with (os error 95) #1775

ppenguin opened this issue Oct 13, 2024 · 18 comments · Fixed by #1805 or #1817
Labels
bug Something isn't working cant reproduce The issue cannot be reproduced as described help wanted Extra attention is needed

Comments

@ppenguin
Copy link

ppenguin commented Oct 13, 2024

What system are you running Yazi on?

Linux Wayland

What terminal are you running Yazi in?

kitty

yazi --debug output

Yazi
    Version: 0.3.3 (Nixpkgs 2024-09-04)
    Debug  : false
    OS     : linux-x86_64 (unix)

Ya
    Version: 0.3.3 (Nixpkgs 2024-09-04)

Emulator
    Emulator.via_env: ("xterm-kitty", "")
    Emulator.via_csi: Ok(Kitty)
    Emulator.detect : Kitty

Adapter
    Adapter.matches: Kitty

Desktop
    XDG_SESSION_TYPE           : Some("wayland")
    WAYLAND_DISPLAY            : Some("wayland-1")
    DISPLAY                    : Some(":0")
    SWAYSOCK                   : None
    HYPRLAND_INSTANCE_SIGNATURE: Some("0f594732b063a90d44df8c5d402d658f27471dfe_1728814821_51066977")
    WAYFIRE_SOCKET             : None

SSH
    shared.in_ssh_connection: false

WSL
    WSL: false

Variables
    SHELL              : Some("/run/current-system/sw/bin/zsh")
    EDITOR             : Some("nvim")
    VISUAL             : None
    YAZI_FILE_ONE      : None
    YAZI_CONFIG_HOME   : None

Text Opener
    default: Some(Opener { run: "${EDITOR:-vi} \"$@\"", block: true, orphan: false, desc: "$EDITOR", for_: None, spread: true })
    block  : Some(Opener { run: "${EDITOR:-vi} \"$@\"", block: true, orphan: false, desc: "$EDITOR", for_: None, spread: true })

Multiplexers
    TMUX               : false
    tmux version       : tmux 3.5a
    ZELLIJ_SESSION_NAME: None
    Zellij version     : No such file or directory (os error 2)

Dependencies
    file             : 5.45
    ueberzugpp       : No such file or directory (os error 2)
    ffmpegthumbnailer: 2.2.2
    magick           : 7.1.1-39
    fzf              : 0.55.0
    fd               : 10.2.0
    rg               : 14.1.1
    chafa            : 1.14.4
    zoxide           : 0.9.6
    7z               : 17.05
    7zz              : 24.08
    jq               : 1.7.1

Did you try the latest nightly build to see if the problem got fixed?

No, and I'll explain why below

Describe the bug

I'm copying files from a mounted ntfs3 volume (external USB HD) to a gvfs mounted samba share (i.e. accessed through /run/user/1000/gvfs/...).

I get many Failed to work on this task: Operation not supported (os error 95)
errors, and all tasks with the same source/destination hang. No further information is given.

It is not unusual to get some errors messages when copying between file-systems, these almost always refer to the inability to copy some kind of extended attribute, however, this type of error is/should be always treated non-fatally (more like a warning), this is the behaviour of all other known software (at least on linux).

Therefore, "not trying harder" to just copy the files and hanging instead should be considered a bug.

Expectation: issues regarding copying of extended attributes are reported as warnings, but the copying tasks are completed if no "real" errors (i.e. no write premission, disk full, ...) occur.

Minimal reproducer

Not sure, probably copying files with some kind of extended attributes to a gvfs mounted remote samba share?

Anything else?

No response

@ppenguin ppenguin added the bug Something isn't working label Oct 13, 2024
@sxyazi
Copy link
Owner

sxyazi commented Oct 13, 2024

Seems a dup of #1703

Could you try this comment #1703 (comment)?

@sxyazi sxyazi added the waiting on op Waiting for more information from the original poster label Oct 13, 2024
@ppenguin
Copy link
Author

Seems a dup of #1703

Could you try this comment #1703 (comment)?

Thanks a lot for the quick reply.

  • It might seem similar, but it's definitely not "permission denied", with pretty much everything else I can copy without issue (now copying with rsync). Also The error is 95 and not 13, maybe that can tell us something more?
  • I'm currently not really able to spin up a rust file copy demonstrator to try bare rust functions like std::xxx... But maybe you know of a cli cp program that uses the same under the hood? Alternatively, can I get more verbose debugging in yazi, to a log file?

@github-actions github-actions bot removed the waiting on op Waiting for more information from the original poster label Oct 13, 2024
@sxyazi
Copy link
Owner

sxyazi commented Oct 13, 2024

I'm currently not really able to spin up a rust file copy demonstrator to try bare rust functions like std::xxx... But maybe you know of a cli cp program that uses the same under the hood?

You can try Joshuto which it's also a file manager built with Rust

@sxyazi sxyazi added the waiting on op Waiting for more information from the original poster label Oct 13, 2024
@AidanJHMurphy
Copy link

I've run into (what I think is) the same issue. Interestingly, running a shell command within yazi using the ":" key binding to create a file (e.g. touch test1.txt or echo "testing" > test2.txt) both succeed, as does using the 'a' key binding to create a file within yazi. It's only the yank-paste ("y" and "p" keybindings) workflow that seems to cause a hang.

It doesn't look like you have to be operating between different file systems either, as I was able to reproduce this yank-pasting a file from a samba share into the same directory. Also, it looks like the operation succeeds in creating the new file, but hangs at some point after that.

I've seen other discussions elsewhere with people having similar problems with the std::fs::copy function, and the hypothesis that "NTFS does not have chmod permissions, therefore attempting to set chmod permissions on a samba share will fail."

If I find the time, I'll try to play around with different versions of copy functions in Rust using my mounted file system, and see if I can isolate the issue and possibly suggest a fix.

@sxyazi
Copy link
Owner

sxyazi commented Oct 18, 2024

Thanks @AidanJHMurphy, this info is really helpful!

It seems like Rust's std::fs::copy() does extra work related to file metadata (permissions) and raises a hard error for safety when it can't copy that metadata, even if the target filesystem doesn't support the operation at all.

I tried io::copy() and found it similar to the system call in fs::copy() on Linux, which means Yazi can still use copy_file_range to benefit from the Copy-on-write filesystem:

// fs::copy()

statx(3, "", AT_STATX_SYNC_AS_STAT|AT_EMPTY_PATH, STATX_ALL, {stx_mask=STATX_ALL|STATX_MNT_ID, stx_attributes=0, stx_mode=S_IFREG|0644, stx_size=13, ...}) = 0
openat(AT_FDCWD, "b", O_WRONLY|O_CREAT|O_TRUNC|O_CLOEXEC, 0100644) = 4
statx(4, "", AT_STATX_SYNC_AS_STAT|AT_EMPTY_PATH, STATX_ALL, {stx_mask=STATX_ALL|STATX_MNT_ID, stx_attributes=0, stx_mode=S_IFREG|0644, stx_size=0, ...}) = 0
fchmod(4, 0100644)                      = 0
copy_file_range(3, NULL, 4, NULL, 1073741824, 0) = 13
copy_file_range(3, NULL, 4, NULL, 1073741824, 0) = 0
close(4)                                = 0
close(3)                                = 0
// io::copy()

statx(3, "", AT_STATX_SYNC_AS_STAT|AT_EMPTY_PATH, STATX_ALL, {stx_mask=STATX_ALL|STATX_MNT_ID, stx_attributes=0, stx_mode=S_IFREG|0644, stx_size=13, ...}) = 0
openat(AT_FDCWD, "b", O_WRONLY|O_CREAT|O_TRUNC|O_CLOEXEC, 0100644) = 4
fchmod(4, 0100644)                      = 0
statx(3, "", AT_STATX_SYNC_AS_STAT|AT_EMPTY_PATH, STATX_ALL, {stx_mask=STATX_ALL|STATX_MNT_ID, stx_attributes=0, stx_mode=S_IFREG|0644, stx_size=13, ...}) = 0
statx(4, "", AT_STATX_SYNC_AS_STAT|AT_EMPTY_PATH, STATX_ALL, {stx_mask=STATX_ALL|STATX_MNT_ID, stx_attributes=0, stx_mode=S_IFREG|0644, stx_size=0, ...}) = 0
copy_file_range(3, NULL, 4, NULL, 1073741824, 0) = 13
copy_file_range(3, NULL, 4, NULL, 1073741824, 0) = 0
close(4)                                = 0
close(3)                                = 0

But there are also some issues with this. It's currently unclear how macOS should implement it, since macOS's io::copy() just awkwardly copies byte by byte without using fclonefileat or fcopyfile.

That said, I'm not even sure if this problem exists on macOS, as no one has reported it yet.

Although Rust's fs::copy() implementation on macOS indeed treats failing to copy metadata as a hard error, it largely depends on whether the OS returns an error when these remote devices don't support metadata copying or fails silently.

Maybe we can try fixing it for Linux first. I'll create a PR for it later.

@sxyazi sxyazi removed the waiting on op Waiting for more information from the original poster label Oct 18, 2024
@sxyazi
Copy link
Owner

sxyazi commented Oct 18, 2024

Please try #1805

@ppenguin
Copy link
Author

Please try #1805

Thanks for looking into this so quickly!
It has become better in that it actually copies the files successfully, nice!

BUT: in the progressbar an error is still reported (red) and the task never finishes (even after the files have actually been copied). The errors are still reported (per file?), which is no problem by itself, but needs an error/warning distinction (see below).

So the error handling is not yet fixed.

Also, it would probably be helpful in general if in the detailed report of a selected task (i.e. w -> <select> -> [enter]) the object (file) name would be listed in the error line.

It is probably necessary to distinguish between "errors" and "warnings", so that we could arrive at a task state where a task could be "finished with errors" or "finished with warnings".

/reopen

@sxyazi
Copy link
Owner

sxyazi commented Oct 19, 2024

Thanks for the test @ppenguin!

The errors are still reported

Can you elaborate a bit more? What error is it? Is it still the same os error 95, or has there been a change?

Are you using the latest commit df78827 (what does your yazi -V say) since I submitted a patch after that PR to fix a few edge cases. Also a video demo of the operation would be helpful!

@ppenguin
Copy link
Author

ppenguin commented Oct 20, 2024

Thanks for the test @ppenguin!

The errors are still reported

Can you elaborate a bit more? What error is it? Is it still the same os error 95, or has there been a change?

Yes, it's the same list of same errors 95 (it seems there's one error report for every file if I select task details)

Are you using the latest commit df78827 (what does your yazi -V say) since I submitted a patch after that PR to fix a few edge cases. Also a video demo of the operation would be helpful!

> yazi -V
Yazi 0.3.3 (24a77d8 2024-10-19)

(last commit on master as of yesterday, two newer than the fix).

A side by side test "before/after" the fix:

Before:
image
image
(Note that the status remains at 0%, files are created but remain 0 B)
image

After
image

Now it gets interesting, with the same (limited) file set, everything is copied, even without errors, but some of the copied files are shown as empty, even if they're not (so a "refresh" would be necessary).

With another file set (nested dirs and more/larger files), I get this:
image
(note the red progress info, it gets stuck at 90%, even though everything was copied successfully => some files were non-zero length, but not fully copied!!!)
image

So in other words: the number of files for this case that is successfully copied went from "zero" to "more", but doesn't reliably work.

You might be able to reproduce this behaviour yourself by mounting an ntfs formatted external harddisk and a remote smb share and then copy files from the former to the latter.

@sxyazi
Copy link
Owner

sxyazi commented Oct 20, 2024

This is really strange, I've checked the code several times and can't find any errors, basically, Yazi is just using io::copy() on Linux instead of fs::copy():

#[cfg(any(target_os = "linux", target_os = "android"))]
{
use std::os::fd::AsRawFd;
let mut reader = fs::File::open(from).await?;
let mut writer = fs::OpenOptions::new()
.mode(cha.perm as u32)
.write(true)
.create(true)
.truncate(true)
.open(to)
.await?;
written = io::copy(&mut reader, &mut writer).await?;
let writer = writer.into_std().await;
_ = tokio::task::spawn_blocking(move || {
unsafe { libc::fchmod(writer.as_raw_fd(), cha.perm) };
writer.set_times(ft).ok();
})
.await;
}

My best guess is that std::io::copy() itself isn't working. If that's the case, there's really nothing more Yazi can do since this is the lowest-level API provided by Rust.

It could also be related to your environment. If possible, I suggest trying to call std::io::copy() directly in a Rust app to see if it works.

I'll mark this issue as "help wanted" and reopen it for a while to see if anyone can reproduce it on their system and debug it at the code level. Any help would be greatly appreciated! 🙏🏻

@sxyazi sxyazi reopened this Oct 20, 2024
@sxyazi sxyazi added help wanted Extra attention is needed cant reproduce The issue cannot be reproduced as described labels Oct 20, 2024
@ppenguin
Copy link
Author

ppenguin commented Oct 20, 2024

Just as some extra info:

if I just do cp -av between the same src/tgt I get errors like

  • cp: preserving permissions for ‘somefile.xxx’: Operation not supported for all files and
  • cp: failed to preserve ownership for 'somedir': Function not implemented for all dirs

But all files are copied completely.

This probably means that in the above code (I don't know any rust) the part where you are handling permissions in the target (unsafe { libc::fchmod(writer.as_raw_fd(), cha.perm) };) will probably also not be successful. This is probably the place where a "best effort" should be made to finish the operation and throw a warning instead of an error.

Indeed, since both operations are async (copying content is decoupled from metadata?), this may also explain how we can end up with incomplete files?

To make sure that the contents (everything else than the metadata) has been indeed copied without difference, I'm not sure whether the part on L268~269 guarantees enough, or whether we'd need more checking?

Further, inspired by this thread, one might consider to make the metadata part a bit more elaborate, basically trying to copy different attributes from the metadata separately and allow failure for them (handle as warning). You could maybe even handle this in "fall-through" fashion, first try everything and then less, based on what errors come back based on the target file system.

@sxyazi
Copy link
Owner

sxyazi commented Oct 20, 2024

Yes, the code above runs exactly as you described, with only the io::copy on line 268 being considered an error.

The subsequent libc::fchmod (for setting permissions) and set_times (for setting file access and modification times) attempt to execute as best as they can and fail silently. This means that even if they both fail, Yazi doesn’t see it as an error.

So, I really can’t imagine what could be causing the issue besides a problem with io::copy() itself. If it is indeed io::copy(), it likely means there’s no real "effective fix" for this issue since it’s the lowest-level API available to Yazi, and in Rust internally, it’s just a wrapper around the copy_file_range [1], and sendfile [2] system call:

@ppenguin
Copy link
Author

ppenguin commented Oct 20, 2024

So, I really can’t imagine what could be causing the issue besides a problem with io::copy() itself. If it is indeed io::copy(), it likely means there’s no real "effective fix" for this issue since it’s the lowest-level API available to Yazi, and in Rust internally, it’s just a wrapper around the copy_file_range [1], and sendfile [2] system call:

I just took a quick look for fun and education at the source of the "original" cp command (from coreutils) and surprise: it's thousands of loc long!
In other words, I'm not sure we should be looking for "the lowest level API" but rather a higher level one, that is actually capable of handling all the complex ACL etc. stuff reliably. Hope this exists?

(I'd say this is pretty much mandatory, a copy program that can't handle the same as the cp baseline is ... well, not yet finished, I'm afraid (Same with e.g. this one)

@sxyazi
Copy link
Owner

sxyazi commented Oct 20, 2024

that is actually capable of handling all the complex ACL etc. stuff reliably. Hope this exists?

Yeah, I would love to switch to such an API if it exists and would accept a PR as long as it's reliable and implemented in Rust — I don't want to introduce a C library because that would complicate the build.

I noticed that another issue you raised with rcp describes the same problem as Yazi wykurz/rcp#26, which likely means this issue affects all Rust programs using std::fs::copy() or std::io::copy() - I took a look at the source of rcp, and it calls tokio::fs::copy(), which is an async wrapper of std::fs::copy().

In this case, I'm going to close this issue since it's beyond what Yazi can handle and also not just specific to Yazi. I really suggest testing std::io::copy() on your system, it's pretty easy to do, if that fails as well, it's best to report it to Rust so all Rust programs can benefit, not just Yazi.

@sxyazi sxyazi closed this as not planned Won't fix, can't repro, duplicate, stale Oct 20, 2024
@ppenguin
Copy link
Author

I noticed that another issue you raised with rcp describes the same problem as Yazi [wykurz/rcp#26]

I actually raised that to make the point that probably many/most rust based programs suffer from this

In this case, I'm going to close this issue since it's beyond what Yazi can handle and also not just specific to Yazi. I really

I understand, but this is a huge thing IMO, which would warrant a big red sticker on the product saying it's severly limited and only works on a limited set of file systems.

suggest testing std::io::copy() on your system, it's pretty easy to do, if that fails as well, it's best to report it to Rust so all

Basically I think we have already proven that both io::copy and fs::copy just can't handle this, so to test this again may be unnecessary. But I'll see what I can do when I happen to be bored 😜 (don't hold your breath though)

@sxyazi
Copy link
Owner

sxyazi commented Oct 21, 2024

I just read the tokio source and noticed that its tokio::io::copy() isn't just a simple wrapper of std::io::copy(), but rather it implements its own userspace read-write loop.

I'm not sure if this is causing the issue, but I replaced it with std::io::copy() in #1817. Please give it a try; this is my last-ditch effort on this problem, hope it makes a difference 🤷🏻

@ppenguin
Copy link
Author

ppenguin commented Oct 22, 2024

Yes, I can confirm that #1817 works for the above test case!
This is a keeper 👍
Thanks for your persistent efforts!

Copy link

I'm going to lock this issue because it has been closed for 30 days. ⏳
This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please file a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working cant reproduce The issue cannot be reproduced as described help wanted Extra attention is needed
Projects
None yet
3 participants