Skip to content

Fix IO safety runtime errors in tests #272

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

Merged
merged 2 commits into from
Aug 12, 2024
Merged

Fix IO safety runtime errors in tests #272

merged 2 commits into from
Aug 12, 2024

Conversation

alyssais
Copy link
Contributor

Summary of the PR

If debug_assertions is enabled, Rust 1.80.0 and later will abort the process when attempting to close a file descriptor that is already closed. This means that, in tests, where an invalid file descriptor object is deliberately constructed, we have to avoid that object being dropped. Using into_raw_fd() allows the object's memory to be freed (unlike std::mem::forget()), without attempting to close the file descriptor.

Requirements

Before submitting your PR, please make sure you addressed the following
requirements:

  • All commits in this PR have Signed-Off-By trailers (with
    git commit -s), and the commit message has max 60 characters for the
    summary and max 75 characters for each description line.
  • All added/changed functionality has a corresponding unit/integration
    test.
  • All added/changed public-facing functionality has entries in the "Upcoming
    Release" section of CHANGELOG.md (if no such section exists, please create one).
  • Any newly added unsafe code is properly documented.

If debug_assertions is enabled, Rust 1.80.0 and later will abort the
process when attempting to close a file descriptor that is already
closed[1].  This means that, in tests, where an invalid file
descriptor object is deliberately constructed, we have to avoid that
object being dropped.  Using into_raw_fd() allows the object's memory
to be freed (unlike std::mem::forget()), without attempting to close
the file descriptor.

[1]: rust-lang/rust@cb49406

Signed-off-by: Alyssa Ross <hi@alyssa.is>
Copy link
Member

@roypat roypat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this! I'm using this as an opportunity to also update our CI toolchain to 1.80.1 (rust-vmm/rust-vmm-container#111) so that we (a) don't regress on these again, and (b) potentially find and fix other issues before downstream users discover them :)

@roypat roypat merged commit a56194d into rust-vmm:main Aug 12, 2024
19 checks passed
@alyssais alyssais deleted the io branch August 12, 2024 16:05
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 this pull request may close these issues.

3 participants