FR: Option for assert on audio thread allocation or syscall

We were discussing this on the Audio Programmer Discord today and concluded it would be great if there was an option that could be set via say CMake that was called something like JUCE_ASSERT_ON_AUDIO_THREAD_ALLOCATION which would trigger an assert if any function was called from the audio thread that ultimately resulted in a call to malloc or free. This could probably be implemented in a platform-specific way using malloc hooks. I’m familiar with the mechanism on Linux implemented by GCC, and surely something similar must be possible on macOS and Windows as well. This would be a huge help in finding performance-killing code that makes its way into the audio thread. It’s not always easy to identify because it could be hidden deep in some library code cough JUCE MIDI SysEx cough.

Perhaps as a stretch goal this could be extended to other syscalls like open etc.

I second that. Much needed feature !

2 Likes

Would love this as well, especially for bugs like this:

It seems like Microsoft has a cross platform solution to create a custom malloc:

2 Likes

I second this! Noticed I accidentally did an allocation when value tree redirected was called. Would have been an easy one to miss

I have an allocation hook for Windows, it patches VirtualAlloc in the binary IAT and I use a thread_local flag to enable asserting on allocation.

I think I can override malloc via the runtime linker on macOS to do the same thing.

I can tidy it up if anyone wants to have a look. Not endorsed by JUCE, obviously.

1 Like

I added an initial version of my allocation checker for Mac (and probably Linux). Hoping to add Windows Support in the next couple of days.

This will hook into malloc/free/calloc/realloc/new and delete in a scoped matter.

The usage in CMake is something like that:

add_subdirectory(ScopedMemoryAllocations)
target_link_libraries(MyPlugin PRIVATE AllocationsChecker)

Then in code:

#include <ScopedMemoryAllocations/Allocations.h>

void processBlock()
{
    EA::Allocations::ScopedSetter setter;
    //your code
}
2 Likes

̶A̶n̶n̶o̶y̶i̶n̶g̶l̶y̶,̶ ̶J̶U̶C̶E̶ ̶s̶t̶i̶l̶l̶ ̶u̶s̶e̶s̶ ̶m̶a̶l̶l̶o̶c̶/̶r̶e̶a̶l̶l̶o̶c̶/̶f̶r̶e̶e̶ ̶i̶n̶ ̶s̶o̶m̶e̶ ̶p̶l̶a̶c̶e̶s̶,̶ ̶w̶h̶i̶c̶h̶ ̶t̶h̶i̶s̶ ̶w̶o̶n̶’̶t̶ ̶c̶a̶t̶c̶h̶.̶

Sorry! So it does, this will do it :+1:

Was this a reply to me? My library does catch malloc/calloc and free. Do you have a specific example in JUCE I can experiment with?

We had an allocation detection implementation that worked via malloc hooks integrated in our in-house wrapper for quite some time. While that seemed like a great tool to spot hidden allocations at first the detector turned out to be a source of problems itself.

The main problem is that the malloc hooks are obviously set up for the entire host process, so we need to store the thread ID of the thread currently calling the processBlock callback and then in the malloc hook check if the thread id of the thread calling malloc equals this stored id if any is stored. This becomes a bit more complex if you have multiple instances of your plugin in a test project and use them in a host that uses multithreaded rendering – which is pretty standard behaviour nowadays. Now we don’t only have to keep a single ID but a list of ids of audio threads that might possibly be active at the same time. This should also not be too hard. Now if you develop multiple plug-ins, which all want to access this malloc hook, and you load them in the same process you end up with multiple chained malloc hooks which lead to some difficult to track down crashes – I don’t remember the details anymore, but in the end it lead us to put all that functionality into a shared library all our plugins linked against in order to be sure that there is only one malloc hook in the process. However, then there is Logic, which started to inject its own malloc implementation starting at some version, which again caused strange bugs.

I won’t say that there are no solutions to all this, but in the end we realised that all the overhead the integration of this safety feature caused and the bugs that were introduced didn’t outweigh the benefit.

We also noticed that after approximately 2 years with this feature integrated the team had learned so much about sources of possible hidden allocations that we didn’t introduce any of them anymore in new code anyway.

So my conclusion after this journey is: We gained some interesting insight, but I wouldn’t recommend integrating a feature like this on a framework level.

2 Likes

Check out my implementation (linked above), it’s addressing that exact issue by using a scoped setting that is set not only per thread but also per specific callback (with the thread id). Setting up multiple of those either in different threads or different callbacks on the same thread should not be problematic.

I don’t think you’d have the allocation checker permanently enabled in a plugin (although you could) but use it more as a development tool and during debug builds. At least that’s what I would do. In that case all the nuances of the host threading implementation etc. wouldn’t matter so much as you’d test it with a simple setup just to make sure no allocations happen. I see it more like something you’d use like ASan, which I try to integrate into all projects when possible. ASan rarely catches anything in my projects, but I still like to use it because things can always sneak through.

2 Likes

Yes, this is exactly what I had in mind. I don’t see it as production code, just something that (like ASAN) you run in a debugging environment, or CI. Ideally, this would help flag bugs like the JUCE Sysex bugs, allocations that could happen as a part of callAsync from the audio thread, etc.

Why you lock and do linear searches in an array every audio block instead of storing a static thread_local bool this_thread_can_alloc = true that is set to false at the beginning of the audio block ?

2 Likes

I just didn’t think of this solution but it certainly sounds like a better idea, I’ll try it!

It is important to note that this is a purely debugging tool and NOT meant to be used in production code, similar to thread/address sanitizer (which also ruin performance while you’re using them), or JUCE_ENABLE_REPAINT_DEBUGGING.

So, the performance overhead of the system shouldn’t have any impact on your “real” code (even if it is not very efficient).

I know, but it doesn’t mean we need to be slower just because it’s a debugging tool. Code will be simpler to maintain and still reasonably efficient (depending on the standard librady thread local implementation).

Also you are not replacing operators new with nothrow_t and alignment, which will allocate silently.

My initial reason to storing an array there is to be able to inspect that array with a breakpoint in case you have multiple threads which is more difficult to do with the local static. I still like the idea of adding the thread local one, but I will first need to test if it actually works correctly, which is more important than performance in this case.

Can you explain your suggestion about operator new? I’m not sure I understand.

Well i honestly don’t use assert anymore (at least not directly), i have my own expect function which is able to break into the debugger without crashing the app, so i could continue from there the debugging session instead of aborting. Once i’m in the debugger with the expectation failing, i should be already positioned in the failing thread, no need to inspect a vector of thread::id which are not easily mappable to thread names (at least from my debugging experience with these things).

See the c++17 overloads you are not overriding in [here]. I make use of those via std::pmr types for overly aligned containers to use for simd, so these overloads taking the alignment argument are called instead of the plain ones without it (via polymorphic_allocator / memory_resource).

Also i forgot one important detail, your way of initialising and storing the “original” malloc/calloc/realloc friends is not thread safe, so you might have data races if any of those functions are called from several threads at the same time.

Well, that inspection is more for me (‘developer of the library’) than you. What I was about to add anyway is a way to customize that function so you can inject jassertfalse (which isn’t aborting) there or your own assert function.

Ah, thank you for this very important resource, I will add that shortly.

Yes, this is a very important bug, thank you for reporting. :slight_smile: