Bug 166440

Andy,

It's reproductible into KDE3 code ?

Gilles

no...
That was fixed by me weeks ago.... and it was a different issue... in KDE3 it 
only occurred when removing the last image in Light table...

see http://bugs.kde.org/show_bug.cgi?id=162814#c17

Andi

On Sunday 13 July 2008 13:49:35 Gilles Caulier wrote:
[bugs.kde.org quoted mail]

I can confirm this bug, same here

I guess I found the problem, but I can't explain it myself. I had the same issue when working on the UndoManager before in KDE3: The deconstructor of the ThumbBarItem is never called (could this be a Qt3 bug in general?).
In Qt4 (KDE4) the deconstructor is hit:

http://lxr.kde.org/source/extragear/graphics/digikam/libs/widgets/common/thumbbar.cpp#845

removeItem has been called before and is again called by the BarItem deconstructor. So the item is removed twice. This can also be seen when running in the debugger and watching the call stack.

The KDE3 code is actually the same but the deconstructor is not hit, so the second call for removeItem is never executed.

So there are two solutions: remove the described source code line in the deconstructor (this works, I have tested it) or remove the other function call in the appropriate slot.

This again gives the question why nearly all deconstructors I tested in digiKam KDE3 are never executed... like I said before I've seen the same behavior in other deconstructors like the one from UndoManager and UndoCache (maybe a thread problem?).

Hum,

I have never seen that here. Destructor is always called.

Marcel, Can you reproduce this on your computer ?

Gilles

Well maybe other deconstructors do work, but the one for ThumbBarItem is definitely not called... I set a breakpoint and did other debugging tasks but it is never ever executed. In KDE4 / Qt4 it is and so the image gets removed twice... which breaks the whole lighttable thumbbar after a while.

Created attachment 26366 [details]
before

Here you can see the first call to removeItem right after the slot is called
(pressing CTRL+K to remove image from thumbbar)

Created attachment 26367 [details]
after

And here after the constructor is called... removeItem is called twice... this
is not the case for the KDE3 version, although the code is the same.

I did not look into the KDE3 problem, but for KDE4 I would suggest this:

add a method takeItem to Thumbbar which gets all the code from removeItem, except for the word "delete" in line 689.

Make removeItem only call "delete item", and make the destructor call takeItem.

So you can still remove items with delete, and if removeItem is called at various places we keep it for compatibility. If it is used only in a few places, we could remove it and use delete everywhere.

Marcel,

sounds fine for me...I will try this today... Or have you patched it already?

Andi

SVN commit 837619 by aclemens:

Images get removed twice from the thumbbar view when removing them via keyboard shortcut or context menu. This happened due to the use of the ThumbBarView::removeItem() method in the appropriate slot as well as in the destructor of a ThumbBarItem.
To obtain backward compatibility we move the context of removeItem() into a new method takeItem() and call this new method from within the destructor of a ThumbBarItem, removeItem only calls delete now.
This way we can remove an item "the old way" but also by calling just delete.
Somehow the KDE3 branch is not affected by this because the destructor is never called at all (I don't know why).

CCBUGS:166440

 M  +23 -20    thumbbar.cpp  
 M  +3 -2      thumbbar.h  


WebSVN link: http://websvn.kde.org/?view=rev&revision=837619

I moved the last commit into the KDE3 source, too. But again the destructor is not called. The only difference I found was in the old removeItem() method:

d->itemDict.remove(item->url().url());

In KDE4 we call

d->itemDict.take(item->url().url());

So I used the take method as well, but still the destructor is not called. The only thing that happens now is that the dictionary is cleared after some time, but the images stay in thumbbar and light table.

But since the removing of images from light table is working for KDE3 anyway, I will close this bug now.

Andi,

But, in KDE3 we have a memory leak ? Try to use valgrind to be sure. Look in HACKING file to see how to use valgrind...

Best

Gilles Caulier

Created attachment 26404 [details]
valgrind

This is what I get. Should I also use "--show-reachable=yes"?