This project is read-only.

Pure sync and async methods

Dec 27, 2011 at 1:01 PM

Hello,

I have investigated the source code of the library and some approaches seems me suspicious.

1. The Session class creates and start Task for Response processing with endless while:

 partial void ExecuteThread(Action action)
 {
       Task.Factory.StartNew(action, TaskCreationOptions.LongRunning);
 }

this.ExecuteThread(() =>
 {
      try
      {
           this.MessageListener();
      }
      finally
      {
           this._messageListenerCompleted.Set();
      }
});

 private void MessageListener()
 {
       try
       {
                while (this._socket.Connected)
                {
                    var message = this.ReceiveMessage();

                    if (message == null)
                    {
                        throw new NullReferenceException("The 'message' variable cannot be null");
                    }

                    this.HandleMessageCore(message);
                }
            }
            catch (Exception exp)
            {
                this.RaiseError(exp);
            }
 }

this.ReceiveMessage calls the following code eventually:

partial void SocketRead(int length, ref byte[] buffer)
        {
            var offset = 0;
            int receivedTotal = 0;  // how many bytes is already received

            do
            {
                try
                {
                    var receivedBytes = this._socket.Receive(buffer, offset + receivedTotal, length - receivedTotal, SocketFlags.None);
                    if (receivedBytes > 0)
                    {
                        receivedTotal += receivedBytes;
                        continue;
                    }
                    else
                    {
                        throw new SshConnectionException("An established connection was aborted by the software in your host machine.", DisconnectReason.ConnectionLost);
                    }
                }
                catch (SocketException exp)
                {
                    if (exp.SocketErrorCode == SocketError.WouldBlock ||
                        exp.SocketErrorCode == SocketError.IOPending ||
                        exp.SocketErrorCode == SocketError.NoBufferSpaceAvailable)
                    {
                        // socket buffer is probably empty, wait and try again
                        Thread.Sleep(30);
                    }
                    else
                        throw;  // any serious error occurred
                }
            } while (receivedTotal < length);
        }

As a result the Task is blocked by the endless reading.

Moreover the resources are wasted by the try/catch block usage only for "fix" endless reading using Thread.Sleep.

The other possible drawback of the code is:

SshConnectionException("An established connection was aborted by the software in your host machine.", DisconnectReason.ConnectionLost);

We encountered with the exception some times and I am not sure it is correct to throw it.

The similar code is encountered in the other places.

Please find ExecuteThread using text search.

2. There are a lot of places with lock(_socket) {} block.

Why it is significant for me?

Now I am engaged in the software development which should run some async ssh operations.

Our team uses Renci ssh library.

On the first look Renci ssh library (for example SftpClient) provides pure sync and pure async methods.

All looks good on the first look.

But internally the same "semi" async logic is used for emulate both sync and async methods.

As a result I am not able for example to ignore not optimal "async" logic and use only pure sync methods and wrap it by TPL Tasks by myself.

Probably my analysis is not correct in some aspect, then could you please answer: is it possible to create pure sync method for reading or writing file using your library without any TPL usage and any locks or other thread synchronization primitives?

If not - could you please create some very simple version of the library without additional "facilities" but based on the simple sync socket like here SendReceiveTest1?:

http://msdn.microsoft.com/en-us/library/w93yy28a.aspx?

Thanks in advance!

 

 

 

 

Coordinator
Dec 27, 2011 at 3:22 PM

Hi,

1. As far as endless while. This while loop will terminate only under one condition, when socket connection will be terminated, either by the server or by the client itself. Unfortunately I do need some background process which will be listening for incoming SSH messages. Also, I tried to avoid using try..catch technique but but I couldn't find any other way which will work under different conditions I tested against correctly, so I had to use this one. Also, some piece of code, which uses Sleep command I took from Microsoft site somewhere as an example on how to use it, so thats why I use it this way. If you ave any other suggestion on how to achieve that, please let me know, I would be glad to update or change the code.

The SshConnectionException usually thrown when server disconnects your session for whatever reason. So if it happens a lot on your server I would recommend to check logs and see why it disconnects.

2. lock (this._socket) appears only 2 times in the code, so I am a little confused when you say it appears a lot. It appears twice for each project in the solution so may be that's why it looks a lot. But it is required in both cases, since first time, I lock it so I could send the whole message without sending another by different thread. The second time it is required when I am cleaning it up, so once again , I have to lock it to ensure no other thread is using the socket.

 

If you want to avoid TPL usage, then you can use .NET 3.5 version, since it doesn't use TPL.

 

Unfortunately I can't create additional version. Originally, as I remember, I tried to implement it using simple version, like you mentioned Send/Receive style, but unfortunately it was not possible with the way SSH protocol works, at least I couldn't find a nice way to do it.

 

Hope it helps,

Thanks,

Oleg

 

Dec 28, 2011 at 1:08 PM

Hi Oleg,

Thank you for honest and detailed response.

I don’t know the details of the ssh protocol specification unfortunately, then let me state my point of view based on some very common concepts.

Let us consider that in order to make any ssh operation it is required to make some series of the request sending – response processing steps using tcp socket.

Let us consider that we expect some possible range of responses (the main type of response is base one – other types are additional).

Then each response should be processed and the next request should be sent then.

That is we have the following sequence of processing:

[operation]->

   [request-creation] ->[request]-> [socket] ->[response]->[response-processing]->

                                                                                                    [response-processing]->

[return_result]->[operation end]

As far as I understand to do it synchronously it is required:

  1. Create socket and connect to the server synchronously using simple sync Connect method

http://msdn.microsoft.com/en-us/library/ych8bz3x.aspx.

 

  1. Use sync socket Send method to send request http://msdn.microsoft.com/en-us/library/w93yy28a.aspx
  2. Use sync socket Receive method to receive response http://msdn.microsoft.com/en-us/library/8s4y8aff.aspx
  3. Make another Send-Receive couple and so on.
  4. Close socket when client is closed.

 

I believe that in such approach no collision should take place because the client does not send the next request without processing the previous response.

Only some rare exceptions is possible as far as I understand according to the http://tools.ietf.org/html/rfc4253#page-24 - 11.1 Disconnect, 11.2 Ignore, 11.3 Debug , 11.4.

All this cases can be ignored accept disconnect.

In order to create asynchronously implementations it is required:

  1. Create socket and connect to the server asynchronously using async BeginConnect method http://msdn.microsoft.com/en-us/library/tad07yt6.aspx pass callback to this method which should begin from EndConnect() you need pass socket to the async BeginConnect method as state parameter.
  2. BeginConnect callback should call BeginSend socket method and you need pass callback to it which should be started with EndSend and then call async BeginReceive method.
  3. BeginReceive method needs appropriate callback which should start with EndReceive and then call another BeginSend method and so on.
  4. Finally you need call Callback which is provided to the Ssh client operation.

In such case the order of the “receive – response” couples will be the same as in the sync case without any lock usage, and you will not block the main thread.

TPL library has special methods which can wrap async methods (From Async) http://msdn.microsoft.com/en-us/library/dd321427.aspx

That is why on the architectural level you can wrap all required async methods and create Tasks.

On the other hand you can wrap you own processing methods and client callbacks with Tasks.

As a result you will receive some collection of task somewhare:

Task[] steps;

Then you initialize continuation:

For each i: steps[i+1].ContinuteWith(()=>steps[i].Start());

Then you can start continuous execution using steps[0].Start();

My sample is primitive you certainly will have more complex Task graph for example in order to call some ssh Client callback during the main pipeline you need start two tasks simultaneously like:

steps[i+1].ContinueWith(()=>callback[k].Start(); steps[i].Start());

Probably it will be reasonable to create some sort of the common graph task architecture.

I have created such sample recently for our project purpose – it is rather brief.

Please let me know if you are interested in.

When some ssh client async method is called then it is simply wrapped by task graph and inserted into already existed one. You only need to know “final” task node for do it. If final node is already completed then you begin new graph. Such it is possible to construct task graph dynamical in the order of the ssh client async methods call.

 

Thanks,

Andrew.

Coordinator
Dec 29, 2011 at 12:29 AM

Well,

Unfortunatly I wont have time to create a new version so wont be able to help you with it

As far as request response logic, then another thing I thought of, is that you not always get response after request being sent.

Sometimes you need to send many many data request before you get a response, or vise versa.

Also, there are some cases where server occasionally might send an empty or keep-alive message which will need to be handled right away and if you don't have another thread which will take care of that, then connection will be closed and when main thread will try to execute a new command, it will be disconnected.

So for those, and probably other reason, which I cant remeber now, I had to create another thread.

I tried to optimize it so all I need is one background listener thread, which will be killed as soon as you dispose the client so I think the resource will not be wasted.

If you have any workable example, feel free to let me know and I can include it in the project if you like, sort of like a light version of the client, or something like that.

 

Hope it explains a little more about SSH logic.

Let me know if you have any further questions or comments.

Thanks,

Oleg