ShellStream.Read not blocking when no data available

[oblaise]

ShellStream.Read returns immediately with 0 bytes read when no data is available instead of blocking the call.
Consequently the other functions implemented in the parent class Stream that implement the asynchronous patterns based on the Read function (BeginRead/EndRead, ReadAsync) return also immediately with a 0 byte read instead of being suspended until data is available.
This prevent using correct asynchronous pattern with these functions.

The following modified function waiting on _dataReceived when no data is available seems to work:

public override int Read(byte[] buffer, int offset, int count)
{

    var i = 0;
    while (true)
    {
        lock (_incoming)
        {
            for (; i < count && _incoming.Count > 0; i++)
            {
                buffer[offset + i] = _incoming.Dequeue();
            }
        }

        if (i != 0) 
            return i;

        _dataReceived.WaitOne();
    }
}

[oblaise]

While looking more further to the ShellStream class I think that the public string Read() function should deserve also a blocking behavior where string is returned only when data is available. It will be more consistent with the public string ReadLine() function.

What do you think about implementing blocking behavior for the Read and ReadLine functions ?

[drieseng]

I agree that we should probably do this.
Still need to look into it a little further though.
I first want to focus on getting a 2016.0.1 bug fix release out.

[ghost]

I agree. Read needs to block. It's useless (at least for me) without blocking

[Bugalex]

Hello,
a long time ago (i think almost 2 years) i had the same problem. At this time i wrote a new Pipe class which behaves as expected. I use this new Implementation only in the SshCommand and ScpClient class. After that change running commands like "cd /var; tar -cvz ./*" or "dd if=/dev/sdb" etc. are no problem anymore.
Another sideeffect was that the download/upload is much faster. Using a StreamReader is also possible.
I provided a patch but the problem is/was that it will break implementations which use the SshCommand class.

If some is interested i can provide a patch for the current version (I´m still using the last release from CodePlex). It will possible also solve other Issues i found here.

[Shrafe]

I would very much like the fix detailed by @oblaise integrated into the next release. Interacting with devices that don't implement the exec channel correctly force interactive terminal emulation, and the fact that Read() does not block makes it very difficult to properly do this.

After patching in @oblaise 's fix and using it, my code works correctly. WIthout it, because .Read() immediately returns the 0 byte read, nothing is output.

[IgorMilavec]

Trying to push towards vNext milestone. The PR only fixes the OP's issue as it is indisputably a bug. It is hard to reason about the problem with public string Read() that was raised later as we do not know how it is used in the wild.
@Bugalex can you please provide a link to your rewrite? In which way do you anticipate it will break callers? Are you willing to contribute your implementation to SSH.NET?

[Bugalex]

I basically created a new Pipe class which is handling the blocking until one side is closed.

This new class can also be used with the Shell:
Dictionary<TerminalModes, uint> terminalModes = new Dictionary<TerminalModes, uint>(); Pipe output = new Pipe(65536 * 2, PipeFlags.NoCopy, PipeFlags.Default); Pipe input = new Pipe(4096, PipeFlags.Sync, PipeFlags.Default); Shell shell = this.client.CreateShell(input.OutStream, output.InStream, output.InStream, "xterm", (uint)tSize.Width, (uint)tSize.Height, (uint)pSize.Width, (uint)pSize.Height, terminalModes);

Also one example for the SshCommand:
`byte[] data;
using (MemoryStream stream = new MemoryStream())
{
SshCommand cmd = this.Owner.Hab.Connection.CreateCommand("dd "if=/var/log/messages"", Encoding.ASCII);
IAsyncResult r = cmd.BeginExecute();

byte[] buf = new byte[1024];
int count = 0;
while ((count = cmd.OutputStream.Read(buf, 0, buf.Length)) > 0)
    stream.Write(buf, 0, count);

string end = cmd.EndExecute(r);
if (end != null && end.Length > 0)
{
    byte[] rest = Encoding.ASCII.GetBytes(end);
    stream.Write(rest, 0, rest.Length);
}
cmd.Dispose();
data = stream.ToArray();

}`
I attachted the files i modified or created. Note: The original version of the files in this repo is from 2016.
SSH.zip

[oblaise]

I think that when moving to .netstandard 2.0 as a min requirement you should consider the System.IO.Pipelines library.

https://devblogs.microsoft.com/dotnet/system-io-pipelines-high-performance-io-in-net/

[WojciechNagorski]

This issue has been fixed in the 2024.0.0 version.