[Qt-jambi-interest] Tracking down a problem in native-code

[Gunnar Sletta]

Tom Schindl wrote:
> Hi,
> 
> Well I now found how I can make my example not crash when I modify my
> remove-method from:
> 
>> 	public void remove(ModelElement element) {
>> 		System.err.println("Removing: " + element);
>> 		QListWidgetItem item = findItem(element);
>> 		if( item != null ) {
>> 			((QListWidgetItemImpl)item).data = null;
>> 			listWidget.removeItemWidget(item);
>> 		}
>> 	}

removeItemWidget, removes a widget set on the item, not the item itself. 
To remove an item from a list you need to use takeItem(). In this 
scenario you haven't actually changed the listwidgets itemset at all.

> to
> 
>> 	public void remove(ModelElement element) {
>> 		System.err.println("Removing: " + element);
>> 		QListWidgetItem item = findItem(element);
>> 		if( item != null ) {
>> 			((QListWidgetItemImpl)item).data = null;
>> 			item.dispose();
>> 		}
>> 	}
> 
> everything works. Looks like a the garbage collection is to working
> appropriately. Is 2nd snippet correct, Gunnar?

When you explicitly call dispose() you will call the destructor of the 
item, which in turn will remove the item from the list. The code is 
certainly one correct way of doing it. The item will be deleted on the 
gui thread.

> If yes this pops up a
> question should I not rely on Java-GC and dispose QObject's myself all
> the time to ensure a valid state?

No, you shouldn't. dispose() is an explicit delete and doesn't take into 
account other potential pointers to the same data. For QObjects we have 
a good chance of catching these things because QObjects have a deletion 
notification on the C++ side, but for non-QObjects, like 
QListWidgetItem, there could be scenarious where catching this is out of 
our control.

> I see this from the point of a library developer - What would you
> suggest - should i dispose things explicitly when I don't need them any
> more? What's better from a performance point of view, I guess dispose,
> right because then the native resource is released immediately.

For images I always dispose immediately, because they are expensive. For 
everything else, I normally rely on GC, because other objects are 
usually small enough to not make a "big" difference...

---

But... Going back to the actual problem, the native crash... The problem 
is caused by the fact that we, when we mapped the API for QListWidget, 
did the same assumption you did, namely that removeItemWidget() removed 
the item. Under this assumption we de-reffed the instance, allowing 
garbage collection for the item even though it was still referenced by 
the QListWidget. GC would kick in and delete the item and if the GUI 
thread accessed the item at this time, this would have caused the crash.

If you either use takeItem() or explicit dispose() this will not happen.

Thank you a lot for finding this!

best regards,
Gunnar