SSH.NET icon indicating copy to clipboard operation
SSH.NET copied to clipboard

SSHClient: ForwardedPortLocal: memory issue

Open jonathanvanpeteghem opened this issue 4 years ago • 6 comments

Hello,

I am using the following code below. I call this ConnectSSH() every 5 seconds (as a test to see how it performs) After I let this run a night, i see that the used memory increased significantly, and it keeps rising.

After some investigation on my own, i notice that when I remove the "forwardedportlocal" object, (so I don't make such an object, and I obviously don't start it) Then there is no memory issue. The client is created, and then disposed cleanly. I can see this happening the diagnostic tool of visual studio.

I already added portfwld.stop() and portfwld.dispose() together with of course SshClient.disconnect() and SshClient.dispose() but that did not help.

I made a version in vb.net and also one in c#, but the results are the same.

I also Commented the connection to the database to show that this is not the issue.

I hope someone can give me some insight of what is happening, or perhaps what I am doing wrong?

    class mySQLviaSHH
    {
        
        public void ConnectSSH()
        {
            using (var SSHClient = new SshClient("myhost", "administrator", "mypassword"))
            {
                try
                {
                    // Console.WriteLine("Trying SSH connection...")
                    SSHClient.Connect();

                    if (SSHClient.IsConnected)
                        Console.WriteLine("SSH connection is active");
                    else
                        Console.WriteLine("SSH connection has failed");

                    Console.WriteLine("Trying port forwarding...");

                    var portFwld = new ForwardedPortLocal("127.0.0.1", "127.0.0.1", 3306);
                    SSHClient.AddForwardedPort(portFwld);
                    portFwld.Start();

                    if (portFwld.IsStarted)
                        Console.WriteLine("Port forwarded: {0}", portFwld.ToString());
                    else
                        Console.WriteLine("Port forwarding has failed.");
                }
                catch (SshException e)
                {
                    Console.WriteLine("SSH client connection error: {0}", e.Message);
                }
                
            }
        }


    }
Public Sub ConnectSSH()
            Using SSHClient = New SshClient("myhost", "administrator", "mypassword")
                Try
                    Console.WriteLine("Trying SSH connection...")
                    SSHClient.Connect()

                    If SSHClient.IsConnected Then
                        Console.WriteLine("SSH connection is active")
                    Else
                        Console.WriteLine("SSH connection has failed")
                    End If

                    Console.WriteLine(vbCrLf & "Trying port forwarding...")

                    Dim portFwld = New ForwardedPortLocal("127.0.0.1", "127.0.0.1", 3306)
                    SSHClient.AddForwardedPort(portFwld)
                    portFwld.Start()

                    If portFwld.IsStarted Then
                        Console.WriteLine("Port forwarded: {0}", portFwld.ToString())

                        '        'Dim dbConnect As DBConnect = New DBConnect(portFwld.BoundHost, "wcs_sch", "root", "dbpassword", "3306")
                        '        'Dim ct = dbConnect.Count("wcs_msg_host")
                        '        'Console.WriteLine(ct.ToString())
                    Else
                        Console.WriteLine("Port forwarding has failed.")
                    End If

                Catch e As SshException
                    Console.WriteLine("SSH client connection error: {0}", e.Message)
                Catch e As System.Net.Sockets.SocketException
                    Console.WriteLine("Socket connection error: {0}", e.Message)
                End Try

            End Using

        End Sub

jonathanvanpeteghem avatar Jan 28 '21 08:01 jonathanvanpeteghem

I had to look for Mem-Leaks in an Application which uses SSH.NET. dotMemory also showed there is an Event handler leak for Forwarded Port Local. So I created a simple Console-Application and started/stopped Forwardings in a loop.

image

Analyzing shows that EventHandler<SocketAsyncEventArgs> are leaking:

image

Some research showed you have to Dispose SocketAsyncEventArgs yourself.

So I came up with this diff:


diff --git a/src/Renci.SshNet/ForwardedPortLocal.NET.cs b/src/Renci.SshNet/ForwardedPortLocal.NET.cs
index 3d9624a..49263ce 100644
--- a/src/Renci.SshNet/ForwardedPortLocal.NET.cs
+++ b/src/Renci.SshNet/ForwardedPortLocal.NET.cs
@@ -11,6 +11,7 @@ namespace Renci.SshNet
     {
         private Socket _listener;
         private CountdownEvent _pendingChannelCountdown;
+        private SocketAsyncEventArgs SocketAsyncEventArgs = null;

         partial void InternalStart()
         {
@@ -42,6 +43,7 @@ namespace Renci.SshNet
             {
                 e = new SocketAsyncEventArgs();
                 e.Completed += AcceptCompleted;
+                SocketAsyncEventArgs = e;
             }
             else
             {
@@ -235,6 +237,12 @@ namespace Renci.SshNet
                     _pendingChannelCountdown = null;
                     pendingRequestsCountdown.Dispose();
                 }
+
+                if (SocketAsyncEventArgs != null)
+                {
+                    SocketAsyncEventArgs.Completed -= AcceptCompleted;
+                    SocketAsyncEventArgs.Dispose();
+                }
             }
         }

Now dotMemory shows no more Event Handler leaks:

image

The leakage is minor, but still there. Might this be the Issue you noticed?

Since I'm not a socket-pro, @drieseng any opinion on that diff? Good enough for an PR?

darinkes avatar Feb 26 '21 08:02 darinkes

Hi,

@darinkes Thanks for picking this up. I think this might be it. I will try to take some time to test this. You are right the leakage is minor, and at first you wont notice it. But i wonder what might happen, if I let my app run for months. over the weekend I gained +/- 30 to 40 Mb, of course with intense use.

@drieseng I also want to note that I think that the mySQL connector which uses the SSH.net probably has the same memory leak problem. They have built in SSH.net in their connector, and if I write something every 5 seconds to my database via the SSH settings, the memory usage starts to rise and is not disposed. Without it, everything is fine.

Here below the implementation of a simple example with the mysql connector. I don't actualy execute a querry, just open and close the connection.

    Public Class SQLconnector
        Public Shared Function make_connection()
            Dim result As String = ""
            Try

                Dim builder = New MySqlConnectionStringBuilder()
                builder.UserID = "root"
                builder.Password = "databasepassword"
                builder.Server = "localhost"
                builder.Database = "wcs_sch"
                builder.Port = 3306
                builder.SshHostName = "something.dyndns.org"
                builder.SshUserName = "administrator"
                builder.SshPassword = "SSHadminpassword"
                builder.SshPort = 22

                'Dim connection As MySqlConnection
                result = "administrator"
                Using connection = New MySqlConnection(builder.ConnectionString)
                    result = "open con"
                    connection.Open()
                    result = connection.State.ToString()
                    connection.Close()
                End Using
            Catch ex As Exception
                result = ex.ToString
            End Try


            Return result
        End Function
    End Class

jonathanvanpeteghem avatar Feb 26 '21 09:02 jonathanvanpeteghem

Aye, SocketAsyncEventArgs seems to be a common issue in many applications. Root cause seems to be the example in the docs doesnt dispose it either: https://docs.microsoft.com/de-de/dotnet/api/system.net.sockets.socketasynceventargs?view=net-5.0

But again, no socket-pro 😅

darinkes avatar Feb 26 '21 10:02 darinkes

Just as a note, cause I noticed it while reading the Forwarding-Code:

Make the code do, what the comment says it does.

diff --git a/src/Renci.SshNet/ForwardedPortLocal.NET.cs b/src/Renci.SshNet/ForwardedPortLocal.NET.cs
index 49263ce..95f87d7 100644
--- a/src/Renci.SshNet/ForwardedPortLocal.NET.cs
+++ b/src/Renci.SshNet/ForwardedPortLocal.NET.cs
@@ -63,7 +63,7 @@ namespace Renci.SshNet
                 }
                 catch (ObjectDisposedException)
                 {
-                    if (_status == ForwardedPortStatus.Stopped || _status == ForwardedPortStatus.Stopped)
+                    if (_status == ForwardedPortStatus.Stopping || _status == ForwardedPortStatus.Stopped)
                     {
                         // ignore ObjectDisposedException while stopping or stopped
                         return;

darinkes avatar Feb 26 '21 10:02 darinkes

Has this issue been fixed or planning to fix ?

dichaos avatar Oct 30 '23 13:10 dichaos

Can you prepare a PR?

WojciechNagorski avatar Oct 30 '23 16:10 WojciechNagorski