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

std::io::copy returns Bad File Descriptor with writer files in append mode in 1.50 #82410

Closed
kurojishi opened this issue Feb 22, 2021 · 11 comments · Fixed by #82417
Closed

std::io::copy returns Bad File Descriptor with writer files in append mode in 1.50 #82410

kurojishi opened this issue Feb 22, 2021 · 11 comments · Fixed by #82417
Assignees
Labels
A-io Area: `std::io`, `std::fs`, `std::net` and `std::path` C-bug Category: This is a bug. O-linux Operating system: Linux P-critical Critical priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@kurojishi
Copy link

kurojishi commented Feb 22, 2021

While updating my code base from 1.49 to 1.50 we noticed a failures in a program that rebuilds files from small chunks.

We are opening a file in append mode, one in read mode, and we stream one into the other (as for the snippet below).

Testing this out confirmed that the error happens on the reader file when the writer is in append mode, whenever we use a BufWriter/BufReader or not. If we load the whole file into a string and then use std::io::copy on the same data it works as intended, as does setting the file in write mode (while it becomes necessary to seek to the end).

Checking what got shipped into 1.50 we suspect the probable culprit to be 028754a#diff-4280ab12d5278289ca8b2e83cad374850eaeac0c18f49a474f5a9b5bf55d3991

What we are guessing is that this change is using the sendfile syscall that doesn't support files in append mode even when the file is in append mode, triggering the Bad File Descriptor error

https://github.com/torvalds/linux/blob/b52bb135aad99deea9bfe5f050c3295b049adc87/fs/read_write.c#L1676

Code

I tried this code (playground):

use std::fs::{File, OpenOptions};
use std::io::prelude::*;

fn main() -> std::io::Result<()> {
    write_file("first.txt")?;
    write_file("second.txt")?;
    println!("{}", read_file("first.txt")?);
    copy_file("first.txt", "second.txt")?;
    Ok(())
}

fn write_file(name: &str) -> std::io::Result<()> {
    let mut file = File::create(name)?;
    file.write_all(b"Hello, world!")?;
    Ok(())
}

fn read_file(name: &str) -> std::io::Result<String> {
    let mut file = File::open(name)?;
    let mut contents = String::new();
    file.read_to_string(&mut contents)?;
    Ok(contents)
}

fn copy_file(from: &str, to: &str) -> std::io::Result<()> {
    let mut source = OpenOptions::new().read(true).open(from)?;
    let mut dest = OpenOptions::new().append(true).open(to)?;
    let bytes_written = std::io::copy(&mut source, &mut dest)?;
    println!("Copied {} bytes", bytes_written);
    std::fs::remove_file(from)?;
    Ok(())
}

I expected to see this happen: the files to be concatenated

Instead, this happened: std::io::copy returns Error: Os { code: 9, kind: Other, message: "Bad file descriptor" }

Strace for the code snipped:

open("first.txt", O_RDONLY|O_CLOEXEC)   = 3
open("second.txt", O_WRONLY|O_APPEND|O_CLOEXEC) = 4
futex(0x7f376caad0ec, FUTEX_WAKE_PRIVATE, 2147483647) = 0
syscall_332(0, 0, 0, 0xfff, 0, 0)       = -1 (errno 38)
fstat(3, {st_mode=S_IFREG|0644, st_size=13, ...}) = 0
syscall_326(0xffffffff, 0, 0xffffffff, 0, 0x1, 0) = -1 (errno 9)
syscall_326(0x3, 0, 0x4, 0, 0x40000000, 0) = -1 (errno 9)
close(4)                                = 0
close(3)                                = 0

Example that works with loading the file into a string before copy (playground):

use std::fs::{File, OpenOptions};
use std::io::BufReader;
use std::io::BufWriter;
use std::io::Read;
use std::io::prelude::*;
fn main() -> std::io::Result<()> {
    write_file("first.txt")?;
    write_file("second.txt")?;
    println!("{}", read_file("first.txt")?);
    copy_file("first.txt", "second.txt")?;
    println!("destination data {}", read_file("second.txt")?);
    Ok(())
}
fn write_file(name: &str) -> std::io::Result<()> {
    let mut file = File::create(name)?;
    file.write_all(b"Hello, world!")?;
    Ok(())
}
fn read_file(name: &str) -> std::io::Result<String> {
    let mut file = File::open(name)?;
    let mut contents = String::new();
    file.read_to_string(&mut contents)?;
    Ok(contents)
}
fn copy_file(from: &str, to: &str) -> std::io::Result<()> {
    let mut source = BufReader::new(OpenOptions::new().read(true).open(from)?);
    let mut source_str = String::new();
    source.read_to_string(&mut source_str)?;
    let mut reader = BufReader::new(source_str.as_bytes());
    let mut dest = BufWriter::new(OpenOptions::new().append(true).open(to)?);
    let bytes_written = std::io::copy(&mut reader, &mut dest)?;
    println!("Copied {} bytes", bytes_written);
    std::fs::remove_file(from)?;
    Ok(())
}

Version it worked on

It most recently worked on: 1.49

Version with regression

1.50
rustc --version --verbose:

rustc 1.50.0 (cb75ad5db 2021-02-10)
binary: rustc
commit-hash: cb75ad5db02783e8b0222fee363c5f63f7e2cf5b
commit-date: 2021-02-10
host: x86_64-unknown-linux-gnu
release: 1.50.0

Backtrace

no backtrace

@kurojishi kurojishi changed the title std::io::copy returns Bad File Descritor with reader files in append mode in 1.50 std::io::copy returns Bad File Descritor with writer files in append mode in 1.50 Feb 22, 2021
@kurojishi kurojishi changed the title std::io::copy returns Bad File Descritor with writer files in append mode in 1.50 std::io::copy returns Bad File Descriptor with writer files in append mode in 1.50 Feb 22, 2021
@the8472
Copy link
Member

the8472 commented Feb 22, 2021

syscall_326 is copy_file_range. And indeed, it has this error code overloading the more common meaning of EBADF.

EBADF The O_APPEND flag is set for the open file description (see open(2)) referred to by the file descriptor fd_out.

@rustbot assign.

@rustbot
Copy link
Collaborator

rustbot commented Feb 22, 2021

Error: Parsing assign command in comment failed: ...'ot assign.' | error: specify user to assign to at >| ''...

Please let @rust-lang/release know if you're having trouble with this bot.

@the8472
Copy link
Member

the8472 commented Feb 22, 2021

@rustbot claim

@the8472
Copy link
Member

the8472 commented Feb 22, 2021

If you need a workaround for the moment you can do io::copy(..., &mut writer as &mut dyn Write) which should defeat the incorrect optimizations by going through a trait object so it can't see it's dealing with a file.

@crisidev
Copy link

Nice, this is awesome. We will try this tomorrow which should allow to migrate our codebase to 1.50! Thanks a lot for the quick help on this..

@kurojishi
Copy link
Author

Test out the workaround and it works, while we wait for the actual fix to come out, thanks a lot!

@the8472
Copy link
Member

the8472 commented Feb 24, 2021

@rustbot label +T-libs-impl +I-prioritize +regression-from-stable-to-stable +C-bug +O-linux

workaround is available. fix needs review.

@rustbot rustbot added C-bug Category: This is a bug. I-prioritize Issue: Indicates that prioritization has been requested for this issue. O-linux Operating system: Linux T-libs Relevant to the library team, which will review and decide on the PR/issue. regression-from-stable-to-stable Performance or correctness regression from one stable version to another. labels Feb 24, 2021
@LeSeulArtichaut LeSeulArtichaut added the A-io Area: `std::io`, `std::fs`, `std::net` and `std::path` label Feb 24, 2021
@apiraino
Copy link
Contributor

Assigning P-critical as discussed as part of the Prioritization Working Group procedure and removing I-prioritize.

@rustbot label -I-prioritize +P-critical

@rustbot rustbot added P-critical Critical priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Feb 24, 2021
@Kixunil

This comment has been minimized.

@the8472

This comment has been minimized.

@Kixunil

This comment has been minimized.

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Mar 11, 2021
… r=m-ou-se

Fix io::copy specialization using copy_file_range when writer was opened with O_APPEND

fixes rust-lang#82410

While `sendfile()` returns `EINVAL` when the output was opened with O_APPEND,  `copy_file_range()` does not and returns `EBADF` instead, which – unlike other `EBADF` causes – is not fatal for this operation since a regular `write()` will likely succeed.

We now treat `EBADF` as a non-fatal error for `copy_file_range` and fall back to a read-write copy as we already did for several other errors.
@bors bors closed this as completed in 03e864f Mar 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-io Area: `std::io`, `std::fs`, `std::net` and `std::path` C-bug Category: This is a bug. O-linux Operating system: Linux P-critical Critical priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants