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

Leaked Objects detected during end of program #10

Closed
steckes opened this issue Dec 17, 2024 · 6 comments
Closed

Leaked Objects detected during end of program #10

steckes opened this issue Dec 17, 2024 · 6 comments

Comments

@steckes
Copy link
Contributor

steckes commented Dec 17, 2024

On my linux machine Juce reports those objects as leaking:

*** Leaked objects detected: 1 instance(s) of class MessageManager
JUCE Assertion failure in juce_LeakedObjectDetector.h:92
JUCE Assertion failure in juce_Singleton.h:50
JUCE Assertion failure in juce_Singleton.h:50

Is there something we could do in the wrapper to prevent this from happening?

@steckes
Copy link
Contributor Author

steckes commented Dec 17, 2024

My bad, I was initializing the object in a global static...

@steckes steckes closed this as completed Dec 17, 2024
@steckes
Copy link
Contributor Author

steckes commented Dec 17, 2024

Okay, I would like to keep the JUCE object away from the user and I can't really work around giving it a static lifetime. In that case drop will never be called an I am leaking memory. One solution that would help me: make juce::shutdown_juce public so I could call it on drop of my structure?

@steckes steckes reopened this Dec 17, 2024
@JamesHallowell
Copy link
Owner

JamesHallowell commented Dec 19, 2024

Hey Stephan!

Okay, I would like to keep the JUCE object away from the user and I can't really work around giving it a static lifetime.

I agree, this is something I've been thinking about recently too.

The original idea was that some of the classes in JUCE require that we've initialised the JUCE "runtime", so I wanted to make it impossible to construct one of these classes before starting the runtime. Thats why JUCE::initialise gives you back the JUCE value, which then gets passed into the constructor of types that require the JUCE runtime.

The trouble with that as you've found is that the JUCE type has a lifetime (because internally a mutex is being used because only one thread at a time can be the JUCE thread). This makes it awkward to use without that 'juce lifetime infecting everything.

One solution that would help me: make juce::shutdown_juce public so I could call it on drop of my structure?

This could work! I think it would need to be an unsafe function because it isn't synchronised internally so only one thread can call it safely, so the caller would be required to uphold that constraint.

Another idea I was playing around with was making the JUCE runtime reference counted so that you get back an owned JUCE value without a lifetime, and when the last reference to it is dropped then it will shutdown the runtime automatically. I've got something sort of working on a branch, I'll push it up so you can see what you think.

Btw I really enjoyed your Rust talk at ADC! I attended a few days online this year as I was going away on holiday for the second half of the conference, otherwise I would've come and said hello in person.

@steckes
Copy link
Contributor Author

steckes commented Dec 19, 2024

Hi James! :)
Thank you for the nice words, I met some colleagues of you at ADC and asked if you are there, but you weren't with them that moment. Maybe we meet next year!

I tried to make it work using an AtomicUsize that counts up always when a new AudioDeviceManager is initialized. When the counter is 0 and you construct the DeviceManager, the init function is called and when the counter goes to 0 in Drop, the shutdown function is called.

But I wasn't sure if it should be allowed to create multiple instances in different threads, so if it shouldn't be allowed my solution is not the best :D

Here is the code: https://github.com/neodsp/cxx-juce

If only one thread should create an audio device manager at a time, maybe it could also just be an atomic flag that indicates someone is using an instance and the other thread needs to wait until the flag is false again or something like that. It could just be a try_create function in AudioDeviceManager, so the caller is responsible for what to do if another thread uses it atm.

@JamesHallowell
Copy link
Owner

But I wasn't sure if it should be allowed to create multiple instances in different threads, so if it shouldn't be allowed my solution is not the best :D

Here is the code: https://github.com/neodsp/cxx-juce

This looks great! Ah yeah, as far as I know only one thread can be the JUCE message thread. I think we may need to make the reference count independent of the AudioDeviceManager as other classes in JUCE also need the message thread to be initialised in order to function correctly.

If only one thread should create an audio device manager at a time, maybe it could also just be an atomic flag that indicates someone is using an instance and the other thread needs to wait until the flag is false again or something like that.

Nice, like this idea! Much simpler than what I originally had. Tried this out here - #11.

@steckes steckes changed the title Leaked Obejcts detected during end of program Leaked Objects detected during end of program Jan 2, 2025
@steckes
Copy link
Contributor Author

steckes commented Jan 3, 2025

Thanks for the update, works great!
Actually I was wondering if the try_initialise function could be public as well, so a second client would have the option to wait until the first one shut down. But let's discuss this somewhere else :)

@steckes steckes closed this as completed Jan 3, 2025
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