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

libnative: process spawning should not close inherited file descriptors #16421

Merged
merged 1 commit into from
Aug 13, 2014
Merged

libnative: process spawning should not close inherited file descriptors #16421

merged 1 commit into from
Aug 13, 2014

Conversation

ipetkov
Copy link
Contributor

@ipetkov ipetkov commented Aug 11, 2014

Retry pull requesting of #16407 after accidentally messing up rebasing of branches and making bors think the PR was merged

let result = file.read_to_string().unwrap();
assert_eq!("hello world\nextra write\n", result.as_slice());
assert!(io::fs::unlink(&path).is_ok());
}
Copy link
Member

Choose a reason for hiding this comment

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

Could you add this test to std::io::process instead? The iotest! macro will allow the test to be run under both libnative and libgreen (to ensure consistency)

@ipetkov
Copy link
Contributor Author

ipetkov commented Aug 12, 2014

@alexcrichton updated!

* The caller should be responsible for cleaning up file descriptors
* If a caller safely creates a file descriptor (via
  native::io::file::open) the returned structure (FileDesc) will try to
  clean up the file, failing in the process and writing error messages
  to the screen.
* This should not happen as the caller has no public interface for
  telling the FileDesc structure to NOT free the underlying fd.
* Alternatively, if another file is opened under the same fd held by
  the FileDesc structure returned by native::io::file::open, it will
  close the wrong file upon destruction.
@ipetkov
Copy link
Contributor Author

ipetkov commented Aug 13, 2014

@alexcrichton I updated the PR since the test I added would fail randomly on my machine for some reason. Rather than write to a file it writes to /dev/null and checks that a subsequent write to the file descriptor succeeds.

This seems to work well on my machine now (OS X), hopefully it works for other platforms as well!

bors added a commit that referenced this pull request Aug 13, 2014
Retry pull requesting of #16407 after accidentally messing up rebasing of branches and making bors think the PR was merged
@bors bors closed this Aug 13, 2014
@bors bors merged commit 3fe0ba9 into rust-lang:master Aug 13, 2014
@ipetkov ipetkov deleted the cmd-fd-fix-retry branch August 16, 2014 04:13
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 3, 2024
…ce, r=Veykril

fix: Wrong closure kind deduction for closures with predicates

Completes rust-lang#16472, fixes rust-lang#16421

The changed closure kind deduction is mostly simlar to `rustc_hir_typeck/src/closure.rs`.
Porting closure sig deduction from it seems possible too and I'm considering doing it with another PR
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