ela-compil / BACnet

BACnet protocol library for .NET :satellite:
https://www.nuget.org/packages/bacnet/
MIT License
215 stars 95 forks source link

Memory leak on Linux #89

Open moebassist opened 2 years ago

moebassist commented 2 years ago

On Linux, there's a small memory leak.

Over a period of 25 minutes, 3.2mb was lost.

When the BACnet server is running but no clients are polling for real-time data (e.g. it's just sat there) - memory is stable. When clients poll, it rises.

This could be a tough one to crack.....any thoughts?

gralin commented 2 years ago

3.2MB isn't much, this could be easily caused by extra resources allocated by the CLR. But if you want to check to make sure, you could try to dump memory from two points in time (forcing GC before each) and check new resources which have been allocated. If you find some that shouldn't be there, check what is holding the mand why there weren't collected. I don't have experience in doing this on Linux but maybe if you attach VS debugger to app running in container/VM you could use the build in memory profiler. There is also dotMemory version for Linux. Tess Ferrandez also wrote a whole article about other options: https://www.tessferrandez.com/blog/2021/03/18/debugging-a-netcore-memory-issue-with-dotnet-dump.html

moebassist commented 2 years ago

Over a 48 hour period my app has lost 27% memory. When the BACnet server is disabled, it sits happily at 19%.....forever.

Definitely a leak somewhere....I've started to trawl through it. The leak is small - I think this will be a nightmare to find since it could be in the BACnet stack, SharpPcap or mono. My hardware is an iMX6 with 1GB ram, so it's not really ideal for installing Rider and profiling with that.

gralin commented 2 years ago

Maybe do a similar test on different Linux machine with more resources? Or compare to a test without Mono by simply using .NET 5?

scottpidzarko commented 1 year ago

@moebassist did you eventually pinpoint the issue / confirm a leak?

Scott

moebassist commented 1 year ago

Hi Scott,

I didn't.....I force a garbage collection and that reclaimed the memory....I know this isn't a great idea but I didn't have the time to review the entire library, and I don't have the CPU power/memory for a decent profiling.

It could be the recursive connection.BeginReceive(OnReceiveData... since tasks aren't disposed....this is a wild guess though!

I made a few changes to this library:

//find in storage
var obj = FindObject(objectId);
if (obj == null)
    return ErrorCodes.UnknownObject;

//object found now find property
var p = FindProperty(objectId, propertyId);
if (p == null)
    return ErrorCodes.NotExist;

became:

if (!Objects.TryGetValue(objectId.instance, out clsBNObject obj)) return eBACnetErrorCodes.UnknownObject;
if (!obj.Properties.TryGetValue((int)propertyId, out clsBNProperty p)) return eBACnetErrorCodes.NotExist;

My application is dynamic...there could be a few hundred objects or a few thousand dependent upon config so I implemented dictionary use for simplicity, and it makes the storage class vary simple to use, since my dictionary keys are my BACnet object numbers.

One thing that was tricky - BACnet Multi States. are array based (1/2/3/4/5) whereas mine are enumerative, including gaps and negative numbers - e.g. an enumeration like this:

-1 : Error 0: Off 1 : On 4 : Failed

Is adjusted by adding the lowest absolute value+1:

1: Error 2: Off 3: On 4: (Not Used) 5: (Not Used) 6: Failed

Anthony.

scottpidzarko commented 1 year ago

Thanks Anthony for your prompt response, this will be really helpful. I am also dealing with a GB of RAM, in Mono, on a Linux kernel.

If it's running in production without major issues coming back to you about memory then that calms my fear of an actual leak down. Maybe when I finish my December deadline and things calm down for the holidays I'll spin up a virtual machine and try and replicate the memory climbing.

moebassist commented 1 year ago

I have a 1 minute timer that performs all sorts of actions; I just banged a GC.Collect() in there. It’s running 24/7 without issues.

Ant.

YarekTyshchenko commented 1 year ago

If GC can collect the memory then it doesn't sound like a leak. I'm sure the library could be made to do fewer allocations, but IMO as long as there's no leak, and it doesn't cause performance issue this isnt something to worry about