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

Why doesn't tokio::process::ChildStdin implements IntoRawFd #4403

Closed
NobodyXu opened this issue Jan 14, 2022 · 10 comments · Fixed by #5899
Closed

Why doesn't tokio::process::ChildStdin implements IntoRawFd #4403

NobodyXu opened this issue Jan 14, 2022 · 10 comments · Fixed by #5899
Labels
A-tokio Area: The main tokio crate M-process Module: tokio/process

Comments

@NobodyXu
Copy link
Contributor

Since tokio::process::ChildStdin already implements AsRawFd, it seems to be only natural that it also implements IntoRawFd like std::process::ChildStdin.

What is the reason for it to not implement IntoRawFd?

@Darksonn Darksonn added A-tokio Area: The main tokio crate M-process Module: tokio/process labels Jan 14, 2022
@Darksonn
Copy link
Contributor

Well, the conversion is fallible, which the trait doesn't support.

@NobodyXu
Copy link
Contributor Author

Is it because the conversion into RawFd requires the O_NONBLOCKING flags to be removed?

@Darksonn
Copy link
Contributor

Yes, but also because its registration with epoll needs to be removed.

@NobodyXu
Copy link
Contributor Author

OK, I got it.

Is it possible to have some alternative API that can return the fd along with its ownership?

I personally is OK with the fd being O_NONBLOCKING.

@Darksonn
Copy link
Contributor

Sure, we could have an into_std function.

@NobodyXu
Copy link
Contributor Author

Sure, we could have an into_std function.

I don’t think we can support “into_std” since “ChildStdin” doesn’t support FromRawFd.

@Darksonn
Copy link
Contributor

Ok, I guess that's why we don't have it already. I'm sure we could find some other alternative.

@NobodyXu
Copy link
Contributor Author

Maybe something like fn to_owned_fd(self) -> Result<io_lifetimes::OwnedFd, io::Error> where io_lifetimes::OwnedFd is implementation of the unstable std::os::unix::io::OwnedFd?

@Darksonn
Copy link
Contributor

We would probably use the same return type as the IntoRawFd trait. We are not going to add a dependency on io_lifetimes for this, nor are we going to define a similar type in Tokio just for the stdio types.

@NobodyXu
Copy link
Contributor Author

I agree that introduce a new dependency is too much and using “RawFd” is fair enough.

I think having a new unix-only trait for this is the way to go, since “ChildStdin”, “ChildStdout” and “ChildStderr” will all have this method implemented.

NobodyXu added a commit to NobodyXu/tokio that referenced this issue Jul 30, 2023
NobodyXu added a commit to NobodyXu/tokio that referenced this issue Jul 30, 2023
NobodyXu added a commit to NobodyXu/tokio that referenced this issue Jul 31, 2023
NobodyXu added a commit to NobodyXu/tokio that referenced this issue Aug 2, 2023
NobodyXu added a commit to NobodyXu/tokio that referenced this issue Aug 4, 2023
NobodyXu added a commit to NobodyXu/tokio that referenced this issue Aug 5, 2023
NobodyXu added a commit to NobodyXu/tokio that referenced this issue Aug 6, 2023
NobodyXu added a commit to NobodyXu/tokio that referenced this issue Aug 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate M-process Module: tokio/process
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants