Task return a StreamReader in c#

Question

I have this task in C# that should return the standard output of DISM, so I can use it where i need:

public async Task<StreamReader> DISM(string Args)
{

   StreamReader DISMstdout = null;

    await Task.Run(() =>
    {
        Process DISMcmd = new Process();

        if (Environment.Is64BitOperatingSystem)
        {
            DISMcmd.StartInfo.FileName = System.IO.Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.Windows), "SysWOW64", "dism.exe");
        }
        else
        {
            DISMcmd.StartInfo.FileName = System.IO.Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.Windows), "System32", "dism.exe");
        }

        DISMcmd.StartInfo.Verb = "runas";

        DISMcmd.StartInfo.Arguments = DISMArguments;

        DISMcmd.StartInfo.WindowStyle = ProcessWindowStyle.Hidden;
        DISMcmd.StartInfo.CreateNoWindow = true;
        DISMcmd.StartInfo.UseShellExecute = false;
        DISMcmd.StartInfo.RedirectStandardOutput = true;
        DISMcmd.EnableRaisingEvents = true;
        DISMcmd.Start();

        DISMstdout = DISMcmd.StandardOutput;

        DISMcmd.WaitForExit();
    });
    return DISMstdout;
}

But it doesn't really work. If I want to read the standardoutput from another task I can't (because it is empty) So there must be a problem with my task?.

public async Task Test()
{
    await Task.Run(() =>
    {

    StreamReader DISM = await new DISM("/Get-ImageInfo /ImageFile:" + ImagePath + @" /Index:1");

    string data = string.Empty;
     MessageBox.Show(DISM.ReadToEnd()); // this should display a msgbox with the standardoutput of dism

     while ((data = DISM.ReadLine()) != null)
     {
         if (data.Contains("Version : "))
         {
               // do something
         }
     }
   }); 
}

What is wrong with this piece of code?

You return the stream for the process' stdout after the process exits, at that point the process no longer exists and I'm guessing neither do its standard IO streams - .NET would have to keep them around until they're read for you. — millimoose, Jul 25 '18 at 09:31
Try reading from the stream before you call `WaitForExit()` and I'm betting your data will be there. I would do something like passing a callback to `DISM()` that reads from the stream, and returning the result of the callback as the return value of the method. Then this whole shebang will have executed once you make it past `await DISM(...)` — millimoose, Jul 25 '18 at 09:34
Last but not least I don't expect `ReadLine()` to return anything after you've called `ReadToEnd()`. `ReadToEnd()` doesn't mean "read all the data that is currently available", it means "block until the stream is closed and no more data will ever be available" — millimoose, Jul 25 '18 at 09:35
The program cannot exit until you've read all the output of the program. So WaitForExit() is very likely to deadlock. It is not clear why you need it, but consider the Exited event if you do. Use StringReader instead. — Hans Passant, Jul 25 '18 at 09:36
Try changing the calling method to: `StreamReader DISMoutput = await this.DISM("(...)");`. (no `new`) Completely eliminate the Task.Run() lambda. In the other method, eliminate `DISMcmd.EnableRaisingEvents = true;`. Should go asynchronous. And check the Arguments you're passing. — Jimi, Jul 25 '18 at 09:53
@millimoose but DISMstdout = DISMcmd.StandardOutput; should to the trick. Even if i remove ReadToEnd it doesn't work. i have that task inside another class, that's why i'm using 'new' .I have removed the task.run() lambda but it doesn't work. — devlime26, Jul 25 '18 at 10:00
In that class i also use NLOG and I can see that the DISM output is right (so the arguments are not the problem). — devlime26, Jul 25 '18 at 10:02
But, as Hans said, you probably want to use the process event. You could pass your stream to whatever is expecting it, when `Exited` is raised. In this case, you should also use `Process.BeginOutputReadLine()` and have an `OutputDataReceived` handler. And eliminate `.WaitForExit();` It would work more or less in the same way. — Jimi, Jul 25 '18 at 10:03
@devlime26 Should it though? Because it clearly doesn't. A Stream object in .NET is probably not much more than a low-level operating system handle, the utility methods that are implemented through system calls with that handle, and maybe a buffer. It does not meaningfully provide a copy of the stream that can survive the lifetime of this low-level stuff. The assignment just stores the same wrapper around the same handle to a new variable. — millimoose, Jul 25 '18 at 10:03
@devlime26 I didn't say to remove `ReadToEnd()`, I said to remove `WaitForExit()` and read all the data *before* the process exits. After you `await DISM(...)`, that method has already finished and the process is gone. Or follow the advice of the other guys, they seem to be on firmer footing here. — millimoose, Jul 25 '18 at 10:04
Then how can I save the StandardOutput so I can use it after the process has been finished? — devlime26, Jul 25 '18 at 10:34
@devlime26 Read it before the process finishes and save the actual output. — millimoose, Jul 25 '18 at 10:41

millimoose · Accepted Answer · 2018-07-25T11:58:27.293

The way I'd write your method to exploit async..await as opposed to the legacy asynchronous approaches is like this:

public async Task<TResult> WithDism<TResult>(string args, Func<StreamReader, Task<TResult>> func)
{
    return await Task.Run(async () =>
    {
        var proc = new Process();

        var windowsDir = Environment.GetFolderPath(Environment.SpecialFolder.Windows);
        var systemDir = Environment.Is64BitOperatingSystem ? "SysWOW64" : "System32";
        proc.StartInfo.FileName = Path.Combine(windowsDir, systemDir, "dism.exe");

        proc.StartInfo.Verb = "runas";

        proc.StartInfo.Arguments = args;

        proc.StartInfo.WindowStyle = ProcessWindowStyle.Hidden;
        proc.StartInfo.CreateNoWindow = true;
        proc.StartInfo.UseShellExecute = false;
        proc.StartInfo.RedirectStandardOutput = true;
        proc.Start();

        Console.Error.WriteLine("dism started");

        var result = await func(proc.StandardOutput);

        Console.Error.WriteLine("func finished");
        // discard rest of stdout
        await proc.StandardOutput.ReadToEndAsync();
        proc.WaitForExit();

        return result;
    });
}

Since realistically, the only part where significant blocking can occur when spawning a process is as you handle the output it produces. Used like this:

var task = WithDism("/?", async sr => await sr.ReadToEndAsync()); // or process line-by-line
Console.WriteLine("dism task running");
Console.WriteLine(await task);

it produces the following output

dism task running
dism started
func finished

Error: 740

Elevated permissions are required to run DISM.
Use an elevated command prompt to complete these tasks.

Do note that when using subprocesses, it's your job to make sure they correctly exit or are shut down to avoid leaving zombie processes around. That's why I've added the possibly redundant ReadToEndAsync() - in case func still leaves some output unconsumed, this should allow the process to reach its natural end.

However, this means the calling function will only proceed once that happens. If you leave behind a lot of unconsumed output you're not interested in, this will cause an unwanted delay. You could work around this by spawning off this cleanup to a different background task and returning the result immediately using something like:

Task.Run(() => {
    // discard rest of stdout and clean up process:
    await proc.StandardOutput.ReadToEndAsync();
    proc.WaitForExit();
});

but I admit I'm going a bit out on a limb there, I'm not entirely sure about the robustness of just letting a task "run wild" like that. What the appropriate way to clean up the process is will, of course, depend on what it's actually doing after you get the output you want to return from func.

I'm using synchronous calls to Console there because they only serve to illustrate the timing of events, I want to know that as execution reaches that point. Normally you would use async in a "viral" way to make sure control passes back to top-level as soon as possible.

@Jimi - the final part is an "it depends" kind of thing. If you can rely on the `func` passed into `WithDism` consuming the whole stream then it makes little difference. But if, say, you're only looking for one specific line in the middle of output, there's no sense in waiting for all the later lines to be read before you return the result. In fact, there might be no reason to even let it finish running so you might want to interrupt it instead of reading what could potentially be a lot of unneeded output. — millimoose, Jul 25 '18 at 12:05
But this being .NET/Windows, you can't just send SIGINT to a process, so you're on your own as to how exactly to cleanly or not cleanly shut down a spawned process. — millimoose, Jul 25 '18 at 12:05
This is because a Process is usually handled with its own Events (it's built to be event-driven, different from the asynchronous patter, nonetheless effective), so you subscribe its `Exited` event (with `.EnableRaisingEvents = true;`) and eventually, have a time-out/done-with-it logic that interrupts the process (a bit harsh, but `Proces.Kill` raises the `Exited` event) => About [sending a signal in Windows](https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/signal) (different language, I agree :) — Jimi, Jul 26 '18 at 07:14
@Jimi So technically to use async/await here you don't even need the events, you just read what you need, set a timer, kill the process in a background task if it still runs? — millimoose, Jul 26 '18 at 08:10
@Jimi Out of curiosity about this I posted a slightly modified version of this answer to Code Review, you might want to answerify your comments there for at least one upvote from me: https://codereview.stackexchange.com/questions/200287/an-async-await-based-way-to-handle-the-output-of-a-child-process — millimoose, Jul 26 '18 at 08:10

score 2 · Answer 2 · answered Jul 27 '18 at 09:43

After playing around with this using Benchmark.NET, it seems that starting a process (I tried DISM and Atom to have something hefty) - from setup to Start() - takes about 50 milliseconds. This seems pretty negligible to me for this use. After all, 50ms is good enough latency for say playing League of Legends, and you're not going to start these in a tight loop.

I'd like to provide an alternative answer of "don't bother with Task.Run() and just use async I/O in a straightforward way" unless you absolutely need to get rid of that delay and believe spawning off a background thread will help:

static string GetDismPath()
{
    var windowsDir = Environment.GetFolderPath(Environment.SpecialFolder.Windows);
    var systemDir = Environment.Is64BitOperatingSystem ? "SysWOW64" : "System32";
    var dismExePath = Path.Combine(windowsDir, systemDir, "dism.exe");

    return dismExePath;
}

static Process StartDism(string args)
{
    var proc = new Process
    {
        StartInfo =
        {
            FileName = GetDismPath(),
            Verb = "runas",
            Arguments = args,
            WindowStyle = ProcessWindowStyle.Hidden,
            CreateNoWindow = true,
            UseShellExecute = false,
            RedirectStandardOutput = true
        }
    };

    proc.Start();

    return proc;
}
static void Cleanup(Process proc)
{
    Task.Run(async () =>
    {
        proc.StandardInput.Close();
        var buf = new char[0x1000];
        while (await proc.StandardOutput.ReadBlockAsync(buf, 0, buf.Length).ConfigureAwait(false) != 0) { }
        while (await proc.StandardError.ReadBlockAsync(buf, 0, buf.Length).ConfigureAwait(false) != 0) { }

        if (!proc.WaitForExit(5000))
        {
            proc.Kill();
        }
        proc.Dispose();
    });
}
static async Task Main(string[] args)
{
    var dismProc = StartDism("/?");

    // do what you want with the output
    var dismOutput = await dismProc.StandardOutput.ReadToEndAsync().ConfigureAwait(false);

    await Console.Out.WriteAsync(dismOutput).ConfigureAwait(false);
    Cleanup(dismProc);
}

I'm only using Task.Run() to keep the cleanup off the main thread in case you need to do something else while DISM keeps producing output you're not interested in that you do not wish to kill outright.

Task return a StreamReader in c#

2 Answers2