Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: possible memory leak #27

Open
amadeo-alex opened this issue Apr 1, 2024 · 11 comments
Open

Bug: possible memory leak #27

amadeo-alex opened this issue Apr 1, 2024 · 11 comments

Comments

@amadeo-alex
Copy link

amadeo-alex commented Apr 1, 2024

Hello,

I've been experimenting with various "CoreAudio" wrapper libraries and it looks like most of them, including this one, appear to be leaking memory - slowly but constantly (or it's me using the library wrong, not excluding that :D)

I have a memory dump created at the end of the tests however github allows attachments only up to 25MB - compressed dump is 30MB+. I can deliver it via preferred method if required.

Test project is a clean new one with CoreAudio installed via nuget.
Code used for testing (put directly into main function, please ignore the var name typo):

        var enumerator = new MMDeviceEnumerator(Guid.Empty);
        while (true)
        {
            foreach (var device in enumerator.EnumerateAudioEndPoints(DataFlow.All, DeviceState.Active))
            {
                var termpVar = device.DeviceFriendlyName;
                device.Dispose();
            }
        }

Results:
oJeViP
OvLLpo
Yk0j7R
2SNesS

@morphx666
Copy link
Owner

morphx666 commented Apr 1, 2024

Yes, continuously calling enumerator.EnumerateAudioEndPoints will cause memory consumption to go up. Not sure if it's a problem with the library or the native code itself.

However, it is not recommended to do so. Instead, in order to keep an updated list of devices you should use MMNotificationClient and listen for notifications (events).

Something like this:

using CoreAudio;

internal class Program {
    private static void Main(string[] args) {
        var devices = new List<MMDevice>();
        var enumerator = new MMDeviceEnumerator(Guid.Empty);

        foreach(var device in enumerator.EnumerateAudioEndPoints(DataFlow.All,DeviceState.Active)) {
            devices.Add(device);
        }

        var nc = new MMNotificationClient(enumerator);
        nc.DeviceAdded += (sender, e) => {
            lock(devices) {
                MMDevice device = enumerator.GetDevice(e.DeviceId);
                devices.Add(device);
            }
        };
        nc.DeviceRemoved += (sender, e) => {
            lock(devices) {
                MMDevice device = enumerator.GetDevice(e.DeviceId);
                devices.Remove(device);
            }
        };
        nc.DeviceStateChanged += (sender, e) => {
            // What to do here is really up to you...
            lock(devices) {
                MMDevice device = enumerator.GetDevice(e.DeviceId);
                if((e.DeviceState & DeviceState.Active) == DeviceState.Active) {
                    devices.Add(device);
                } else {
                    devices.Remove(device);
                }
            }
        };

        Task.Run(async () => {
            while(true) {
                lock(devices) {
                    foreach(var device in devices) {
                        var deviceName = device.DeviceFriendlyName;

                        // Not required because we never called GetAudioEndpointVolume or GetAudioSessionManager2
                        //device.Dispose(); 

                        // Do something with the device's name...
                    }
                }

                await Task.Delay(10);
            }
        }).Wait();
    }
}

Please avoid using lock the way I did. Use a thread-safe collection instead.

@amadeo-alex
Copy link
Author

Thank very much for a quick answer!
I really like the proposed solution with events, didn't know it can be done this (cleaner) way.
I'll play with it, this library is one currently used in the project I'm maintaining so I'd be really glad to keep it.

@amadeo-alex
Copy link
Author

amadeo-alex commented Apr 2, 2024

Is session handling supposed to be done in similar fashion - OnSessionCreated? I've created a simple example based on the code above that iterates the sessions and I can still see memory usage creeping up slowly despite disposing "everything".
Code (put instead of the Task from your example above):

        while (true)
        {
            lock (devices)
            {
                foreach (var device in devices)
                {
                    var deviceName = device.DeviceFriendlyName;
                    var sessionManager = device.AudioSessionManager2;
                    if(sessionManager == null)
                        continue;

                    var sessions = sessionManager.Sessions;
                    if (sessions == null)
                        continue;

                    foreach (var session in sessions)
                    {
                        var s = session;
                        s.Dispose();
                    }

                    sessionManager.Dispose();
                    device.Dispose();
                }
            }
        }

@amadeo-alex
Copy link
Author

I haven't confirmed that yet but it looks like even the basic setup with events leak a bit of memory?
Not certain at this point, I'll let it run for a few hours and report back.

@amadeo-alex
Copy link
Author

I'll be honest, I'm confused at this point :D
Code:

internal class Program
{
    private static ConcurrentDictionary<string, MMDevice> _devices = [];


    private static void AddDevice(MMDevice device)
    {
        Console.WriteLine($"adding device: {device.DeviceFriendlyName}");
        _devices[device.DeviceFriendlyName] = device;
    }

    private static void RemoveDevice(MMDevice device)
    {
        Console.WriteLine($"removing device: {device.DeviceFriendlyName}");
        _devices.Remove(device.DeviceFriendlyName, out var removedDevice);
        removedDevice?.Dispose();
        device?.Dispose();
    }


    static void Main(string[] args)
    {
        var enumerator = new MMDeviceEnumerator(Guid.Empty);

        foreach (var device in enumerator.EnumerateAudioEndPoints(DataFlow.All, DeviceState.Active))
        {
            AddDevice(device);
        }

        var nc = new MMNotificationClient(enumerator);
        nc.DeviceAdded += (sender, e) =>
        {
            var device = enumerator.GetDevice(e.DeviceId);
            AddDevice(device);
        };
        nc.DeviceRemoved += (sender, e) =>
        {
            var device = enumerator.GetDevice(e.DeviceId);
            RemoveDevice(device);
        };
        nc.DeviceStateChanged += (sender, e) =>
        {
            var device = enumerator.GetDevice(e.DeviceId);
            if ((e.DeviceState & DeviceState.Active) == DeviceState.Active)
            {
                AddDevice(device);
            }
            else
            {
                RemoveDevice(device);
            }
        };

        Console.WriteLine("---------------------------------------");
        foreach (var device in _devices.Values)
        {
            Console.WriteLine($"device: {device.DeviceFriendlyName}");
        }
        Console.WriteLine("---------------------------------------");
 

        while (true)
        {

        }
    }
}

Information from VS:
UypX4o
jGNycO
7Ag0YK

Opposing information from Task Manager:
MBjQ8g

@morphx666
Copy link
Owner

At first glance, your code looks fine... I will run it locally to see if I can figure out what's going on.

@morphx666
Copy link
Owner

morphx666 commented Apr 4, 2024

To be honest, I'm not sure what's going on... but I feel that this appears to be normal behavior.

I followed the instructions found here and couldn't find anything useful.

After all, the app sits there, doing nothing in an infinite loop... there's no chance that the CoreAudio library is leaking memory if it's not being used.

Here's one test you can do... remove the code related to CoreAudio and run the app with just the infinite loop and you will see that, although not as fast, the process memory slowly, but steadily, keeps growing.

image

Maybe someone with a deeper understanding of dotnet's inner workings (I'm thinking of @Elfocrash) could shed some light on this issue...

@amadeo-alex
Copy link
Author

Ohhh, that is more than interesting!
Thank you for your tests and investigation - I'll try that right away.

@amadeo-alex
Copy link
Author

amadeo-alex commented Apr 4, 2024

You seem to be quite correct (it started at 12MB):
4OD52h
This kind of behaviour is very interesting and confusing at the same time :D

@morphx666
Copy link
Owner

morphx666 commented Apr 4, 2024

Consider this code:

internal class Program {
    private static void Main(string[] args) {
        Task.Run(async () => {
            while(true) {
                await Task.Delay(10);
            }
        }).Wait();
    }
}

I just asked CoPilot why this code is leaking memory... the answer is just what one would've expected:

The code you've provided is a simple infinite loop that waits for 10 milliseconds on each iteration. It doesn't seem to be leaking memory directly, as it doesn't allocate any unmanaged resources or create any objects that aren't being cleaned up.

However, if this code is part of a larger program, it could potentially contribute to a memory leak if it's preventing other tasks from completing or if it's keeping some objects alive longer than necessary.

For example, if this loop is running on the main thread and there are other tasks that are waiting to run on the main thread, those tasks won't be able to complete until this loop finishes. If those tasks are holding onto some resources, those resources won't be freed until the tasks complete, which could lead to a memory leak.

Also, if there are any local variables in the Main method that are being captured by the lambda expression, those variables will be kept alive for as long as the loop is running, which could also lead to a memory leak.

Without more context, it's hard to say for sure why your program might be leaking memory. You might want to use a memory profiler to get a better idea of what's going on.

However, it got me thinking: what if we are blocking some internal dotnet process?
After all, the loop lives inside the main program thread/loop.

So, changing the code slightly, running the infinite loop on a separate thread seems to solve the problem:

internal class Program {
    private static void Main(string[] args) {
        Thread t = new(() => {
            while(true) {
                Thread.Sleep(10);
            }
        });
        t.Start();
    }
}

Another thing is that running this in VS still presents the apparent memory leak. But running it with dotnet run -c Release doesn't, so I guess VS is also to blame for this behavior.

@amadeo-alex
Copy link
Author

amadeo-alex commented Apr 4, 2024

Your observations seems to be again quite correct + looks like abusing GC a little bit causes the memory to be constant (started at 16):
oSPQua

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants