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

`Start-Process` stream redirection is not faithful (it eats blank lines or adds blank lines) #8702

Open
GeeLaw opened this Issue Jan 21, 2019 · 2 comments

Comments

Projects
None yet
2 participants
@GeeLaw
Copy link

GeeLaw commented Jan 21, 2019

Steps to reproduce

Create a file a.cpp with the following content:

#include<cstdio>
int main()
{
    char buffer[1024];
    size_t rc;
    while ((rc = fread(buffer, 1, 1024, stdin)) != 0)
    {
        fwrite(buffer, 1, rc, stdout);
    }
    // There is a blank line on purpose!

    return 0;
}
// The file does not end with "\n" on purpose!

Then, in bash, compile it with g++ (or any C++ compiler you find on macOS) and test redirection in bash and PowerShell:

g++ a.cpp
./a.out < a.cpp > bash_redir.cpp
exec pwsh

Then in PowerShell:

Start-Process a.out -RedirectStandardInput a.cpp -RedirectStandardOutput pwsh_redir.cpp -Wait
Get-FileHash *.cpp

Expected behavior

All the three .cpp files have the same hash (are the same).

Actual behavior

The file pwsh_redir.cpp is different from the other two. The on-purpose blank line is eaten, and the on-purpose no-"\n" is broken. Specifically, pwsh_redir.cpp is the following:

#include<cstdio>
int main()
{
    char buffer[1024];
    size_t rc;
    while ((rc = fread(buffer, 1, 1024, stdin)) != 0)
    {
        fwrite(buffer, 1, rc, stdout);
    }
    // There is a blank line on purpose!
    return 0;
}
// The file does not end with "\n" on purpose!

Note that an extra "\n" has been appended at the end.

Environment data

Name                           Value
----                           -----
PSVersion                      6.1.2
PSEdition                      Core
GitCommitId                    6.1.2
OS                             Darwin 18.2.0 Darwin Kernel Version 18.2.0: Mo...
Platform                       Unix
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

The full value of OS property is Darwin 18.2.0 Darwin Kernel Version 18.2.0: Mon Nov 12 20:24:46 PST 2018; root:xnu-4903.231.4~2/RELEASE_X86_64. The full table of PSCompatibleVersions is

Major  Minor  Build  Revision
-----  -----  -----  --------
1      0      -1     -1
2      0      -1     -1
3      0      -1     -1
4      0      -1     -1
5      0      -1     -1
5      1      10032  0
6      1      2      -1

Comments

The standard stream might not be textual and they are binary in many cases. E.g., the curl might write the response stream to stdout.

PowerShell for macOS does the redirection by using ForkAndExecProcess, which creates pipes connecting the parent and the child. The parent, which is PowerShell, will then copy the stream from the pipe to file in a faulty way -- it assumes textual stream (as it's using StreamReader and StreamWriter).

There are TWO faults:

  1. Assuming the streams are textual.
  2. Copying text streams with ReadLine and WriteLine carelessly, which might create extraneous or missing line breaks.

The fix is to access the underlying pipe/file stream when doing the copy, and do a binary copy (resembling the C++ example code above).

This problem does not reproduce on Windows PowerShell, because Windows PowerShell implements Start-Process stream redirection by opening the actual files, inheriting the handles and setting the correct handles in STARTUPINFO.

This problem is not related to the long-discussed broken native-piping in PowerShell.

@GeeLaw

This comment has been minimized.

Copy link
Author

GeeLaw commented Jan 22, 2019

This shouldn't be labeled Area-Engine. It's a cmdlet issue. The problem lies in how Start-Process copies the streams. I would recommend Area-Cmdlets or one of its sublabel.

@GeeLaw

This comment has been minimized.

Copy link
Author

GeeLaw commented Jan 22, 2019

The solution mentioned in the body of this issue is the one involving minimal change to the current code. The better, complete solution is to start the subprocess with P/Invoke. Just like on Windows, Start-Process uses CreateProcess, on *nix, it should call (something similar to) ForkAndExecProcess.

The job is to add functionality of standard stream redirection to file to ForkAndExecProcess shim function so that Start-Process can avoid doing the copy. In addition to being error-prone as already proven and inefficient, the current code for stream copying involves some Thread.Sleep hacks, which are also bad. Getting rid of stream copying can be done by creating a new ForkAndFreopenAndExecProcess shim function, which, unsurprisingly, forks the pal process, calls freopen in the subprocess to redirect standard stream to the files, and calls execve to replace the forked process with the desired one.

The second solution (here) requires a bit of overhaul of the code. Specifically, .NET Core had better include this shim function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment