Skip to content

PipeStream returns 0 instead of blocking when running a command with intermittent output #12

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

Closed
lucastheisen opened this issue May 6, 2016 · 2 comments · Fixed by #1399
Labels
Milestone

Comments

@lucastheisen
Copy link

For details, you can see this issue on StackOverflow.

@lucastheisen
Copy link
Author

I kinda resolved this with this patch:

ltheisen@MM206678-PC ~/git/sshnet
$ git diff
diff --git a/src/Renci.SshNet/Common/PipeStream.cs b/src/Renci.SshNet/Common/PipeStream.cs
index bfb7787..57e3fc6 100644
--- a/src/Renci.SshNet/Common/PipeStream.cs
+++ b/src/Renci.SshNet/Common/PipeStream.cs
@@ -213,10 +213,22 @@
         /// <returns><c>True</c> if data available; otherwise<c>false</c>.</returns>
         private bool ReadAvailable(int count)
         {
+            if (Length == 0 && !IsEndOfStream())^M
+            {
+                return false;
+            }
+
             return (Length >= count || _isFlushed) &&
                    (Length >= (count + 1) || !BlockLastReadBuffer);
         }

+        public Func<bool> EndOfStream { private get; set; }
+
+        private bool IsEndOfStream()
+        {
+            return (EndOfStream == null) ? Length == 0 : EndOfStream();
+        }
+
         ///<summary>
         ///When overridden in a derived class, writes a sequence of bytes to the current stream and advances the current position within this stream by the number of bytes written.
         ///</summary>
diff --git a/src/Renci.SshNet/SshCommand.cs b/src/Renci.SshNet/SshCommand.cs
index 391bec8..86be886 100644
--- a/src/Renci.SshNet/SshCommand.cs
+++ b/src/Renci.SshNet/SshCommand.cs
@@ -246,7 +246,9 @@ namespace Renci.SshNet

             //  Initialize output streams
             OutputStream = new PipeStream();
+            ((PipeStream)OutputStream).EndOfStream = () => { return _asyncResult.IsCompleted; };
             ExtendedOutputStream = new PipeStream();
+            ((PipeStream)ExtendedOutputStream).EndOfStream = () => { return _asyncResult.IsCompleted; };

             _result = null;
             _error = null;

Though I imagine you may have a better approach in mind. If you would like, I could create a pull request with this patch... Just let me know.

lucastheisen added a commit to lucastheisen/SSH.NET that referenced this issue May 6, 2016
@drieseng drieseng added this to the 2016.1.0 milestone Jul 29, 2016
@drieseng drieseng modified the milestones: 2017.0.0, 2016.1.0 Dec 7, 2016
@TheCloudlessSky
Copy link

@lucastheisen I know this is older and I'm sure you've since moved on from this. But, I saw your StackOverflow question and provided an answer. I've replicated it here:

Another problem with PipeStream is that if you attempt to read more than the available bytes, it will block. For performance reasons, blocking should be the job of the consumer of the PipeStream. I use SSH.NET to execute SshCommand and asynchronously read the standard output/error. My workaround for the problems is to write to an intermediary MemoryStream and then use standard mechanisms like StreamReader. This is a more generalized answer to read from the PipeStream:

public class SshCommandStreamReader : IDisposable
{
    private readonly Stream stream;
    private readonly MemoryStream intermediateStream;
    private readonly StreamReader reader;

    public SshCommandOutputReader(Stream stream)
    {
        this.stream = stream;
        this.intermediateStream = new MemoryStream();
        this.reader = new StreamReader(intermediateStream, Encoding.UTF8);
    }

    private int FlushToIntermediateStream()
    {
        var length = stream.Length;

        if (length == 0)
        {
            return 0;
        }

        // IMPORTANT: Do NOT read with a count higher than the stream length (which is typical of reading
        // from streams). The streams for SshCommand are implemented by PipeStream (an internal class to
        // SSH.NET). Reading more than the current length causes it to *block* until data is available.
        // If the stream is flushed when reading, it does not block. It is not reliable to flush and then
        // read because there is a possible race condition where a write might occur between flushing and
        // reading (writing resets the flag that it was flushed). The only reliable solution to prevent
        // blocking when reading is to always read the current length rather than an arbitrary buffer size.
        var intermediateOutputBuffer = new byte[length];
        var bytesRead = stream.Read(intermediateOutputBuffer, 0, intermediateOutputBuffer.Length);
        intermediateStream.Write(intermediateOutputBuffer, 0, bytesRead);
        return bytesRead;
    }

    public string Read()
    {
        var bytesFlushed = FlushToIntermediateStream();

        // Allow reading the newly flushed bytes.
        intermediateStream.Position -= bytesFlushed;

        // Minor optimization since this may be called in a tight loop.
        if (intermediateStream.Position == intermediateStream.Length)
        {
            return null;
        }
        else
        {
            var result = reader.ReadToEnd();
            return result;
        }
    }

    public void Dispose()
    {
        reader.Dispose();
        intermediateStream.Dispose();
    }
}

And then use it:

using (var command = client.CreateCommand("your command text"))
{
    var cmdAsyncResult = command.BeginExecute();

    using (var standardOutputReader = new SshCommandStreamReader(command.OutputStream))
    {
        while (!cmdAsyncResult.IsCompleted)
        {
            var result = standardOutputReader.Read();
            if (!String.IsNullOrEmpty(result))
            {
                Console.Write(result);
            }

            // Or what ever mechanism you'd like to use to prevent CPU thrashing.
            Thread.Sleep(1);
        }

        // This must be done *after* the loop and *before* EndExecute() so that any extra output
        // is captured (e.g. the loop never ran because the command was so fast).
        var resultFinal = standardOutputReader.Read();
        if (!String.IsNullOrEmpty(resultFinal))
        {
            Console.Write(resultFinal);
        }
    }

    command.EndExecute(cmdAsyncResult);
}

You should be able to modify this sample to read from standard error (via the ExtendedOutputStream) and also change it to read line-by-line which is specific to your application.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants