From dc55144b85d01ce84e0b0c3cfdc12da39bd437d7 Mon Sep 17 00:00:00 2001 From: Ivan Petkov Date: Thu, 9 Apr 2015 22:04:41 -0700 Subject: [PATCH 01/11] RFC for redirecting stdio of child processes to open file handles --- text/0000-process-stdio-redirection.md | 93 ++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) create mode 100644 text/0000-process-stdio-redirection.md diff --git a/text/0000-process-stdio-redirection.md b/text/0000-process-stdio-redirection.md new file mode 100644 index 00000000000..2c71eb12e6a --- /dev/null +++ b/text/0000-process-stdio-redirection.md @@ -0,0 +1,93 @@ +- Feature Name: process-stdio-redirection +- Start Date: 2015-04-10 +- RFC PR: +- Rust Issue: + +# Summary + +Update the `std::process` API with the ability to redirect stdio of child +processes to any opened file handle. + +# Motivation + +The current API in `std::process` allows to either pipe stdio between parent and +child process or redirect stdio to `/dev/null`. It would also be largely useful +to allow stdio redirection to any currently opened `std::fs::File` (henceforth +`File`) handle. This would allow redirecting stdio with a physical file or even +another process (via OS pipe) without forcing the parent process to buffer the +data itself. + +For example, one may wish to spawn a process which prints gigabytes +of data (e.g. logs) and use another process to filter through it, and save the +result to a file. The current API would force the parent process to dedicate a +thread just to buffer all data between child processes and disk, which is +impractical given that the OS can stream the data for us via pipes and file +redirection. + +# Detailed design + +First, the standard library should provide an OS agnostic way of creating OS +in-memory pipes, i.e. a `Pipe`, providing reader and writer handles as `File`s. +This would avoid the need for users to write OS specific (and `unsafe`) code +while retaining all the benefits offered by `File`. This proposal considers +making the `Pipe`'s reader and writer fields public to allow easily moving +ownership of the two handles, but any other appropriate interface is acceptable. + +```rust +pub struct Pipe { + pub reader: File, + pub writer: File, +} +``` + +Next, `std::process::Stdio` should provide a `redirect` method which accepts and +stores a `&File`, ensuring the underlying handle will not go out of scope +unexpectedly before the child is spawned. The spawning implementation can then +extract and use the `File`'s OS specific handle when creating the child +process. + +This `File` reference should be an immutable one, to allow "reuse" of the handle +across several `Command`s simultaneously. This, however, can allow code to +indirectly mutate a `File` through an immutable reference by passing it on to a +child process, although retrieving and mutating through the underlying OS handle +(via `AsRaw{Fd, Socket, Handle}`) is already possible through a `&File`. Thus +this API would not introduce any "mutability leaks" of `File`s that were not +already present. + +This design also offers benefits when the user may wish to close (drop) a +`File`, for example, closing the read end of a pipe and sending EOF to its +child. The compiler can infer the lifetimes of all references to the original +`File`, eliminating some guesswork on whether all ends of a pipe have been +closed. This would not be possible in a design which duplicates the OS handle +internally which could more easily lead to a deadlock (e.g. waiting on a child +to exit while the same scope holds an open pipe to the child's stdin). + +Reclaiming ownership of the borrowed `File` may require locally scoping the +creation of `Stdio` or `Command` instances to force their lifetimes to end, +however, this is minimally intrusive compared to alternative designs. + +# Drawbacks + +Implementing a design based on `File` borrows will require adding lifetime +parameters on stabilized `Stdio` and `Command` close to the 1.0 release. + +# Alternatives + +Do nothing now and choose a stability compatible design, possibly being stuck +with less ergonomic APIs. + +One alternative strategy is to duplicate the underlying OS handle and have the +`std::process` APIs take ownership of the copy. When working with OS pipes, +however, the user would have to manually keep track where the duplicates have +gone if they wish to close them all; failing to do so may cause deadlocks. + +Another strategy would be for `Stdio` to take ownership of a `File` and wrap it +as a `Rc`, allowing it to be "reused" in any number of redirections by +cloning the `Stdio` wrapper. A caller could try to regain ownership (via +`try_unwrap` on the internal wrapper), but they would have to (manually) ensure +all other `Stdio` clones are dropped. This design would also suffer from +potential deadlocks, making it by far the least ergonomic option. + +# Unresolved questions + +None at the moment. From c854d082faf528ba70888a84fde929aed705263e Mon Sep 17 00:00:00 2001 From: Ivan Petkov Date: Sun, 12 Apr 2015 10:30:04 -0700 Subject: [PATCH 02/11] Improve and rework design * Don't use `File` as the underlying pipe handle, it is too limiting and carries file related assumptions. * Instead, define a new `Pipe` type whose reader/writer are wrappers over a fd/HANDLE. * Note how pipes will be opened in Unix and Windows * Propose new redirection API: based on ownership, defined for concrete types provided by the standard library --- text/0000-process-stdio-redirection.md | 135 +++++++++++++++---------- 1 file changed, 84 insertions(+), 51 deletions(-) diff --git a/text/0000-process-stdio-redirection.md b/text/0000-process-stdio-redirection.md index 2c71eb12e6a..43d479e4c95 100644 --- a/text/0000-process-stdio-redirection.md +++ b/text/0000-process-stdio-redirection.md @@ -6,16 +6,15 @@ # Summary Update the `std::process` API with the ability to redirect stdio of child -processes to any opened file handle. +processes to any opened file handle or equivalent. # Motivation The current API in `std::process` allows to either pipe stdio between parent and child process or redirect stdio to `/dev/null`. It would also be largely useful -to allow stdio redirection to any currently opened `std::fs::File` (henceforth -`File`) handle. This would allow redirecting stdio with a physical file or even -another process (via OS pipe) without forcing the parent process to buffer the -data itself. +to allow stdio redirection to any currently opened file or pipe handles. This +would allow redirecting stdio with a physical file or even another process (via +OS pipe) without forcing the parent process to buffer the data itself. For example, one may wish to spawn a process which prints gigabytes of data (e.g. logs) and use another process to filter through it, and save the @@ -27,66 +26,100 @@ redirection. # Detailed design First, the standard library should provide an OS agnostic way of creating OS -in-memory pipes, i.e. a `Pipe`, providing reader and writer handles as `File`s. -This would avoid the need for users to write OS specific (and `unsafe`) code -while retaining all the benefits offered by `File`. This proposal considers -making the `Pipe`'s reader and writer fields public to allow easily moving -ownership of the two handles, but any other appropriate interface is acceptable. +in-memory pipes, i.e. a `Pipe`, providing reader and writer handles as +`PipeReader` and `PipeWriter` respectively. The reader and writer will simply be +an abstraction over the OS specific file descriptor/HANDLE, and will implement +the respective `Read`/`Write` trait, making it impossible to confuse the two +together. In addition, each should implement the appropriate `AsRaw{Fd, +Handle}`/`FromRaw{Fd, Handle}` for wrapping/unwrapping of the OS handles. + +On Unix systems the pipe can be created with `libc::pipe` and the file +descriptors wrapped as appropriate (e.g. `sys::fs2::File`). On Windows the pipe +can be created via Windows' `CreatePipe` (a stub for which is missing in +`liblibc` at the moment) and the resulting HANDLEs also appropriately wrapped +(using `sys::fs2::File`). + +This proposal considers making the `Pipe`'s reader and writer fields public +to allow easily moving ownership of the two handles, but any other appropriate +interface is acceptable. ```rust pub struct Pipe { - pub reader: File, - pub writer: File, + pub reader: PipeReader, + pub writer: PipeWriter, } + +pub struct PipeReader(sys::fs2::File); +pub struct PipeWriter(sys::fs2::File); + +impl Read for PipeReader { ... } +impl Write for PipeWriter { ... } +``` + +Next, several `redirect_*` methods should be added to `Stdio` for certain +"blessed" types offered by the standard library, such as `File`, `PipeRead`, and +`PipeWrite`. By white-listing the accepted file-like types we can ensure the API +and its behavior is consistent across Windows and Unix. + +```rust +fn redirect_file(f: File) -> Stdio { ... } +fn redirect_pipe_read(r: PipeRead) -> Stdio { ... } +fn redirect_pipe_write(w: PipeWrite) -> Stdio { ... } ``` -Next, `std::process::Stdio` should provide a `redirect` method which accepts and -stores a `&File`, ensuring the underlying handle will not go out of scope -unexpectedly before the child is spawned. The spawning implementation can then -extract and use the `File`'s OS specific handle when creating the child -process. - -This `File` reference should be an immutable one, to allow "reuse" of the handle -across several `Command`s simultaneously. This, however, can allow code to -indirectly mutate a `File` through an immutable reference by passing it on to a -child process, although retrieving and mutating through the underlying OS handle -(via `AsRaw{Fd, Socket, Handle}`) is already possible through a `&File`. Thus -this API would not introduce any "mutability leaks" of `File`s that were not -already present. - -This design also offers benefits when the user may wish to close (drop) a -`File`, for example, closing the read end of a pipe and sending EOF to its -child. The compiler can infer the lifetimes of all references to the original -`File`, eliminating some guesswork on whether all ends of a pipe have been -closed. This would not be possible in a design which duplicates the OS handle -internally which could more easily lead to a deadlock (e.g. waiting on a child -to exit while the same scope holds an open pipe to the child's stdin). - -Reclaiming ownership of the borrowed `File` may require locally scoping the -creation of `Stdio` or `Command` instances to force their lifetimes to end, -however, this is minimally intrusive compared to alternative designs. +These methods should take ownership of their arguments since storing references +will require `Stdio` and `Command` to gain lifetime parameters, which will break +the currently stabilized implementations. Thus the caller will be responsible +for duplicating their handles appropriately if they wish to retain ownership. + +To make redirections easier to use `Stdio` should become clonable so that once a +file-like handle is wrapped, the wrapper can be passed to any number of +`Commands` simultaneously. This can be accomplished by reference counting the +wrapped file handle. + +```rust +impl Clone for Stdio { ... } + +#[deriving(Clone)] +struct StdioImp { + ... + // sys::fs2::File is a safe wrapper over a fd/HANDLE, + // not to be confused with the public `std::fs::File` + Redirect(Rc), +} +``` # Drawbacks -Implementing a design based on `File` borrows will require adding lifetime -parameters on stabilized `Stdio` and `Command` close to the 1.0 release. +If one wishes to close (drop) a redirected handle, for example, closing the +read end of a pipe and sending EOF to its child, they will have to manually +ensure all cloned `Stdio` wrappers are dropped so that the underlying handle is +closed. Otherwise it would be possible to deadlock while waiting on a child +which is waiting on and input handle held in the same thread. + +In addition, if one desires to use a file-like handle outside of process +redirection, they will need to rely on an external mechanism for duplicating the +handle. In the case of an actual file, it can simply be reopened, but in the +case of OS pipes or sockets the underlying OS handle may need to be duplicated +via `libc` calls or the object itself would need to provide a duplication +mechanism. # Alternatives -Do nothing now and choose a stability compatible design, possibly being stuck -with less ergonomic APIs. +* Instead of specifying numerous `redirect_*` methods, simply accept anything + that implements the appropriate `AsRaw{Fd, Handle}` trait. This will cause OS + specific issues, however. For example, on Unix sockets are simple file + descriptors, but on Windows sockets aren't quite HANDLES, meaning the API + would be inconsistent across OS types. -One alternative strategy is to duplicate the underlying OS handle and have the -`std::process` APIs take ownership of the copy. When working with OS pipes, -however, the user would have to manually keep track where the duplicates have -gone if they wish to close them all; failing to do so may cause deadlocks. +* Duplicate the underlying OS handle and have the `std::process` APIs take + ownership of the copy. When working with OS pipes, however, the user would + have to manually keep track where the duplicates have gone if they wish to + close them all; failing to do so may cause deadlocks. -Another strategy would be for `Stdio` to take ownership of a `File` and wrap it -as a `Rc`, allowing it to be "reused" in any number of redirections by -cloning the `Stdio` wrapper. A caller could try to regain ownership (via -`try_unwrap` on the internal wrapper), but they would have to (manually) ensure -all other `Stdio` clones are dropped. This design would also suffer from -potential deadlocks, making it by far the least ergonomic option. +* Do not make `Stdio` clonable, and rely on caller duplicating all handles as + necessary. This could be particularly limiting if certain implementations do + not allow duplication. # Unresolved questions From 2d67ee4be6d4bc862fae40f83b8e59851333aa43 Mon Sep 17 00:00:00 2001 From: Ivan Petkov Date: Mon, 13 Apr 2015 15:48:12 -0700 Subject: [PATCH 03/11] Make sure that Stdio stores an Arc, not Rc * We don't want to break Stdio's already present Sync and Send bounds --- text/0000-process-stdio-redirection.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-process-stdio-redirection.md b/text/0000-process-stdio-redirection.md index 43d479e4c95..564915c53aa 100644 --- a/text/0000-process-stdio-redirection.md +++ b/text/0000-process-stdio-redirection.md @@ -85,7 +85,7 @@ struct StdioImp { ... // sys::fs2::File is a safe wrapper over a fd/HANDLE, // not to be confused with the public `std::fs::File` - Redirect(Rc), + Redirect(Arc), } ``` From 4e19aefde14c3ccdf8a3679f315e51b1fc0541e0 Mon Sep 17 00:00:00 2001 From: Ivan Petkov Date: Sat, 18 Apr 2015 11:55:54 -0700 Subject: [PATCH 04/11] Rework design and simplify design again * Use an OS specific `StdioExt` trait to allow for redirection based on `AsRawFd` and `AsRawHandle` implementors. * Leverage the OS pipes already present in `ChildStd{in, out, err}` after spawning a `Command` to easily allow piping data between processes. --- text/0000-process-stdio-redirection.md | 141 ++++++++++++------------- 1 file changed, 65 insertions(+), 76 deletions(-) diff --git a/text/0000-process-stdio-redirection.md b/text/0000-process-stdio-redirection.md index 564915c53aa..bac0b34fd77 100644 --- a/text/0000-process-stdio-redirection.md +++ b/text/0000-process-stdio-redirection.md @@ -5,7 +5,7 @@ # Summary -Update the `std::process` API with the ability to redirect stdio of child +Update the standard library API with the ability to redirect stdio of child processes to any opened file handle or equivalent. # Motivation @@ -25,101 +25,90 @@ redirection. # Detailed design -First, the standard library should provide an OS agnostic way of creating OS -in-memory pipes, i.e. a `Pipe`, providing reader and writer handles as -`PipeReader` and `PipeWriter` respectively. The reader and writer will simply be -an abstraction over the OS specific file descriptor/HANDLE, and will implement -the respective `Read`/`Write` trait, making it impossible to confuse the two -together. In addition, each should implement the appropriate `AsRaw{Fd, -Handle}`/`FromRaw{Fd, Handle}` for wrapping/unwrapping of the OS handles. - -On Unix systems the pipe can be created with `libc::pipe` and the file -descriptors wrapped as appropriate (e.g. `sys::fs2::File`). On Windows the pipe -can be created via Windows' `CreatePipe` (a stub for which is missing in -`liblibc` at the moment) and the resulting HANDLEs also appropriately wrapped -(using `sys::fs2::File`). - -This proposal considers making the `Pipe`'s reader and writer fields public -to allow easily moving ownership of the two handles, but any other appropriate -interface is acceptable. +Process redirection should be provided as a system dependent extension which +accepts the appropriate OS file/pipe representation. Namely, the API should be +provided as a `StdioExt` implementation, whose `redirect` method accepts +implementors of `AsRawFd` and `AsRawHandle` for Unix and Windows, respectively. + This trait should be publicly exported under the `std::os::$platform::io` +module. + +The private `StdioImp` enum in `std::process` should be extended with a +`Redirect` variant which will hold the appropriate OS handle type. To avoid +breaking changes with the stabilized interfaces (such as `Stdio` and `Command`) +or dealing with internal reference counts over the handle to be redirected, the +most convenient implementation would be to `unsafe`ly extract and store the raw +fd/HANDLE to which the redirection should occur. Thus it would be the caller's +responsibility to ensure the open file or pipe remains valid until a child +process is spawned. ```rust -pub struct Pipe { - pub reader: PipeReader, - pub writer: PipeWriter, +StdioImp { + ... + #[cfg(unix)] Redirect(sys::io::RawFd), + #[cfg(windows)] Redirect(sys::io::RawHandle), } -pub struct PipeReader(sys::fs2::File); -pub struct PipeWriter(sys::fs2::File); +// Unix, in libstd/sys/unix/ext.rs +pub impl StdioExt { + unsafe fn redirect(t: &T) -> Stdio { ... } +} -impl Read for PipeReader { ... } -impl Write for PipeWriter { ... } +// Windows, in libstd/sys/windows/ext.rs +pub impl StdioExt { + unsafe fn redirect(t: &T) -> Stdio { ... } +} ``` -Next, several `redirect_*` methods should be added to `Stdio` for certain -"blessed" types offered by the standard library, such as `File`, `PipeRead`, and -`PipeWrite`. By white-listing the accepted file-like types we can ensure the API -and its behavior is consistent across Windows and Unix. +One of the benefits of this design is that it still makes the caller aware they +are using system dependent extensions, while being able to utilize the API +without suffering the constant need of `cfg!` checks to specify the exact target +OS. For example, the code below would work on both Unix and Windows: ```rust -fn redirect_file(f: File) -> Stdio { ... } -fn redirect_pipe_read(r: PipeRead) -> Stdio { ... } -fn redirect_pipe_write(w: PipeWrite) -> Stdio { ... } -``` +#[cfg(unix)] use std::os::unix::io::StdioExt as StdioExt; +#[cfg(windows)] use std::os::windows::io::StdioExt as StdioExt; -These methods should take ownership of their arguments since storing references -will require `Stdio` and `Command` to gain lifetime parameters, which will break -the currently stabilized implementations. Thus the caller will be responsible -for duplicating their handles appropriately if they wish to retain ownership. +let file = File::open(...); +let stdio = unsafe { StdioExt::redirect(&file) }; +... +``` -To make redirections easier to use `Stdio` should become clonable so that once a -file-like handle is wrapped, the wrapper can be passed to any number of -`Commands` simultaneously. This can be accomplished by reference counting the -wrapped file handle. +Next, the `AsRaw{Fd, Handle}` traits should be implemented for the +`ChildStd{in,out,err}` types. This would allow easily piping output from one +child process to another by leveraging the underlying OS pipes that were already +created when spawning the child. ```rust -impl Clone for Stdio { ... } - -#[deriving(Clone)] -struct StdioImp { - ... - // sys::fs2::File is a safe wrapper over a fd/HANDLE, - // not to be confused with the public `std::fs::File` - Redirect(Arc), -} +// Equivalent of `foo | bar` +let foo = Command::new("foo").spawn().unwrap(); +let out = foo.stdout.as_ref().unwrap(); +let bar = Command::new("bar").stdin(StdioExt::redirect(out)).spawn().unwrap(); +// close foo.stdout here so that bar is the only pipe reader + +// Alternatively +let bar = Command::new("bar").spawn().unwrap(); +let in = bar.stdin.as_ref().unwrap(); +let foo = Command::new("foo").stdout(StdioExt::redirect(in)).spawn().unwrap(); +// close bar.stdin here so that foo is the only pipe writer ``` -# Drawbacks +This would require that the internally defined `AnonPipe` wrapper be implemented +using HANDLEs (and not file descriptors) on Windows. This can easily be +accomplished by wrapping the resulting HANDLEs from Windows' `CreatePipe` API +(a stub for which is missing in `libc` at the moment). The Unix implementation +can continue to use `libc::pipe`, of course. With these changes in place, +`AnonPipe` can implement `AsRaw{Fd, Handle}`, and allow `ChildStd{in, out, err}` +to implement the traits as well. -If one wishes to close (drop) a redirected handle, for example, closing the -read end of a pipe and sending EOF to its child, they will have to manually -ensure all cloned `Stdio` wrappers are dropped so that the underlying handle is -closed. Otherwise it would be possible to deadlock while waiting on a child -which is waiting on and input handle held in the same thread. +# Drawbacks -In addition, if one desires to use a file-like handle outside of process -redirection, they will need to rely on an external mechanism for duplicating the -handle. In the case of an actual file, it can simply be reopened, but in the -case of OS pipes or sockets the underlying OS handle may need to be duplicated -via `libc` calls or the object itself would need to provide a duplication -mechanism. +Unsafely using raw OS file handles could potentially cause issues, however, +`Command`s are are usually spawned immediately after building during which time +open file descriptors/HANDLEs are still valid. # Alternatives -* Instead of specifying numerous `redirect_*` methods, simply accept anything - that implements the appropriate `AsRaw{Fd, Handle}` trait. This will cause OS - specific issues, however. For example, on Unix sockets are simple file - descriptors, but on Windows sockets aren't quite HANDLES, meaning the API - would be inconsistent across OS types. - -* Duplicate the underlying OS handle and have the `std::process` APIs take - ownership of the copy. When working with OS pipes, however, the user would - have to manually keep track where the duplicates have gone if they wish to - close them all; failing to do so may cause deadlocks. - -* Do not make `Stdio` clonable, and rely on caller duplicating all handles as - necessary. This could be particularly limiting if certain implementations do - not allow duplication. +None that don't involve breaking changes or verbose interfaces. # Unresolved questions From 2f0e0020887e006881d432494dc2a0e485b3dd71 Mon Sep 17 00:00:00 2001 From: Ivan Petkov Date: Sat, 18 Apr 2015 11:55:54 -0700 Subject: [PATCH 05/11] Update location of `StdioExt` * `StdioExt` should be defined in `std::os::$platform::process` not `std::os::$platform::io` --- text/0000-process-stdio-redirection.md | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/text/0000-process-stdio-redirection.md b/text/0000-process-stdio-redirection.md index bac0b34fd77..f32e00438d6 100644 --- a/text/0000-process-stdio-redirection.md +++ b/text/0000-process-stdio-redirection.md @@ -29,8 +29,8 @@ Process redirection should be provided as a system dependent extension which accepts the appropriate OS file/pipe representation. Namely, the API should be provided as a `StdioExt` implementation, whose `redirect` method accepts implementors of `AsRawFd` and `AsRawHandle` for Unix and Windows, respectively. - This trait should be publicly exported under the `std::os::$platform::io` -module. +This implementation should be publicly exported under the +`std::os::$platform::process` module. The private `StdioImp` enum in `std::process` should be extended with a `Redirect` variant which will hold the appropriate OS handle type. To avoid @@ -49,12 +49,14 @@ StdioImp { } // Unix, in libstd/sys/unix/ext.rs -pub impl StdioExt { +pub struct StdioExt; +impl StdioExt { unsafe fn redirect(t: &T) -> Stdio { ... } } // Windows, in libstd/sys/windows/ext.rs -pub impl StdioExt { +pub struct StdioExt; +impl StdioExt { unsafe fn redirect(t: &T) -> Stdio { ... } } ``` @@ -65,8 +67,8 @@ without suffering the constant need of `cfg!` checks to specify the exact target OS. For example, the code below would work on both Unix and Windows: ```rust -#[cfg(unix)] use std::os::unix::io::StdioExt as StdioExt; -#[cfg(windows)] use std::os::windows::io::StdioExt as StdioExt; +#[cfg(unix)] use std::os::unix::process::StdioExt; +#[cfg(windows)] use std::os::windows::process::StdioExt; let file = File::open(...); let stdio = unsafe { StdioExt::redirect(&file) }; From 8b8a46839cd1db338f0d40376790076dd576ed9d Mon Sep 17 00:00:00 2001 From: Ivan Petkov Date: Sun, 19 Apr 2015 11:13:02 -0700 Subject: [PATCH 06/11] Correct example usage * Default IO of `Command`s is to inherit the stdio handles, so we should specify `Stdio::piped` where applicable. --- text/0000-process-stdio-redirection.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/text/0000-process-stdio-redirection.md b/text/0000-process-stdio-redirection.md index f32e00438d6..85bfc62542f 100644 --- a/text/0000-process-stdio-redirection.md +++ b/text/0000-process-stdio-redirection.md @@ -82,13 +82,13 @@ created when spawning the child. ```rust // Equivalent of `foo | bar` -let foo = Command::new("foo").spawn().unwrap(); +let foo = Command::new("foo").stdout(Stdio::piped()).spawn().unwrap(); let out = foo.stdout.as_ref().unwrap(); let bar = Command::new("bar").stdin(StdioExt::redirect(out)).spawn().unwrap(); // close foo.stdout here so that bar is the only pipe reader // Alternatively -let bar = Command::new("bar").spawn().unwrap(); +let bar = Command::new("bar").stdin(Stdio::piped()).spawn().unwrap(); let in = bar.stdin.as_ref().unwrap(); let foo = Command::new("foo").stdout(StdioExt::redirect(in)).spawn().unwrap(); // close bar.stdin here so that foo is the only pipe writer From 4ac365e5f769cd3f1db90c2cc4de905ac3636f7e Mon Sep 17 00:00:00 2001 From: Ivan Petkov Date: Mon, 20 Apr 2015 15:39:43 -0700 Subject: [PATCH 07/11] AnonPipes no longer use file descriptors on Windows --- text/0000-process-stdio-redirection.md | 8 -------- 1 file changed, 8 deletions(-) diff --git a/text/0000-process-stdio-redirection.md b/text/0000-process-stdio-redirection.md index 85bfc62542f..f7a0e760d86 100644 --- a/text/0000-process-stdio-redirection.md +++ b/text/0000-process-stdio-redirection.md @@ -94,14 +94,6 @@ let foo = Command::new("foo").stdout(StdioExt::redirect(in)).spawn().unwrap(); // close bar.stdin here so that foo is the only pipe writer ``` -This would require that the internally defined `AnonPipe` wrapper be implemented -using HANDLEs (and not file descriptors) on Windows. This can easily be -accomplished by wrapping the resulting HANDLEs from Windows' `CreatePipe` API -(a stub for which is missing in `libc` at the moment). The Unix implementation -can continue to use `libc::pipe`, of course. With these changes in place, -`AnonPipe` can implement `AsRaw{Fd, Handle}`, and allow `ChildStd{in, out, err}` -to implement the traits as well. - # Drawbacks Unsafely using raw OS file handles could potentially cause issues, however, From 70e404366471dcf6203de08274e2db806a7e5f0e Mon Sep 17 00:00:00 2001 From: Ivan Petkov Date: Fri, 15 May 2015 16:25:13 -0700 Subject: [PATCH 08/11] Update proposal * Take `FromRaw{Fd, Handle}` into account as a possible implementation * Open discussion on a higher level API for child process stdio redirection --- text/0000-process-stdio-redirection.md | 139 ++++++++++++++----------- 1 file changed, 77 insertions(+), 62 deletions(-) diff --git a/text/0000-process-stdio-redirection.md b/text/0000-process-stdio-redirection.md index f7a0e760d86..41f3fc840e2 100644 --- a/text/0000-process-stdio-redirection.md +++ b/text/0000-process-stdio-redirection.md @@ -25,84 +25,99 @@ redirection. # Detailed design -Process redirection should be provided as a system dependent extension which -accepts the appropriate OS file/pipe representation. Namely, the API should be -provided as a `StdioExt` implementation, whose `redirect` method accepts -implementors of `AsRawFd` and `AsRawHandle` for Unix and Windows, respectively. -This implementation should be publicly exported under the -`std::os::$platform::process` module. - -The private `StdioImp` enum in `std::process` should be extended with a -`Redirect` variant which will hold the appropriate OS handle type. To avoid -breaking changes with the stabilized interfaces (such as `Stdio` and `Command`) -or dealing with internal reference counts over the handle to be redirected, the -most convenient implementation would be to `unsafe`ly extract and store the raw -fd/HANDLE to which the redirection should occur. Thus it would be the caller's -responsibility to ensure the open file or pipe remains valid until a child -process is spawned. - -```rust -StdioImp { - ... - #[cfg(unix)] Redirect(sys::io::RawFd), - #[cfg(windows)] Redirect(sys::io::RawHandle), -} - -// Unix, in libstd/sys/unix/ext.rs -pub struct StdioExt; -impl StdioExt { - unsafe fn redirect(t: &T) -> Stdio { ... } -} - -// Windows, in libstd/sys/windows/ext.rs -pub struct StdioExt; -impl StdioExt { - unsafe fn redirect(t: &T) -> Stdio { ... } -} -``` +The least intrusive way to implement this functionality would be to implement +`FromRaw{Fd, Handle}` for `Stdio`, taking ownership of the underlying handle. +This approach would not require the addition of any new public APIs. + +There are several disadvantages to this design: it does not allow for using +one handle for several redirections without forcing the caller to `unsafe`ly +duplicate the handle, in addition, any cross-platform code will need to be +littered with `cfg!` to call the appropriate methods. Furthermore, trying to use +standard types like `File` will require that they be leaked by `mem::forget` so +that their own destructors do not close the underlying handles. However, these +inefficiencies can be addressed through a cross-platform, high level API. + +Lastly, the `AsRaw{Fd, Handle}` traits should be implemented for the +`ChildStd{in,out,err}` types. This would allow easily piping output from one +child process to another by leveraging the underlying OS pipes that were already +created when spawning the child. -One of the benefits of this design is that it still makes the caller aware they -are using system dependent extensions, while being able to utilize the API -without suffering the constant need of `cfg!` checks to specify the exact target -OS. For example, the code below would work on both Unix and Windows: +## A High Level API Proposal + +In order to facilitate cross-platform usage, the API should be defined to +operate on the respective `AsRaw{Fd, Handle}` traits for Unix and Windows, +respectively. All method signatures should match lexically such that +using `cfg!($platform)` checks will not be necessary. The private `StdioImp` +enum in `std::process` should be extended with a `Redirect` variant which will +hold a wrapper over the appropriate OS handle type to ensure it is properly +closed (e.g. using an `AnonPipe`). + +Next, the API should expose methods for redirection that both do and do not take +ownership of the underlying handle, e.g. `redirect(t: T)` and +`redirect_by_ref(t: &T)`. + +The method that takes ownership retains the +benefits of using `FromRaw*` directly while helping the caller avoid making +platform specific calls. Unfortunately, since we cannot guarantee that an +implementor of `AsRaw*` is the sole owner of the OS handle they return, this +method will have to be `unsafe`. + +The method which does not take ownership allows the +caller to reuse their handle without making excessive duplications, which would +not be possible by using `FromRaw*` directly. The caller is, however, forced to +ensure the handle will remain valid until the child is spawned, making this +method `unsafe` as well. + +Below are several alternative ways of exposing a high level API. They are +ordered in the author's personal preference, but neither is strictly better than +the others designs. + +1. Exposing `redirect` methods via separate `StdioExt` struct: It will live in +`std::os::$platform::process`, thus making it apparent to the caller that they +are using an OS specific extension when importing it. This design offers large +flexibility in external libraries need only define `AsRaw*` or have access to +the raw OS handle itself (which would trivially define `AsRaw*` for itself). + +2. Exposing `redirect` methods via trait, e.g. `ToStdio`: This design will give +greatest control to us (std) as to what can be used for redirection, however, it +gives less flexibility to external libraries as they may need to implement +additional traits. Moreover, and any blanket impls over `AsRaw*` invalidate the +tight control (if it is desired) of redirectables. An unresolved question is +what to name this trait as there are no such clear patterns established in the +standard libraries or on `crates.io`. For example, the trait could be `ToStdio`, +`To`, `Into`, etc. + +3. Expose `redirect` methods directly on `Stdio`: Cutting out the middleman +(middletrait?) and defining the methods directly on the source minimizes APIs +that will be eventually stabilized. This design, however, blurs the distinction +that OS specifics apply (e.g. a file and socket are both file descriptors on +Unix, but not necessarily HANDLEs on Windows). + +Example API usage based on the `StdioExt` design described above: ```rust #[cfg(unix)] use std::os::unix::process::StdioExt; #[cfg(windows)] use std::os::windows::process::StdioExt; -let file = File::open(...); -let stdio = unsafe { StdioExt::redirect(&file) }; -... -``` - -Next, the `AsRaw{Fd, Handle}` traits should be implemented for the -`ChildStd{in,out,err}` types. This would allow easily piping output from one -child process to another by leveraging the underlying OS pipes that were already -created when spawning the child. - -```rust // Equivalent of `foo | bar` let foo = Command::new("foo").stdout(Stdio::piped()).spawn().unwrap(); -let out = foo.stdout.as_ref().unwrap(); +let out = foo.stdout.take().unwrap(); let bar = Command::new("bar").stdin(StdioExt::redirect(out)).spawn().unwrap(); -// close foo.stdout here so that bar is the only pipe reader - -// Alternatively -let bar = Command::new("bar").stdin(Stdio::piped()).spawn().unwrap(); -let in = bar.stdin.as_ref().unwrap(); -let foo = Command::new("foo").stdout(StdioExt::redirect(in)).spawn().unwrap(); -// close bar.stdin here so that foo is the only pipe writer ``` # Drawbacks -Unsafely using raw OS file handles could potentially cause issues, however, -`Command`s are are usually spawned immediately after building during which time -open file descriptors/HANDLEs are still valid. +Without using a high level API callers will be forced to use verbose and +`unsafe` code more than they should or could get away with. Even with using a +high level API there will be `unsafe`ty present due to stability lock on +`Command` and `Stdio` (i.e. we cannot simply store references to the handles +ensuring they remain valid). However, `Command`s are are usually spawned +immediately after building during which time open file descriptors/HANDLEs are +still valid. # Alternatives -None that don't involve breaking changes or verbose interfaces. +High level API alternatives discussed above. # Unresolved questions From 113bd5ab599ae982fe9bdba04d61d0e8ed7362f9 Mon Sep 17 00:00:00 2001 From: Ivan Petkov Date: Sun, 14 Jun 2015 08:59:14 -0700 Subject: [PATCH 09/11] Add method signatures for each high level API alternative --- text/0000-process-stdio-redirection.md | 43 ++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 3 deletions(-) diff --git a/text/0000-process-stdio-redirection.md b/text/0000-process-stdio-redirection.md index 41f3fc840e2..e0892209a99 100644 --- a/text/0000-process-stdio-redirection.md +++ b/text/0000-process-stdio-redirection.md @@ -72,13 +72,25 @@ Below are several alternative ways of exposing a high level API. They are ordered in the author's personal preference, but neither is strictly better than the others designs. -1. Exposing `redirect` methods via separate `StdioExt` struct: It will live in +1. +Exposing `redirect` methods via separate `StdioExt` struct: It will live in `std::os::$platform::process`, thus making it apparent to the caller that they are using an OS specific extension when importing it. This design offers large flexibility in external libraries need only define `AsRaw*` or have access to the raw OS handle itself (which would trivially define `AsRaw*` for itself). -2. Exposing `redirect` methods via trait, e.g. `ToStdio`: This design will give +```rust +pub struct StdioExt; +impl StdioExt { + // Take ownership of the handle + pub fn redirect(t: T) -> Stdio; + // Unsafely borrow the handle, letting caller ensure it is valid + pub unsafe fn redirect_by_ref(t: T) -> Stdio; +} +``` + +2. +Exposing `redirect` methods via trait, e.g. `ToStdio`: This design will give greatest control to us (std) as to what can be used for redirection, however, it gives less flexibility to external libraries as they may need to implement additional traits. Moreover, and any blanket impls over `AsRaw*` invalidate the @@ -87,12 +99,37 @@ what to name this trait as there are no such clear patterns established in the standard libraries or on `crates.io`. For example, the trait could be `ToStdio`, `To`, `Into`, etc. -3. Expose `redirect` methods directly on `Stdio`: Cutting out the middleman +```rust +pub trait ToStdio { + unsafe fn to_stdio(t: T) -> Stdio; +} + +impl ToStdio for T where T: AsRaw* { + // Unsafely borrow the handle, letting caller ensure it is valid + unsafe fn to_stdio(t: T) -> Stdio; +} +``` + +3. +Expose `redirect` methods directly on `Stdio`: Cutting out the middleman (middletrait?) and defining the methods directly on the source minimizes APIs that will be eventually stabilized. This design, however, blurs the distinction that OS specifics apply (e.g. a file and socket are both file descriptors on Unix, but not necessarily HANDLEs on Windows). +```rust +impl Stdio { + pub fn piped() -> Stdio; + pub fn inherit() -> Stdio; + pub fn null() -> Stdio; + + // Take ownership of the handle + pub fn redirect(t: T) -> Stdio; + // Unsafely borrow the handle, letting caller ensure it is valid + pub unsafe fn redirect_by_ref(t: T) -> Stdio; +} +``` + Example API usage based on the `StdioExt` design described above: ```rust From 690d99b1fae8206b80424cf8e0bd17f2b7a68067 Mon Sep 17 00:00:00 2001 From: Ivan Petkov Date: Mon, 15 Jun 2015 13:24:41 -0700 Subject: [PATCH 10/11] Update the motivation behind the RFC * Low level `FromRaw*` `AsRaw*` APIs are already present in libstd, thus the focus of this RFC has shifted to just developing a high level API. --- text/0000-process-stdio-redirection.md | 31 ++++++++++++++------------ 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/text/0000-process-stdio-redirection.md b/text/0000-process-stdio-redirection.md index e0892209a99..9878f54b10d 100644 --- a/text/0000-process-stdio-redirection.md +++ b/text/0000-process-stdio-redirection.md @@ -5,23 +5,26 @@ # Summary -Update the standard library API with the ability to redirect stdio of child -processes to any opened file handle or equivalent. +Update the standard library with a cross-platform high level API for redirecting +stdio of child processes to any opened file handle or equivalent. # Motivation -The current API in `std::process` allows to either pipe stdio between parent and -child process or redirect stdio to `/dev/null`. It would also be largely useful -to allow stdio redirection to any currently opened file or pipe handles. This -would allow redirecting stdio with a physical file or even another process (via -OS pipe) without forcing the parent process to buffer the data itself. - -For example, one may wish to spawn a process which prints gigabytes -of data (e.g. logs) and use another process to filter through it, and save the -result to a file. The current API would force the parent process to dedicate a -thread just to buffer all data between child processes and disk, which is -impractical given that the OS can stream the data for us via pipes and file -redirection. +The current API in `std::process` allows the usage of raw file descriptors or +HANDLEs for child process stdio redirection by leveraging the `FromRaw{Fd, +Handle}` and `AsRaw{Fd, Handle}` trait implementations on `Stdio` and +`ChildStd{In, Out, Err}`, respectively. + +Unfortunately, since the actual methods pertaining to `FromRaw*` and `AsRaw*` +are OS specific, their usage requires either constant `cfg` checks or OS +specific code duplication. Moreover, the conversion to `Stdio` is `unsafe` and +requires the caller to ensure the OS handle remains valid until spawning the +child. In the event that a caller wishes to give a child exclusive ownership of +an OS handle, they must still go through the headache of manually keeping the +handle alive and valid. + +Developing a high level cross-platform API will make stdio redirection more +ergonomic and reduce code noise. # Detailed design From a8e1c8bf9d5079b796b852f46df4f78fd6a866ab Mon Sep 17 00:00:00 2001 From: Ivan Petkov Date: Mon, 15 Jun 2015 14:34:06 -0700 Subject: [PATCH 11/11] Pick a favorite design approach --- text/0000-process-stdio-redirection.md | 144 ++++++++----------------- 1 file changed, 43 insertions(+), 101 deletions(-) diff --git a/text/0000-process-stdio-redirection.md b/text/0000-process-stdio-redirection.md index 9878f54b10d..806ce71c894 100644 --- a/text/0000-process-stdio-redirection.md +++ b/text/0000-process-stdio-redirection.md @@ -28,112 +28,34 @@ ergonomic and reduce code noise. # Detailed design -The least intrusive way to implement this functionality would be to implement -`FromRaw{Fd, Handle}` for `Stdio`, taking ownership of the underlying handle. -This approach would not require the addition of any new public APIs. - -There are several disadvantages to this design: it does not allow for using -one handle for several redirections without forcing the caller to `unsafe`ly -duplicate the handle, in addition, any cross-platform code will need to be -littered with `cfg!` to call the appropriate methods. Furthermore, trying to use -standard types like `File` will require that they be leaked by `mem::forget` so -that their own destructors do not close the underlying handles. However, these -inefficiencies can be addressed through a cross-platform, high level API. - -Lastly, the `AsRaw{Fd, Handle}` traits should be implemented for the -`ChildStd{in,out,err}` types. This would allow easily piping output from one -child process to another by leveraging the underlying OS pipes that were already -created when spawning the child. - -## A High Level API Proposal - -In order to facilitate cross-platform usage, the API should be defined to -operate on the respective `AsRaw{Fd, Handle}` traits for Unix and Windows, -respectively. All method signatures should match lexically such that -using `cfg!($platform)` checks will not be necessary. The private `StdioImp` -enum in `std::process` should be extended with a `Redirect` variant which will -hold a wrapper over the appropriate OS handle type to ensure it is properly -closed (e.g. using an `AnonPipe`). - -Next, the API should expose methods for redirection that both do and do not take -ownership of the underlying handle, e.g. `redirect(t: T)` and -`redirect_by_ref(t: &T)`. - -The method that takes ownership retains the -benefits of using `FromRaw*` directly while helping the caller avoid making -platform specific calls. Unfortunately, since we cannot guarantee that an -implementor of `AsRaw*` is the sole owner of the OS handle they return, this -method will have to be `unsafe`. - -The method which does not take ownership allows the -caller to reuse their handle without making excessive duplications, which would -not be possible by using `FromRaw*` directly. The caller is, however, forced to -ensure the handle will remain valid until the child is spawned, making this -method `unsafe` as well. - -Below are several alternative ways of exposing a high level API. They are -ordered in the author's personal preference, but neither is strictly better than -the others designs. - -1. -Exposing `redirect` methods via separate `StdioExt` struct: It will live in -`std::os::$platform::process`, thus making it apparent to the caller that they -are using an OS specific extension when importing it. This design offers large -flexibility in external libraries need only define `AsRaw*` or have access to -the raw OS handle itself (which would trivially define `AsRaw*` for itself). +The de facto method for adding system specific extensions to the standard +library has been to define an extension trait--following this approach a +`StdioExt` trait should be defined under `std::os::$platform::process` to +provide the redirection functionality. Unlike other system specific extensions, +however, the methods of this trait should match lexically, differing only in the +`AsRaw*` type they accept, such that rebuilding the same source on a different +platform will only require the import of the OS specific trait rather than +changing method invodations as well. + +This trait should define two methods which accept the appropriate `AsRaw*` +implementor and return an `Stdio`: +* One which (safely) takes ownership of the raw handle or its wrapper. The + wrapper should be boxed and stored by the `Stdio` wrapper so its destructor + can run when it goes out of scope. +* Another method which simply extracts the raw handle without taking ownership: + this method will essentially be a cross-platform abstraction over using + `FromRaw*` on `Stdio`, thus making this method `unsafe` as well. ```rust -pub struct StdioExt; -impl StdioExt { +pub trait StdioExt { // Take ownership of the handle - pub fn redirect(t: T) -> Stdio; + fn redirect(t: T) -> Stdio; // Unsafely borrow the handle, letting caller ensure it is valid - pub unsafe fn redirect_by_ref(t: T) -> Stdio; + unsafe fn redirect_by_ref(t: &T) -> Stdio; } ``` -2. -Exposing `redirect` methods via trait, e.g. `ToStdio`: This design will give -greatest control to us (std) as to what can be used for redirection, however, it -gives less flexibility to external libraries as they may need to implement -additional traits. Moreover, and any blanket impls over `AsRaw*` invalidate the -tight control (if it is desired) of redirectables. An unresolved question is -what to name this trait as there are no such clear patterns established in the -standard libraries or on `crates.io`. For example, the trait could be `ToStdio`, -`To`, `Into`, etc. - -```rust -pub trait ToStdio { - unsafe fn to_stdio(t: T) -> Stdio; -} - -impl ToStdio for T where T: AsRaw* { - // Unsafely borrow the handle, letting caller ensure it is valid - unsafe fn to_stdio(t: T) -> Stdio; -} -``` - -3. -Expose `redirect` methods directly on `Stdio`: Cutting out the middleman -(middletrait?) and defining the methods directly on the source minimizes APIs -that will be eventually stabilized. This design, however, blurs the distinction -that OS specifics apply (e.g. a file and socket are both file descriptors on -Unix, but not necessarily HANDLEs on Windows). - -```rust -impl Stdio { - pub fn piped() -> Stdio; - pub fn inherit() -> Stdio; - pub fn null() -> Stdio; - - // Take ownership of the handle - pub fn redirect(t: T) -> Stdio; - // Unsafely borrow the handle, letting caller ensure it is valid - pub unsafe fn redirect_by_ref(t: T) -> Stdio; -} -``` - -Example API usage based on the `StdioExt` design described above: +Example API usage with minimal `cfg` checks and safe methods: ```rust #[cfg(unix)] use std::os::unix::process::StdioExt; @@ -142,7 +64,7 @@ Example API usage based on the `StdioExt` design described above: // Equivalent of `foo | bar` let foo = Command::new("foo").stdout(Stdio::piped()).spawn().unwrap(); let out = foo.stdout.take().unwrap(); -let bar = Command::new("bar").stdin(StdioExt::redirect(out)).spawn().unwrap(); +let bar = Command::new("bar").stdin(Stdio::redirect(out)).spawn().unwrap(); ``` # Drawbacks @@ -157,7 +79,27 @@ still valid. # Alternatives -High level API alternatives discussed above. +An alternative approach would be to expose `redirect` methods directly on +`Stdio`. This design, however, blurs the distinction of platform specific +details (e.g. a file and socket are both file descriptors on Unix, but not +necessarily HANDLEs on Windows) and may cause some confusion and give rise to +platform specific bugs. + +```rust +impl Stdio { + pub fn piped() -> Stdio; + pub fn inherit() -> Stdio; + pub fn null() -> Stdio; + + // Take ownership of the handle + #[cfg(unix)] pub fn redirect(t: T) -> Stdio; + #[cfg(windows)] pub fn redirect(t: T) -> Stdio; + + // Unsafely borrow the handle, letting caller ensure it is valid + #[cfg(unix)] pub unsafe fn redirect_by_ref(t: T) -> Stdio ; + #[cfg(windows)] pub unsafe fn redirect_by_ref(t: T) -> Stdio; +} +``` # Unresolved questions