ofxThreadedImageLoader and for-loop introduces "pointer being freed was not allocated"

bugs, errors
#1

Hey,
I currently try to do a project where I need to load many (small) images, around 10k in around 5 minutes - so I can’t fit them into memory at startup but need to (re-)load them during runtime - I thought that ofxThreadedImageLoader would be the proper tool for the job (yet I am still unsure as it seems difficult to figure out when an image is indeed loaded?)

Nonetheless, I tried to use it and I ran into a strange error when using this loader. Lets assume during setup I call this block of code

        for(int i=0; i<2; i++) {
            ofLogError(ofToString(dir.getPath(i)));
            ofImage image = ofImage();
            loadedImages.push_back(image);

        }
        imageLoader.loadFromDisk(loadedImages[0], dir.getPath(0));
        imageLoader.loadFromDisk(loadedImages[1], dir.getPath(1));

which works fine but is a bit “verbose” when scaling up to 100 images. When I now try to put it into a for loop I get an error?

        for(int i=0; i<2; i++) {
            ofLogError(ofToString(dir.getPath(i)));
            ofImage image = ofImage();
            loadedImages.push_back(image);
            imageLoader.loadFromDisk(loadedImages[i], dir.getPath(i));
        }

where the error is

2eyes-ofDebug(8997,0x7000004f2000) malloc: *** error for object 0x11: pointer being freed was not allocated
2eyes-ofDebug(8997,0x7000004f2000) malloc: *** set a breakpoint in malloc_error_break to debug
2eyes-ofDebug(8997,0x7000004f2000) malloc: *** error for object 0x11: pointer being freed was not allocated

where ofxThreadedImageLoader imageLoader and std::vector<ofImage> loadedImages;.

I am still not fully confident with pointers and references, but this seems like a strange thing to happen? Shouldn’t both statements be quite the same?

#2

I got a hint that this is caused by an reallocation of memory which happens when pushing into a vector - this could be fixed by calling loadedImages.reserve(100); to allocate the memory prior to the loop :slight_smile:

#3

hello – as you noted the reserve() call will prevent the crash (and it’s actually what is used in the example), but it is a “risky” pattern. i wanted to take the occasion to raise awareness of the real problem:

what actually creates the crash stems from the fact that the calls to loadFromDisk() immediately starts the thread working, so when you are doing the second iteration of the loop the thread is already in the background processing things. so you have the main thread writing to the vector while the loader thread reads from it.

there are many things that can go wrong from there, such as that vector is not thread-safe for writing; see c++ - std::vector, thread-safety, multi-threading - Stack Overflow

why reserve() solves the problem is that the allocation is done beforehand, hence the push_back()s are not affecting the vector in the same manner and somehow the threads cohabit. but it’s a gray area.

for instance you could also “fix” the problem by iterating once to create all the objects, and another loop for the loadFromDisk() calls, so you don’t have direct concurrency between the memory alloc and the thread – we could say things are “stabilized” before starting the threaded stuff.

for your second question, you can check image.isAllocated() – it should return 1 only when the image is ready for use.

1 Like
#4

Hey @burton, this is a wonderful explanation! So would resizing the vector first be a safe way to do it? Or does resize() more or less do the same as reserve()?

std::vector<ofImage> loadedImages;
loadedImages.resize(100);
 for(int i=0; i<2; i++) {
            ofLogError(ofToString(dir.getPath(i)));
            imageLoader.loadFromDisk(loadedImages[i], dir.getPath(i));
        }

I’ve always kinda wondered if empty objects like ofImage pose a problem when used this way with resize(), because they’re unallocated. It seems like it’s always worked OK though. Any thoughts are much appreciated!

#5

resize() is different as it actually grows it’s size by initializing the grown members, so they are not only allocated but default-constructed.

reserve() merely lays out the memory area for the vector (hence in the case above the memory is not moving around as the other thread picks into it), but the vector is still size 0, and needs the push_back(ofImage()) to actually create the elements.

NB the threadedImageLoaderExample actually uses resize(): openFrameworks/ofApp.cpp at 67e0b87c72539b1473eafa3031f70569c3273e4b · openframeworks/openFrameworks · GitHub

in the context of typical OF apps it’s reasonably safe as things are generally statically organized in setup() and stuff lasts the lifetime of the app, but if you if you have truly dynamic allocated things moving across threads, std::vector requires care. (there are thread-safe implementation of vector-like containers such as concurrent_vector).