Pass back QImage from thread repeatedly, deletes, proper way

[Linwood]

- create the QImage as a stack variable (not a pointer variable) in the thread class

(My C++ programming days date to about 1999, so there's a whole lot of refreshing memory going on, and not the virtual kind, so this may be digging my hole deeper still...).

OK, but...

The Mandelbrot example is not allocating on the stack either. It does:

QImage image(resultSize, QImage::Format_RGB32);

QImage image(resultSize, QImage::Format_RGB32);

To copy to clipboard, switch view to plain text mode 

inside the run(), and then

emit renderedImage(image, scaleFactor);

emit renderedImage(image, scaleFactor);

To copy to clipboard, switch view to plain text mode 

(With the renderedImage() declared as passing the QImage as &image)

and then in the main thread uses it directly:

pixmap = QPixmap::fromImage(image);

pixmap = QPixmap::fromImage(image);

To copy to clipboard, switch view to plain text mode 

This is your memory leak. Your signal passes a reference to the QImage back to the main thread, not a pointer, and this new allocation never gets deleted.

I get why that would leak, but not why I can't delete it. If I'm passing a reference to the QImage, then deleting against that - why does it crash (and why only if it is passed up through the tread and not by a direct call)? Isn't the reference to the QImage I passed out a reference that preserves it from automatic deletion, and shouldn't it be available for a manual delete?

As I mentioned in the second posting I found a way to do it more efficiently by just letting it (suitably protected by mutex) update the main thread's data structure directly from the thread, and just emit a signal to say it's ready. But I remain a bit confused why the program above doesn't allow the delete without crashing?

[d_stranz]

QImage image(resultSize, QImage::Format_RGB32);

Sorry, but this is an allocation on the stack. Dig into your C++ archive a bit deeper. That line of code constructs a QImage within the scope of the function where it is defined. That instance is created on the stack. When the function exits, the stack unwinds, the instance goes out of scope, and is deleted.

emit renderedImage(image, scaleFactor);

This is just Qt "syntactic sugar". A Qt signal is nothing more than a C++ method call. It is synchronous and is handled by your slot (which should make a copy of the image content, not the image address) prior to it going out of scope.

If I'm passing a reference to the QImage, then deleting against that - why does it crash

You may be passing a reference to the QImage (via *image), but then in your main thread, you are assigning the address of that reference (i.e. a pointer to the QImage created in and *owned by* your thread) to your pointer variable in the main thread. So long as the instance owned by the render thread is alive and in scope, you can dereference that pointer in your main thread (via operator->()), maybe safely, maybe not - it depends what the thread is doing to that instance. operator->() isn't protected by a mutex.

But you can't call operator delete() on it in the main thread because the memory it points to isn't owned by your main thread. The only place you can delete it is in the render thread, and if you do that, then the address you have copied into the pointer variable in your main thread will no longer be valid and the main thread will crash as soon as it tries to dereference the pointer.

As I mentioned in the second posting I found a way to do it more efficiently by just letting it (suitably protected by mutex) update the main thread's data structure directly from the thread

Which may work, so long as your main thread isn't doing anything to that QImage instance. As I said, the safest thing is to follow the Mandelbrot example exactly and pass a reference to a stack-based QImage owned by the thread, then make a copy of that QImage into a QImage variable in your main thread.

If you need to keep more than one QImage instance around in your main thread, you can put them into a QVector< QImage > (in which the QImage instances are stack based), or allocate new QImage instances on the heap (using QImage * pImage = new QImage( imageReferenceFromSignal )) and pushing those onto a QVector< QImage * >. In the latter case, you will need to manually delete these QImage instances yourself when you remove them from the QVector; in the former case, QVector::erase(), clear(), and other manipulators will result in the stack-based instance going out of scope and automatically being destroyed.

[Linwood]

Sorry, but this is an allocation on the stack. Dig into your C++ archive a bit deeper.
...

Indeed. Too many years in between, some pollution from C# added to the mix. I need to read through this again and do a bit of research into the "syntactical sugar" in particular. I'm still puzzled a bit by how the actual threading played a role, I do want to find that discussion again about queued vs direct connections and how objects are passed. At best I'm confused by it (and how it affects holding a reference to an object and whether that allows it to delete an object out from under me differently when threaded).

Thank you for taking the time to provide some pointers (no pun intended).

I think I have it working with a suitably synchronized thread just updating a cache in the main thread with images when needed, primarily as I think it may be more efficient (if nothing else the copies are made in parallel). And it ran fairly nicely on the Pi in some brief testing, and high volume testing on a faster system showed no leaks.

Thanks again.

Linwood