lifeemotions / knx.net

KNX.net provides a KNX API for .NET
MIT License
101 stars 47 forks source link

Memory leak in KnxConnection.Connect() #36

Closed FalcoGer closed 5 years ago

FalcoGer commented 6 years ago

I've been using the example code provided in the form of

conn = new KnxConnectionTunneling(remoteIp, 3671, localIp, 3672);
// create event handlers
conn.KnxConnectedDelegate += Connected;
conn.KnxEventDelegate += Event;
conn.KnxStatusDelegate += Status;
conn.KnxDisconnectedDelegate += Disconnected;
conn.Connect();

where conn is a private field of type KnxConnection

now with Disconnected being:

private static void Disconnected()
        {
            if (conn == null)
            {
                Console.WriteLine("ERR: KNX Disconnected. Connection NULL");
            }
            else
            {
               Console.WriteLine("Disconnected from KNX.");
               Console.WriteLine("Disconnected. Attempting Reconnect in 5 seconds.");
               Thread.Sleep(5000);
               conn.Connect();
            }
        }

I've had no troubles for a good while. But at some point the knx ip gateway was disconnected (physically) and not reconnected. and after a while I got a lovely System.OutOfMemoryException. I traced back the problem to Connect(). so I tried (knx detached)

while(true)
{
    conn.Connect();
}

and within a minute or so memory went through the roof, eventually throwing the same exception. I tried calling GC.Collect() after each connection attempt, and then it took 2 minutes instead of one to die. Then I tried calling disconnect before calling GC.Collect(); and then making a brand new connection object. The same problem persists.

try
{
    while (true)
    {
        if (conn != null)
        {
            conn.Disconnect();
        }
        conn = null;
        GC.Collect();
        conn = new KnxConnectionTunneling(remoteIp, 3671, localIp, 3672);
        // create event handlers
        conn.KnxConnectedDelegate += Connected;
        conn.KnxEventDelegate += Event;
        conn.KnxStatusDelegate += Status;
        conn.KnxDisconnectedDelegate += Disconnected;
        conn.Connect();
    }
}
catch (Exception e)
{
    Console.WriteLine(e.ToString());
    Console.ReadKey();
    throw e;
}

Result:

System.OutOfMemoryException: Eine Ausnahme vom Typ "System.OutOfMemoryException" wurde ausgelöst.
   bei System.Threading.Thread.StartInternal(IPrincipal principal, StackCrawlMark& stackMark)
   bei System.Threading.Thread.Start(StackCrawlMark& stackMark)
   bei System.Threading.Thread.Start()
   bei KNXLib.KnxReceiver.Start()
   bei KNXLib.KnxConnectionTunneling.Connect()
   bei XXX.XXX.Main(String[] args) in C:\Users\<user>\Desktop\XXX\XXX\XXX.cs:Zeile 125.

Then I tried to remove the delegates by setting them null before disconnecting and calling GC.Collect(). That didn't help either. So I'm all out of ideas for a workaround.

I'm using this library in a critical application and can't have it crash on me. Please fix your stuff.

abelsilva commented 6 years ago

Thanks for the report, I'll look into it

abelsilva commented 6 years ago

can you check latest version 1.1.10? Should be available on nuget in a few minutes https://www.nuget.org/packages/KNX.net/1.1.10

FalcoGer commented 6 years ago

thanks for checking that out. I'll get the new version first thing tomorrow.

FalcoGer commented 5 years ago

This is solved