Deleting a shared object

[deisik]

What is the canonical way (if there is any) of deleting a shared object, that is, an object shared across an arbitrary number of threads (more than two), to avoid potential heap corruption (use after free)?

For example, in a non-GUI thread I create an object which is then shown via QTableView (GUI) and independently processed in yet another thread. How can I safely dispose of it when it is no longer needed (which is determined programmatically by the first thread where the object originates), and all the other threads should discard it, with the last (first?) thread in the object propagation chain deleting this object?

Right now, the last thread to receive the deletion signal (passed on by the processing thread) deletes the object (in my case, it's GUI). But there might be better (simpler) ways to accomplish this

[deisik]

So I tried to use a QSharedPointer, and it didn't make sense. I have only 3 threads and easily over 1M data objects. In this case specifically, using a QSharedPointer for each of these data objects is a huge overkill incurring as much overhead, both in terms of effort to code and the CPU cycles to run this code – the code to handle the stack of the QSharedPointer pointers themselves (which includes tracking and deleting them)

Instead, I just emit the queued deletion signal from the first thread (where the data object is created) to the second (where the data is actually used, as designed), and the second thread then passes it on after processing to the third (GUI) where the object is deleted in a deterministic, thread-safe manner, with no memory footprint and virtually no extra execution time (compared to keeping track and managing 1M of QSharedPointer instances). Simple, safe and efficient

Indeed, when it becomes a matter of dozens or even hundreds of threads, the QSharedPointer may be a viable and in many cases probably the best solution (I'm far from there yet), but not in the case where the number of threads is small while their interaction simple and well defined (again, by design), with lots of data objects (on the order of many thousands or even millions)

Thereby, I mark this topic as solved (but your comments are welcome anyway)

[SGaist]

Hi,

Looks like the latest KDAB article on QPointer is what you are looking for :-)

[deisik]

@SGaist Okay, I will look into it

[deisik]

As I got it from the article linked, QPointer is not fail-safe as it is not supposed to work across multiple threads:

Doesn’t this code suffer from the problem that weak_ptr works so hard to prevent? Well yes, it does. However, in practice it does not constitute a problem, because we are never supposed to touch QObjects from arbitrary threads — and this includes deleting them

QPointer is for checking whether a pointed-to object has not been destroyed before, but it doesn't guarantee that it is not destroyed by another thread right after or even during the check

[Chris Kawa]

@deisik Use a shared pointer, like QSharedPointer. That's what they're for - the object lives as long as any of the pointers to it live. The object reference counting in QSharedPointer is atomic, so you can use it across threads.

[deisik]

@Chris-Kawa Can it be said that QSharedPointer works as garbage collector for the managed pointer?

[Chris Kawa]

@deisik Not really. Shared pointers are explicit - all holders are responsible for the resource (thus the name "shared") and last person exiting the room turns off the light. Garbage collector would be like a light sensor that turns off the light at some unspecified time after everybody left.

[SimonSchroeder]

A QObject (and anything derived from it) belongs to a thread. It is either the thread it was created in or moved to using moveToThread() explicitly. If I am not mistaken, you need to delete it inside the thread it belongs to. A shared pointer is the way to go (either QSharedPointer or std::shared_ptr). But you should set the deleter to QObject::deleteLater(). deleteLater() will delete the object inside the correct thread. (I have never used QSharedPointer. Does it do this automatically?)

[deisik]

@SimonSchroeder said in Deleting a shared object:

A QObject (and anything derived from it) belongs to a thread. It is either the thread it was created in or moved to using moveToThread() explicitly. If I am not mistaken, you need to delete it inside the thread it belongs to

Well, this is the implicit assumption (which I agree with in general, just in case). However, if you delete the object in the last thread using it (while discarding it in all other threads), it doesn't feel like an inherently wrong thing to me

But you should set the deleter to QObject::deleteLater(). deleteLater() will delete the object inside the correct thread. (I have never used QSharedPointer. Does it do this automatically?)

From what I learned so far, by default it calls the object's default destructor (pardon the pun). But you can specify your own deleter, which (as I see it) can be used for deleting the QSharedPointer object itself (since it is no longer needed then)

[deisik]

@deisik said in Deleting a shared object:

@SimonSchroeder said in Deleting a shared object:
From what I learned so far, by default it calls the object's default destructor (pardon the pun). But you can specify your own deleter, which (as I see it) can be used for deleting the QSharedPointer object itself (since it is no longer needed then)

So the question that remains is, how can I get a pointer to the QSharedPointer object holding (the pointer to) the data object to be destroyed in the deleter function directly (something like sender() called in a slot)?

Or, more generally, should I explicitly delete a QSharedPointer object after it has done its job, or will it take care of itself?

[SGaist]

@deisik it is shown in the documentation of the corresponding constructor.

[deisik]

@SGaist said in Deleting a shared object:

@deisik it is shown in the documentation of the corresponding constructor.

It is the data object referenced by the QSharedPointer object which gets destroyed, not the QSharedPointer object itself (unless it is deleted along with the data object)

What am I missing?

[SGaist]

@deisik you are missing the scope of the QSharedPointer. The custom deleted is call when the QSharedPointer is destroyed provided there are no other QSharedPointer referencing the object.

[deisik]

@SGaist So I should create it on stack if I want it to autodelete, right?

[SGaist]

@deisik yes. Otherwise it's your job to delete the QSharedPointer object at the correct time.

[Chris Kawa]

For multithreaded sharing you can use it like this:

vois SomeClass::someMethod()
{
   QSharedPointer<MyObject> ptrA {new MyObject, &QObject::deleteLater};
   
   QtConcurrent::run([](QSharedPointer<MyObject> ptrB){ /* do something with ptrB */ }, ptrA);
    
   // do something with ptrA
}

In this case the object will be deleted by either ptrA or ptrB, depending on whether someMethod finishes first or the lambda. Of course you can use any other type of threading. QtConcurrent is just an example here.

QObjects should (in general) be deleted on their thread, so you should be using deleteLater for multithreaded case. This is because object events are processed by the event loop of the thread the object belongs to, so deleting it from another thread is unsafe.

Keep in mind that to share an object QSharedPointers need to be copied. Merely creating a new pointer to the same object will not share it and can lead to crashes, e.g.

MyObject* obj = new MyObject();
QSharedPointer<MyObject> ptr1(obj);  // this creates a reference counted pointer
QSharedPointer<MyObject> ptr2(obj); // this creates another, independent reference counted pointer

In this case one of these pointers will delete the object when pointer goes out of scope and the app will crash when the other pointer goes out of scope, because it will try to delete it again.
The correct way to share is like this:

MyObject* obj = new MyObject();
QSharedPointer<MyObject> ptr1(obj);  // this creates a reference counted pointer
QSharedPointer<MyObject> ptr2 = ptr1; // this bumps the existing, shared refcount

[deisik]

@SGaist said in Deleting a shared object:

@deisik yes. Otherwise it's your job to delete the QSharedPointer object at the correct time.

Gotcha

[deisik]

So I tried to use a QSharedPointer, and it didn't make sense. I have only 3 threads and easily over 1M data objects. In this case specifically, using a QSharedPointer for each of these data objects is a huge overkill incurring as much overhead, both in terms of effort to code and the CPU cycles to run this code – the code to handle the stack of the QSharedPointer pointers themselves (which includes tracking and deleting them)

Instead, I just emit the queued deletion signal from the first thread (where the data object is created) to the second (where the data is actually used, as designed), and the second thread then passes it on after processing to the third (GUI) where the object is deleted in a deterministic, thread-safe manner, with no memory footprint and virtually no extra execution time (compared to keeping track and managing 1M of QSharedPointer instances). Simple, safe and efficient

Indeed, when it becomes a matter of dozens or even hundreds of threads, the QSharedPointer may be a viable and in many cases probably the best solution (I'm far from there yet), but not in the case where the number of threads is small while their interaction simple and well defined (again, by design), with lots of data objects (on the order of many thousands or even millions)

Thereby, I mark this topic as solved (but your comments are welcome anyway)

[Chris Kawa]

@deisik I'll be honest. Your conclusions are completely bogus.

using a QSharedPointer for each of these data objects is a huge overkill

How so? It's just a pointer with a refcount.

both in terms of effort to code

Again - what effort - one line to create a pointer (which you do anyway, just with a raw pointer) and one to copy it to the thread.

and the CPU cycles to run the code

What? Copying a QShared pointer is creating a pointer on the stack and atomically bumping a refcount. That's not even noise level of computing.

Instead, I just emit the queued deletion signal

Do you even know how much memory and CPU cycles it takes to emit a cross thread queued signal? At least an order of magnitude more than an atomic refcount, I can assure you.

the object is deleted in a deterministic, thread-safe manner

What if the thread quits before it gets the signal to delete the object? Doesn't sound safe at all. Sounds like memory leak waiting to happen. QSharedPointer is a RAII type. It's deterministic, (really) safe and a lot more performant than passing object around via signals until someone maybe deletes it if they're still around.

Indeed, when it becomes a matter of dozens or even hundreds of threads, the QSharedPointer may be a viable and in many cases probably the best solution, but not in the case where the number of threads is small while their interaction is simple and well defined

That conclusion doesn't make any sense to me. QSharedPointer is basically just a pointer. It's as simple to use as a regular pointer, just that the last one will delete the object. Why do you think it has no use for many objects or few threads is beyond me.

[deisik]

@Chris-Kawa said in Deleting a shared object:

What if the thread quits before it gets the signal to delete the object?

If it quits that way, it means there is something wrong with the implementation (read, the code has bugs), not with the design, because by design it will not. And should it, for some inexplicable reason, actually quit, the ensuing memory leak will be the least of my concerns