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

Not receiving write failures in Win32 when writing to UNC network shares #151

Open
kevinhoffman opened this issue Aug 18, 2021 · 3 comments

Comments

@kevinhoffman
Copy link

We are using tempfile crate in a Windows application that is creating temp files on network share (e.g., \\10.2.180.35\ra2050\file), is writing some data to the tempfile, and then is persisting that tempfile. The network share itself is powered by some Linux NAS device (outside of our control, the network share location/device is chosen by our customers).

When the network share was approaching 100% full disk space, we discovered that we could write data to the tempfile, persist the file, and close the file handle without any failures from Win32, but the data was not actually being written by the NAS device (the data was either only partially written and truncated, or was not written at all, depending on how full the NAS device was).

This is not the fault of the tempfile crate or Rust. We reproduced the issue with a plain C application making Win32 calls, where we open a file on the NAS device with the following code:

const HANDLE file_handle = CreateFileW(
     L"\\\\10.2.180.35\\ra2050\\file", // name of the write
     GENERIC_WRITE | GENERIC_READ,     // open for writing and reading
     0,                                // do not share
     nullptr,                          // default security
     CREATE_NEW,                       // create new file only
     FILE_ATTRIBUTE_NORMAL,            // normal file
     nullptr);                         // no attr. template

We could successfully write data to this file handle and also close the file handle without any error, even though the NAS filesystem was full. This is a serious problem as it leads to data loss as we are unable to recognize that the data could not be written because the NAS filesystem is full. In our experimentation, if we remove | GENERIC_READ so that the file is only opened for writing, then we will properly get win32 error 112 during writing.

This is likely the fault of the NAS device not properly implementing the CIFS/SMB protocol, or perhaps it's actually a bug in the Windows SMB implementation that doesn't propagate an error (less likely). We cannot control either, and so for us the solution is to add a new method to tempfile crate to open a file only with write permissions (even though normally most applications likely only write data to tempfiles, we didn't want to suggest a breaking change...). We're testing this approach now, and once tested will open a PR with our changes for discussion.

@Stebalien
Copy link
Owner

I'll take a look at the patch. But please make sure it's generally useful. E.g., extends the tempfile builder to allow one to specify permissions/modes etc, not a one-off function to fix your issue.

Also, have you tried calling sync before closing? Not returning errors on write when the disk is full (a rare condition) is pretty reasonable if the alternative is to block on a network round-trip.

even though normally most applications likely only write data to tempfiles, we didn't want to suggest a breaking change...

No? Most applications use temporary files as scratch spaces, requiring both read and write support.

@kevinhoffman
Copy link
Author

I'll take a look at the patch. But please make sure it's generally useful. E.g., extends the tempfile builder to allow one to specify permissions/modes etc, not a one-off function to fix your issue.

Yes, the Builder already has a similar method append that changes the OpenOptions so this can be structured in a similar way. The create_named function is always forcing .read(true) in the passed OpenOptions right now so that needs a little rework in the PR.

Also, have you tried calling sync before closing? Not returning errors on write when the disk is full (a rare condition) is pretty reasonable if the alternative is to block on a network round-trip.

Yes, we were calling sync_data on the file before closing and that was not failing either. We were also using a technique to manually close the file handle associated with the rust file, and that close call was also succeeding as well. (In our C program that reproduced it, we also verified this as well... both sync and close were succeeding.)

@jasonwhite
Copy link
Contributor

@kevinhoffman If you're still having this problem, then maybe PR #177 would let you create the file with write-only permissions. For example, this should do what you want:

use std::fs::OpenOptions;
use std::os::windows::fs::OpenOptionsExt;

let tempfile = tempfile::Builder::new()
    .make(|path| {
        OpenOptions::new()
            .create_new(true)
            .write(true)
            .custom_flags(winapi::um::winnt::FILE_ATTRIBUTE_TEMPORARY)
            .open(path)
    })?;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants