StreamReader on ShellStream runs into a lock

[cjung95]

I tried to update ssh.net to the latest version in my project and encountered a problem while testing my application.

The problematic change in ShellStream came with the commit “Fix a few issues with ShellStream (https://github.com/sshnet/SSH.NET/pull/1322)”. The method where the lock occurs is the read method in line 747 in the ShellStream.cs file. There is the following while loop:

while (_head == _tail && !_disposed)
{                 
    _ = Monitor.Wait(_sync);
}

But since _head == _tail returns true and the server sends no further response, the Monitor.Wait method does not return.

Before the change, it only checked if _incoming.Count > 0.

Is there a reason for this change that I can't see, or is it an error?

Edit: Maby a little bit more context could be nice

try
{
    _shell = _shellStreamClient.CreateShellStream("xterm", 80, 24, 800, 600, 1024);
    _reader = new StreamReader(_shell);
    _writer = new StreamWriter(_shell)
    {
        AutoFlush = true,
        NewLine = "\n"
    };

    _writer.Write("/tmp/power.sh\n");

    while (true)
    {
        if (DateTime.Now.Subtract(startTime) > timeout)
        {
            throw new TimeoutException("Timeout message [...]");
        }

        var output = _reader.ReadLine(); // The application locks here

        if (output == null || output == "")
        {
            Thread.Sleep(100);
            continue;
        }

        if (output.StartsWith("Enter on, off"))
        {
            _writer.Write("on\n");
        }

        if (output.StartsWith("relay on"))
        {
            break;
        }
    }
}
catch
{
    DisposeConnection();
    throw;
}

I can't put the reader, writer and stream in using blocks because I continue to use them outside the method scope.

[Rob-Hague]

Yeah it was an intentional decision to make ShellStream conform more to the usual Stream contract - the previous implementation of ShellStream.Read would return 0 when it shouldn't and at other times block when it shouldn't. The new implementation only returns when data is available and returns as soon as data is available, but I can see how it could make this kind of interaction difficult. Apologies that it was not called out in the release notes.

For your example, are you able to use the Write and ReadLine methods defined on ShellStream? ReadLine has a timeout argument. I imagine it would look something like:

  try
  {
      _shell = _shellStreamClient.CreateShellStream("xterm", 80, 24, 800, 600, 1024);
-     _reader = new StreamReader(_shell);
-     _writer = new StreamWriter(_shell)
-     {
-         AutoFlush = true,
-         NewLine = "\n"
-     };
  
-     _writer.Write("/tmp/power.sh\n");
+     _shell.Write("/tmp/power.sh\n");
  
      while (true)
      {
-         if (DateTime.Now.Subtract(startTime) > timeout)
+         var output = _shell.ReadLine(timeout);
+ 
+         if (output == null)
          {
              throw new TimeoutException("Timeout message [...]");
          }
  
-         var output = _reader.ReadLine(); // The application locks here
- 
-         if (output == null || output == "")
-         {
-             Thread.Sleep(100);
-             continue;
-         }
- 
          if (output.StartsWith("Enter on, off"))
          {
-             _writer.Write("on\n");
+             _shell.Write("on\n");
          }
  
          if (output.StartsWith("relay on"))
          {
              break;
          }
      }
  }
  catch
  {
      DisposeConnection();
      throw;
  }

[cjung95]

Hey Rob,

I am interacting with a shell script that contains read functions where no new line character is sent before waiting for input.
In order to handle this output, I needed a method that returns on a new line character or if nothing is sent for x seconds.
I came up with this solution.

public static string ReadUntilEolOrTimeout(ShellStream stream, int timeoutMilliseconds)
{
    var sb = new StringBuilder();
    var buffer = new byte[1]; // Read one char at a time
    var startTime = DateTime.Now;
    bool dataRead = false;

    while (true)
    {
        if (stream.DataAvailable)
        {
            // Attempt to read one character
            stream.Read(buffer, 0, 1);

            // Reset the start time after a successful read
            startTime = DateTime.Now;
            dataRead = true;

            if (buffer[0] == '\r')
            {
                continue;
            }

            // If we encounter EOL, exit the loop
            if (buffer[0] == '\n')
            {
                break;
            }

            // If previous character was '\r' and current is not '\n', we have EOL
            if (sb.Length > 1 && sb[sb.Length - 2] == '\r' && buffer[0] != '\n')
            {
                stream.Position--; // not possible becaue this always throws *NotSupportedException*
                break;
            }

            sb.Append(value: (char)buffer[0]);
        }
        else
        {
            // If no data was read, check if the timeout has been reached
            if (!dataRead || (DateTime.Now - startTime).TotalMilliseconds >= timeoutMilliseconds)
            {
                break;
            }

            Thread.Sleep(10);
        }
    }

    return sb.ToString();
}

But there is a problem when the stream returns a \r without a following \n, which is the normal behavior on Mac systems.
I can't move the _readHead back one step and I can't peek the next charakter in the stream.
So I have no way to check which end of line is being sent.

Do you have any idea how to fix this?

[Rob-Hague]

Do you need to know whether you hit \r or \n? If not, does this suit? ReadLine deals with \r and \n

public static string ReadUntilEolOrTimeout(ShellStream stream, int timeoutMilliseconds)
{
    string? line = stream.ReadLine(TimeSpan.FromMilliseconds(timeoutMilliseconds));
    
    if (line != null)
    {
        return line;
    }
    else // Timeout
    {        
        return stream.Read(); // This reads all available data in the buffer as a string (does not block)
    }
}

[cjung95]

Yes, you're right, that's all I need - and it's that simple!
Thank you so much for your help! :)