MedallionShell icon indicating copy to clipboard operation
MedallionShell copied to clipboard

Allow RedirectFrom to exit when the command exits even if the source stream is not exhausted

Open madelson opened this issue 6 years ago • 9 comments

Ran into this when having one console app invoke another and "forward" its stdin. If nothing is coming into stdin then the IO task will never finish.

madelson avatar Mar 26 '19 14:03 madelson

Also saw this come up with GetErrorAndOutputLines. I think one way this can happen is if the sub-process kicks off another process that inherits the std io streams and that process continues after the current one exits.

madelson avatar Apr 11 '19 00:04 madelson

Hi! I'm having exactly this issue, and it's killing me. I had written my own forwarding before finding out about this library, and I hoped migrating to this would help me get rid of it's problem, but it doesn't. I have a reproduction project at https://gist.github.com/Brice-xCIT/cd0449db814dfc7a17c17ede9582a38b (to reproduce download the files

Steps to reproduce:

  • dotnet run -v m --project .\ShellTest.csproj
  • Wait for the prompt (>)
  • Type anything then enter
  • Wait for a second prompt (>). Notice as you wait how "INPUT TO CONTINUE" appears. This should be executed after the command exits.
  • Type anything then enter.
  • It gets weird here. Press enter again, and now the program resumes execution.

Essentially, redirecting Console.In into the Command puts me into a catch-22:

  • using Command.Wait() hangs indefinitely
  • not using Command.Wait() continues the program flow before the command terminates.

The feature you propose in this issue would be ideal for me. I tried to do it in my previous codebase but never found a way... Is there any change to make it happen @madelson ? :)

Brice-xCIT avatar Sep 04 '19 17:09 Brice-xCIT

Hi @Brice-xCIT . Glad you agree that this would be a good build. The problem I've run into with trying to fix this in a general way thus far is that Console stream reads/writes are not cancel-able. Because of this, while we can exit the command we can't do anything to clean up the task/thread that is doing the reading.

HOWEVER, I think there is a workaround you can use for your specific use-case of piping from Console.In. I'd do something like this:

// no piping yet
var command = Command.Run(...); 
var pipeTask = command.StandardInput.PipeFromAsync(Console.In);
command.Wait(); // don't wait for pipeTask, since it won't finish

Does that work?

madelson avatar Sep 06 '19 02:09 madelson

Hi @madelson, thanks for your answer! I ran into the same problem as you describe when implementing this on my own. Fortunately, it seems like your workaround works if there's no Console.ReadLine() performed after command.Wait returns. (That's a great breakthrough, thank you! I'll be using that!) However, if there is one Console.ReadLine() right after the command returns, one needs to press enter twice for the Console.ReadLine() call to return. Also I'm wondering about that task that is spawned at command.StandardInput.PipeFromAsync(Console.In). Does it ever end?

Brice-xCIT avatar Sep 10 '19 14:09 Brice-xCIT

@Brice-xCIT sorry for missing your last question here. I believe that it does end in at least some cases, because windows will throw a broken pipe exception when we try to write to the underlying process.

We could probably handle this better, though.

As mentioned previously, the real issue is that we have no way to cancel console.in reads, so if the task is at that portion and no more input is coming from console.in and console.in does not close, then the task cannot complete.

madelson avatar Dec 17 '19 12:12 madelson

Ideas for how we could get this to work:

  • One windows, use native methods GetCurrentThread and CancelSynchronousIo for sync IO (the only thing our pipes support currently).
  • One linux, use select/poll as suggested here: https://stackoverflow.com/questions/16637086/linux-cancel-blocking-read

madelson avatar Dec 28 '19 13:12 madelson

Getting back to this a bit late, but I ended up not managing to make this work. Somehow your Sep 6th suggestion still blocked the thread on Linux. I ended up ditching inputs entirely which, in my use case, was not such a big trouble. I'm still puzzled as to why I never got it to work though.

Brice-xCIT avatar Jan 06 '20 10:01 Brice-xCIT

Thanks for the update @Brice-xCIT . Looking back at your original posted gist, wouldn't it always hang on line 101 since the nested command will be waiting on its input and the code never writes anything to that input stream?

madelson avatar Jan 07 '20 01:01 madelson

Working sample of read cancellation (windows):

void Main()
{
	var path = @"C:\Users\mikea_000\Documents\Interests\CS\MedallionShell\SampleCommand\bin\Debug\net46\SampleCommand.exe";
	var process = new Process
	{
		StartInfo = { FileName = path, RedirectStandardInput = true, RedirectStandardOutput = true, Arguments = "echo --per-char", UseShellExecute = false }
	};
	process.Start();
	try {
		Task.Run(() => {
			var res = DuplicateHandle(
				hSourceProcessHandle: GetCurrentProcess(),
				hSourceHandle: GetCurrentThread(),
				hTargetProcessHandle: GetCurrentProcess(),
				lpTargetHandle: out var threadHandle,
				dwDesiredAccess: 0, // ignored
				bInheritHandle: false,
				dwOptions: 0x00000001 | 0x00000002 // close source, same access
			);
			new { res, err = Marshal.GetLastWin32Error() }.Dump();
			using (threadHandle)
			{

				Task.Run(async () =>
				{
					await Task.Delay(TimeSpan.FromSeconds(2));
					var cancelRes = CancelSynchronousIo(threadHandle);
					new { cancelRes, error = Marshal.GetLastWin32Error() }.Dump();
				});	

				process.StandardOutput.ReadLine().Dump();
			}
		})
		.Wait(TimeSpan.FromSeconds(5));
	}
	finally
	{
		if (!process.HasExited) { process.Kill(); }
	}
}

[DllImport("kernel32.dll", SetLastError = true)]
static extern IntPtr GetCurrentThread();

[DllImport("kernel32.dll", SetLastError = true)]
static extern IntPtr GetCurrentProcess();

[DllImport("kernel32.dll", SetLastError = true)]
static extern bool CancelSynchronousIo(MySafeHandle hThread);

[DllImport("kernel32.dll", SetLastError = true)]
static extern bool DuplicateHandle(
	IntPtr hSourceProcessHandle,
	IntPtr hSourceHandle,
	IntPtr hTargetProcessHandle,
	out MySafeHandle lpTargetHandle,
	int dwDesiredAccess,
	bool bInheritHandle,
	int dwOptions
);

// https://docs.microsoft.com/en-us/archive/blogs/shawnfa/safehandle
[SuppressUnmanagedCodeSecurity]
[ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)]
[DllImport("kernel32.dll")]
static extern bool CloseHandle(IntPtr hObject);

internal sealed class MySafeHandle : SafeHandle
{
	public MySafeHandle() : base(IntPtr.Zero, ownsHandle: true) { }
	
	public override bool IsInvalid => this.handle == IntPtr.Zero;
	
	protected override bool ReleaseHandle() => CloseHandle(this.handle);
}

madelson avatar Jan 08 '20 01:01 madelson