Skip to content

PollFd implementing Copy is a footgun #2630

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

Closed
JonathonReinhart opened this issue Apr 14, 2025 · 1 comment · Fixed by #2631
Closed

PollFd implementing Copy is a footgun #2630

JonathonReinhart opened this issue Apr 14, 2025 · 1 comment · Fixed by #2631

Comments

@JonathonReinhart
Copy link
Contributor

I think this code speaks for itself:

fn example(file: &std::fs::File) -> io::Result<Option<()>> {
    use nix::poll::{poll, PollFd, PollFlags, PollTimeout};
    use std::os::fd::AsFd;

    let poll_fd = PollFd::new(file.as_fd(), PollFlags::POLLIN);

    let mut poll_fds = [poll_fd];                          // <<<< Copy happens here

    let poll_res = poll(&mut poll_fds, PollTimeout::NONE)?;
    if poll_res == 0 {
        // Timeout
        return Ok(None);
    }
    assert_eq!(poll_res, 1);

    // This fails because poll_fd is not the object that was actually updated
    //assert_eq!(poll_fd.any(), Some(true));

    // You have to access the item in the array
    assert_eq!(poll_fds[0].any(), Some(true));

    Ok(Some(()))
}

The fact that PollFd implements Copy makes it easy to make this mistake.

Normally, Rust doesn't let you make mistakes like this. If PollFd didn't implement Copy, rust would complain if you tried to write this code. You'd have to call .clone() and then there would be an explicit clue that you're not operating on the same object.

error[E0382]: borrow of moved value: `poll_fd`
  --> src/main.rs:23:16
   |
11 |     let poll_fd = PollFd::new(file.as_fd(), PollFlags::POLLIN);
   |         ------- move occurs because `poll_fd` has type `PollFd<'_>`, which does not implement the `Copy` trait
12 |
13 |     let mut poll_fds = [poll_fd];                          // <<<< Copy happens here
   |                         ------- value moved here
...
23 |     assert_eq!(poll_fd.any(), Some(true));
   |                ^^^^^^^ value borrowed here after move
   |
help: consider cloning the value if the performance cost is acceptable
   |
13 |     let mut poll_fds = [poll_fd.clone()];                          // <<<< Copy happens here
   |                                ++++++++
JonathonReinhart added a commit to JonathonReinhart/nix that referenced this issue Apr 14, 2025
PollFd implementing Copy makes it easy to accidentally refer to the
wrong object after putting one into an array.

This fixes nix-rust#2630
@asomers
Copy link
Member

asomers commented Apr 14, 2025

Good point.

JonathonReinhart added a commit to JonathonReinhart/nix that referenced this issue Apr 15, 2025
PollFd implementing Copy makes it easy to accidentally refer to the
wrong object after putting one into an array. Remove Copy to force move
semantics.

This also updates some related docs to improve overall clarity.

This fixes nix-rust#2630
JonathonReinhart added a commit to JonathonReinhart/nix that referenced this issue Apr 15, 2025
PollFd implementing Copy makes it easy to accidentally refer to the
wrong object after putting one into an array. Remove Copy to force move
semantics.

This also updates some related docs to improve overall clarity.

This fixes nix-rust#2630
JonathonReinhart added a commit to JonathonReinhart/nix that referenced this issue Apr 15, 2025
PollFd implementing Copy makes it easy to accidentally refer to the
wrong object after putting one into an array. Remove Copy to force move
semantics.

This also updates some related docs to improve overall clarity.

This fixes nix-rust#2630
JonathonReinhart added a commit to JonathonReinhart/nix that referenced this issue Apr 15, 2025
PollFd implementing Copy makes it easy to accidentally refer to the
wrong object after putting one into an array. Remove Copy to force move
semantics.

This also updates some related docs to improve overall clarity.

This fixes nix-rust#2630
JonathonReinhart added a commit to JonathonReinhart/nix that referenced this issue Apr 17, 2025
PollFd implementing Copy makes it easy to accidentally refer to the
wrong object after putting one into an array. Remove Copy to force move
semantics.

This also updates some related docs to improve overall clarity.

This fixes nix-rust#2630
JonathonReinhart added a commit to JonathonReinhart/nix that referenced this issue Apr 22, 2025
PollFd implementing Copy makes it easy to accidentally refer to the
wrong object after putting one into an array. Remove Copy to force move
semantics.

This also updates some related docs to improve overall clarity.

This fixes nix-rust#2630
github-merge-queue bot pushed a commit that referenced this issue Apr 22, 2025
PollFd implementing Copy makes it easy to accidentally refer to the
wrong object after putting one into an array. Remove Copy to force move
semantics.

This also updates some related docs to improve overall clarity.

This fixes #2630
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

Successfully merging a pull request may close this issue.

2 participants