Skip to content

Use less syscalls in FileDesc::set_{nonblocking,cloexec} #39514

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Feb 5, 2017

Conversation

tbu-
Copy link
Contributor

@tbu- tbu- commented Feb 4, 2017

Only set the flags if they differ from what the OS reported, use
FIONBIO to atomically set the non-blocking IO flag on Linux.

Only set the flags if they differ from what the OS reported, use
`FIONBIO` to atomically set the non-blocking IO flag on Linux.
@rust-highfive
Copy link
Contributor

r? @aturon

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton
Copy link
Member

@bors: r+

Thanks!

@bors
Copy link
Collaborator

bors commented Feb 4, 2017

📌 Commit efeb42b has been approved by alexcrichton

@nagisa
Copy link
Member

nagisa commented Feb 4, 2017

FIONBIO is supported by pretty much every recent BSD as well.

@tbu-
Copy link
Contributor Author

tbu- commented Feb 4, 2017

@nagisa It's hard to find docs on ioctls, do you know any sources?

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Feb 5, 2017
…chton

Use less syscalls in `FileDesc::set_{nonblocking,cloexec}`

Only set the flags if they differ from what the OS reported, use
`FIONBIO` to atomically set the non-blocking IO flag on Linux.
frewsxcv added a commit to frewsxcv/rust that referenced this pull request Feb 5, 2017
…chton

Use less syscalls in `FileDesc::set_{nonblocking,cloexec}`

Only set the flags if they differ from what the OS reported, use
`FIONBIO` to atomically set the non-blocking IO flag on Linux.
frewsxcv added a commit to frewsxcv/rust that referenced this pull request Feb 5, 2017
…chton

Use less syscalls in `FileDesc::set_{nonblocking,cloexec}`

Only set the flags if they differ from what the OS reported, use
`FIONBIO` to atomically set the non-blocking IO flag on Linux.
bors added a commit that referenced this pull request Feb 5, 2017
Rollup of 12 pull requests

- Successful merges: #39439, #39472, #39481, #39491, #39501, #39509, #39514, #39519, #39526, #39528, #39530, #39538
- Failed merges:
@bors bors merged commit efeb42b into rust-lang:master Feb 5, 2017
@nagisa
Copy link
Member

nagisa commented Feb 6, 2017

@tbu- not really. This is a pretty obscure API and hence rarely documented. I know it to work because I had opportunity to test it out in an actual system.

mio may be a good indicator it “works” as it uses this API unconditionally.

@joshtriplett
Copy link
Member

You may also want to document, in the documentation for set_cloexec, that you often want to open with cloexec already set rather than setting it afterward, and show how to do that. (Doing it within open avoids a race that can leak file descriptors to child processes.)

@tbu-
Copy link
Contributor Author

tbu- commented Feb 7, 2017

@joshtriplett This is private API as far as I can tell, and it's only used in places where we already pass O_CLOEXEC. It's apparantly just for old kernels which are either unaware of O_CLOEXEC or have a bug relating to it.

@joshtriplett
Copy link
Member

@tbu- Ah, OK; thanks for the clarification.

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

Successfully merging this pull request may close these issues.

7 participants