Strange bug in juce image caching code

1

Hi all,

I’m having a strange error while running (debugging) my JUCE app,
i’ve been digging and trying for hours and can’t find any solution.

Basically, when I try to compile, then run my app in debug mode (or in release),
My application crashes with the call stack and error as shown in the attached screenshot image.

It seems to want to try to delete some cached images or something,
but i’ve no idea why it would do this.

Here’s a simplified view of my class, as you can see,
the bug occurs during a deep delete of a tree of classes,
and here’s where the error occurs, before it goes into the juce code,
eg, while deleting my std::vector of modules (not juce modules, my own ones).
the vector is properly loaded and all values are OK.

class PipeLineComponent : public WindowComponent
{
public:
PipeLineComponent(PipeLineTreeNode* pltn);

~PipeLineComponent() override {
	for (int32 i = 0; i < pipeLineModules.size(); i++)
		delete pipeLineModules[i];                     < ------ BUG
};

void paint(juce::Graphics&) override;
void resized() override;

PipeLineTreeNode* pipeLineTreeNode;
std::vector<Module*> pipeLineModules;

};

On a side note,
I am using some images that I load from strings in separate C++ file,
converted using my pngtoascii tool for JUCE,
but these are simple class variables, not allocated with pointers,
and this class does not appear anywhere in the debugger’s call stack,
so I am not sure if it applies to the bug or not.

Just FYI, here is a simplified version of the class to show what images I’m using
elsewhere in the application:

class WindowComponent : public juce::Component
{
public:

WindowComponent();
~WindowComponent() {};

void PreLoadUIImages();

...

// Pre-Loaded Images
Image menuBarLeftSide;
Image menuBarCenterSide;
Image menuBarRightSide;
Image windowMaximizeButtonImage;
Image windowPanebackgroundImage;

};

void WindowComponent::PreLoadUIImages()
{
// Pre-load Images
menuBarLeftSide =
ImageFileFormat::loadFrom(eightk_panemenubar_left_png, eightk_panemenubar_left_pngSize);

menuBarCenterSide =
	ImageFileFormat::loadFrom(eightk_panemenubar_center_png, eightk_panemenubar_center_pngSize);

menuBarRightSide =
	ImageFileFormat::loadFrom(eightk_panemenubar_right_png, eightk_panemenubar_right_pngSize);

windowMaximizeButtonImage =
	ImageFileFormat::loadFrom(eightk_panemenubar_maximize_png, eightk_panemenubar_maximize_pngSize);

windowPanebackgroundImage =
	ImageFileFormat::loadFrom(eightk_panemenu_panebackground_png, eightk_panemenu_panebackground_pngSize);

}

void WindowComponent::drawWindowComponents(juce::Graphics& g,
const char* iconData, const int iconDataSize, bool bg_menubar_gradient)
{
// Draw Component’s menubar
if (menubarTop) {
//g.fillRect(0, 0, getWidth(), 24);
g.drawImage(menuBarLeftSide, 0, 0, 4, 28, 0, 0,
menuBarLeftSide.getWidth(),
menuBarLeftSide.getHeight());

	g.drawImage(menuBarCenterSide, 4, 0, getWidth() - 4 - 4, 28, 0, 0,
		menuBarCenterSide.getWidth(),
		menuBarCenterSide.getHeight());

	g.drawImage(menuBarRightSide, getWidth() - 4, 0, 4, 28, 0, 0,
		menuBarRightSide.getWidth(),
		menuBarRightSide.getHeight());
}

...

}

It would be great if someone here can provide some advice as I seem to be stuck at the moment…

Thank you!,
Terrence

bug1
bug11920×919 117 KB

2

Hi there,

Based on the address of the access violation 0xFFFF...FFF, it looks like you could have some memory corruption going on. Maybe something is being double deleted?

It’s difficult to tell from the code snippets what’s going wrong. But a simple recommendation would be to use smart pointers for memory management. One way is to change the type of pipeLineModules from std::vector<Module*> to std::vector<std::unique_ptr<Module>>. Then instead of doing new Module(...), you use std::make_unique<Module>(...).

This would also mean that you don’t need to override the PipeLineComponent destructor to do the manual deletes.

I hope this helps a bit. :slight_smile:

3

Hi,

I have limited experience with these latest C++ enhancements as I haven’t done any serious C++ coding for nearly 10 years now…
I’m used to just using normal pointers with “new” and “delete” etc…
Things have definately changed while I was away :slight_smile: LOL

So I tried to use the unique pointer in my classes, but I hit some brickwalls…
Here’s a very simple example class:

class PipeLineTreeNode : public TreeNode
{
public:
PipeLineTreeNode() {
nodeName = “PIPELINE”;
}

~PipeLineTreeNode() {
	// Delete all child pipeLine Modules
	if (pipeLineModules.size() <= 0) {
		for (int i = 0; i < pipeLineModules.size(); i++) {
			delete pipeLineModules[i];
		}
	}
}

int32 GetNrModules() { return pipeLineModules.size(); }
Module* GetModule(int32 i) { return pipeLineModules[i]; }

// Serialization Methods
XmlElement* GetRecursiveXmlElement();
bool CreateFromXmlElement(XmlElement* xml);

Module* GetModuleFromXML(XmlElement* xml);

protected:
Module* pipeLineModules;

//JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR(PipeLineTreeNode)

};

Here’s my attempt to change it to use the unique pointers:

class PipeLineTreeNode : public TreeNode
{
public:
PipeLineTreeNode() {
nodeName = “PIPELINE”;
}

~PipeLineTreeNode() {
	// Delete all child pipeLine Modules
	if (pipeLineModules.size() <= 0) {
		for (int i = 0; i < pipeLineModules.size(); i++) {
			delete pipeLineModules[i];
		}
	}
}

int32 GetNrModules() { return pipeLineModules.size(); }
std::unique_ptr<Module> GetModule(int32 i) { return pipeLineModules[i]; }

// Serialization Methods
XmlElement* GetRecursiveXmlElement();
bool CreateFromXmlElement(XmlElement* xml);

std::unique_ptr<Module> GetModuleFromXML(XmlElement* xml);

protected:
std::vector <std::unique_ptr> pipeLineModules;

//JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR(PipeLineTreeNode)

};

But it won’t build due to several errors, mainly “delete pipeLineModules[i];” does’nt work.
So, can you explain in more detail how all this works and how I can create a class
that contains a protected std::vector array of Module objects,
allows one to retrieve them with a GetModule(int32 i),
and once the class is destroyed, everything inside should be destroyed too offcourse.

Could you modify my example so everything is clear for me ?
I need to know how everything works before I can start the major work of refactoring all my 100+ C++ and header files of my project…

Do I need to delete anything in my class destructor ? or does the unique ptr automatically do this when the whole class is deleted ?
Also, How do I access std::vector<unique_ptr> array variables like what I am use to doing with "return pipeLineModules[i]; ?

Thanks you so much, it’s highly appreciated…
Terrence

4

Also,

If you look at a method like this one:

Module* PipeLineTreeNode::GetModuleFromXML(XmlElement* xml)
{
Module* outMod = NULL;

if (xml->hasTagName("INPUT_MODULE"))
	outMod = new InputModule();

if (xml->hasTagName("OUTPUT_MODULE"))
	outMod = new OutputModule();

return outMod;

}

If I were to change the first line “Module* outMod = NULL;” to a unique_ptr,
won’t it be deleted at the end of this method/scope,
and the returned value will be invalid ?
How do I handle this ?

Thanks!
Terrence

5

I need to know how everything works before I can start the major work of refactoring all my 100+ C++ and header files of my project…

Sorry, I got the impression you were on a completely fresh project. I don’t think it’s a good idea to rewrite your entire project to use smart pointers. But still they are very good to know and can save you some serious headaches. I would recommend picking up a book on modern C++, there are some really good ones out there. I enjoyed this one: Beautiful C++: 30 Core Guidelines for Writing Clean, Safe, and Fast Code [Book]. There are also great online resources, FluentCpp is one example, and has a whole series on memory management: Smart developers use smart pointers (1/7) - Smart pointers basics - Fluent C++.

Do I need to delete anything in my class destructor ? or does the unique ptr automatically do this when the whole class is deleted ?

The unique_ptr takes care of this for you. In your case this means that you can rid of the destructor entirely.

Also, How do I access std::vector<unique_ptr> array variables like what I am use to doing with "return pipeLineModules[i]; ?

The simplest way to do it would be return pipeLineModules[i].get();. But then you have to be completely sure the returned pointer is not used after PipeLineTreeNode is destroyed. And that goes a bit against the point of moving to smart pointers in the first place.

If I were to change the first line “Module* outMod = NULL;” to a unique_ptr,
won’t it be deleted at the end of this method/scope,
and the returned value will be invalid ?

No, because the unique_ptr has move semantics, see for instance https://www.learncpp.com/cpp-tutorial/stdunique_ptr/ for an explanation.

Coming back to the root of your problem, maybe you should try coming at it from a different angle. One way is to enable the AddressSanitizer: AddressSanitizer (ASan) for Windows with MSVC - C++ Team Blog. This will add extra memory safety checks to your code, so if something is being double freed or otherwise corrupted, you will get detailed information at the point of the problem instead of some random time later.

6

Hey,

Since I’m always working in x64 Debug mode in MSVC,
today, since I wanted to test the speed of something else,
I ran my app in x64 Release mode and it works fine.
I can do File → New, Open, Save, Save as… and Exit, all no problems,
with no memory leaks at all.

Very weird…

Since I will be needing to use debug mode in the future,
I need to get to the root of this.

I’ve done some googling and the Asan is enabled in my Release target (was so by default),
how do I use it ? (eg how does it work, can’t find much about it via google, just how to install/enable on older VS 2019 etc…)

Cheers,
Terrence

7

You simply enable it and start using your program. Address Sanitizer is just like your regular debugger but it’s more restrictive. It complains about everything that might have a chance to access bad memory.
Hopefully the error message you will receive from it will be more useful.