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

[Tom Schindl]

Hi Gunnar,

Gunnar Sletta schrieb:
> 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.

Ok.

> 
>> 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.

That's what I assumed.

> 
>> 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.

Ok. So I'm better of using takeItem. I guess the same is true for:
- QTableWidget:
  Where I currently use #removeRow but should have used
  #takeItem

- QTreeWidget:
  Where I currently use #removeItemWidget() and
  QTreeWidgetItem#removeChild but should have used #takeTopLevelItem
  and QTreeWidgetItem#takeItem

When I currently completely refresh the List/Table/Tree structure I do a:
- QTreeWidget#clear()
- QTableWidget#clearContents()
- QListWidget#clear()

Is this correct or do I run in the same problems with my remove code?

> 
>> 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...

Ok. Thanks.

> 
> ---
> 
> 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.

Maybe the JavaDoc should clarify this whole situation.

> 
> If you either use takeItem() or explicit dispose() this will not happen.
> 
> Thank you a lot for finding this!
> 

Thanks for your detailed explaination a have a final question to all
those Structured-Controls (List, Table, Tree).

Tom