KCategoryDrawer and memory leak...

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

KCategoryDrawer and memory leak...

Gilles Caulier-4
Hi all,

Valgrind crying again on my computer with this trace :

==16673== 3,152 (12 direct, 3,140 indirect) bytes in 1 blocks are
definitely lost in loss record 6,503 of 6,663
==16673==    at 0x40244F0: operator new(unsigned int) (vg_replace_malloc.c:214)
==16673==    by 0x82B6F71:
Digikam::DigikamImageDelegate::DigikamImageDelegate(Digikam::ImageCategorizedView*)
(digikamimagedelegate.cpp:63)
==16673==    by 0x82B4611:
Digikam::DigikamImageView::DigikamImageView(QWidget*)
(digikamimageview.cpp:81)
==16673==    by 0x82883A4:
Digikam::AlbumWidgetStack::AlbumWidgetStack(QWidget*)
(albumwidgetstack.cpp:96)
==16673==    by 0x82BADA1: Digikam::DigikamView::DigikamView(QWidget*,
Digikam::DigikamModelCollection*) (digikamview.cpp:168)
==16673==    by 0x829AD32: Digikam::DigikamApp::setupView() (digikamapp.cpp:490)
==16673==    by 0x82998A0: Digikam::DigikamApp::DigikamApp()
(digikamapp.cpp:229)
==16673==    by 0x83863D1: main (main.cpp:172)

If i look in code, i can see that memory leak is relevant of this call

    d->categoryDrawer = new ImageCategoryDrawer(parent);

ImageCategoryDrawer is based on KCategoryDrawer. on KDE4 api doc we
can see that KCategoryDrawer is not based on QObject :

http://api.kde.org/4.x-api/kdelibs-apidocs/kdeui/html/classKCategoryDrawer.html

... and instance is not delete in destructor. Also, KCategoryDrawer is
deprecated. KCategoryDrawerV3 must be used instead.

http://api.kde.org/4.x-api/kdelibs-apidocs/kdeui/html/classKCategoryDrawerV3.html

I'm right or not ?

Gilles Caulier
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel
Reply | Threaded
Open this post in threaded view
|

Re: KCategoryDrawer and memory leak...

Gilles Caulier-4
Some precision :

KCategoryDrawer is an old class.

KCategoryDrawerV2 exist since KDE 4.4. I need to use it under Mandriva
2010.1 which use KDE 4.4.3, and it compile fine.

KCategoryDrawerV3 exist since KDE 4.5. Why not to use preprocessor
rules to choose right parent class depending of KDE version ?

Gilles

2010/11/3 Gilles Caulier <[hidden email]>:

> Hi all,
>
> Valgrind crying again on my computer with this trace :
>
> ==16673== 3,152 (12 direct, 3,140 indirect) bytes in 1 blocks are
> definitely lost in loss record 6,503 of 6,663
> ==16673==    at 0x40244F0: operator new(unsigned int) (vg_replace_malloc.c:214)
> ==16673==    by 0x82B6F71:
> Digikam::DigikamImageDelegate::DigikamImageDelegate(Digikam::ImageCategorizedView*)
> (digikamimagedelegate.cpp:63)
> ==16673==    by 0x82B4611:
> Digikam::DigikamImageView::DigikamImageView(QWidget*)
> (digikamimageview.cpp:81)
> ==16673==    by 0x82883A4:
> Digikam::AlbumWidgetStack::AlbumWidgetStack(QWidget*)
> (albumwidgetstack.cpp:96)
> ==16673==    by 0x82BADA1: Digikam::DigikamView::DigikamView(QWidget*,
> Digikam::DigikamModelCollection*) (digikamview.cpp:168)
> ==16673==    by 0x829AD32: Digikam::DigikamApp::setupView() (digikamapp.cpp:490)
> ==16673==    by 0x82998A0: Digikam::DigikamApp::DigikamApp()
> (digikamapp.cpp:229)
> ==16673==    by 0x83863D1: main (main.cpp:172)
>
> If i look in code, i can see that memory leak is relevant of this call
>
>    d->categoryDrawer = new ImageCategoryDrawer(parent);
>
> ImageCategoryDrawer is based on KCategoryDrawer. on KDE4 api doc we
> can see that KCategoryDrawer is not based on QObject :
>
> http://api.kde.org/4.x-api/kdelibs-apidocs/kdeui/html/classKCategoryDrawer.html
>
> ... and instance is not delete in destructor. Also, KCategoryDrawer is
> deprecated. KCategoryDrawerV3 must be used instead.
>
> http://api.kde.org/4.x-api/kdelibs-apidocs/kdeui/html/classKCategoryDrawerV3.html
>
> I'm right or not ?
>
> Gilles Caulier
>
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel
Reply | Threaded
Open this post in threaded view
|

Re: KCategoryDrawer and memory leak...

Marcel Wiesweg
In reply to this post by Gilles Caulier-4

> ==16673== 3,152 (12 direct, 3,140 indirect) bytes in 1 blocks are
> definitely lost in loss record 6,503 of 6,663
> ==16673==    at 0x40244F0: operator new(unsigned int)
> (vg_replace_malloc.c:214) ==16673==    by 0x82B6F71:
> Digikam::DigikamImageDelegate::DigikamImageDelegate(Digikam::ImageCategoriz
> edView*) (digikamimagedelegate.cpp:63)

See destructor ~ImageDelegate.
It's a leak, but the deletion crashes (230515) for a lot of people, never for
me, and I dont know why.

KCategoryDrawer (V1) works just fine for us, if just for the compiler
warnings, we can use V2 or V3 or whatever.

Marcel
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel
Reply | Threaded
Open this post in threaded view
|

Re: KCategoryDrawer and memory leak...

Gilles Caulier-4
Ok, i remember

which KDE4 version you use ?

Gilles Caulier

2010/11/3 Marcel Wiesweg <[hidden email]>:

>
>> ==16673== 3,152 (12 direct, 3,140 indirect) bytes in 1 blocks are
>> definitely lost in loss record 6,503 of 6,663
>> ==16673==    at 0x40244F0: operator new(unsigned int)
>> (vg_replace_malloc.c:214) ==16673==    by 0x82B6F71:
>> Digikam::DigikamImageDelegate::DigikamImageDelegate(Digikam::ImageCategoriz
>> edView*) (digikamimagedelegate.cpp:63)
>
> See destructor ~ImageDelegate.
> It's a leak, but the deletion crashes (230515) for a lot of people, never for
> me, and I dont know why.
>
> KCategoryDrawer (V1) works just fine for us, if just for the compiler
> warnings, we can use V2 or V3 or whatever.
>
> Marcel
> _______________________________________________
> Digikam-devel mailing list
> [hidden email]
> https://mail.kde.org/mailman/listinfo/digikam-devel
>
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel
Reply | Threaded
Open this post in threaded view
|

Re: KCategoryDrawer and memory leak...

Gilles Caulier-4
In all case, KDE API said that KCategoryDrawer V1 is obsolete and unmaintained.

At least V2 need to be used instead, or better V3.

I propose to patch code to use V1 for old KDE < 4.4, V2 for KDE 4.4.x,
V3 for KDE > 4.5

What do you think about ?

Gilles Caulier

2010/11/6 Gilles Caulier <[hidden email]>:

> Ok, i remember
>
> which KDE4 version you use ?
>
> Gilles Caulier
>
> 2010/11/3 Marcel Wiesweg <[hidden email]>:
>>
>>> ==16673== 3,152 (12 direct, 3,140 indirect) bytes in 1 blocks are
>>> definitely lost in loss record 6,503 of 6,663
>>> ==16673==    at 0x40244F0: operator new(unsigned int)
>>> (vg_replace_malloc.c:214) ==16673==    by 0x82B6F71:
>>> Digikam::DigikamImageDelegate::DigikamImageDelegate(Digikam::ImageCategoriz
>>> edView*) (digikamimagedelegate.cpp:63)
>>
>> See destructor ~ImageDelegate.
>> It's a leak, but the deletion crashes (230515) for a lot of people, never for
>> me, and I dont know why.
>>
>> KCategoryDrawer (V1) works just fine for us, if just for the compiler
>> warnings, we can use V2 or V3 or whatever.
>>
>> Marcel
>> _______________________________________________
>> Digikam-devel mailing list
>> [hidden email]
>> https://mail.kde.org/mailman/listinfo/digikam-devel
>>
>
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel
Reply | Threaded
Open this post in threaded view
|

Re: KCategoryDrawer and memory leak...

Martin Klapetek
On Sat, Nov 6, 2010 at 15:01, Gilles Caulier <[hidden email]> wrote:
In all case, KDE API said that KCategoryDrawer V1 is obsolete and unmaintained.

At least V2 need to be used instead, or better V3.

I propose to patch code to use V1 for old KDE < 4.4, V2 for KDE 4.4.x,
V3 for KDE > 4.5

What do you think about ?

Well since the newest digiKam needs at least KDE 4.5 libs (kdegraphics), I'd say it's useless to use add this, because you'll always need 4.5, no?  I mean you won't compile it without kdegraphics-4.5 installed and you won't install this package without the rest of KDE 4.5...

Marty

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel
Reply | Threaded
Open this post in threaded view
|

Re: KCategoryDrawer and memory leak...

Gilles Caulier-4
Marcel,

Why we use Q_DECLARE_PRIVATE() macro under imagedelegate.h ?

This macro is used only internally in Qt4 and is not documented :

http://www.qtcentre.org/threads/14928-Q_declare_private%28%29

Same for Q_D() macro of course used in imagedelegate.cpp...

Gilles Caulier

2010/11/6 Martin Klapetek <[hidden email]>:

> On Sat, Nov 6, 2010 at 15:01, Gilles Caulier <[hidden email]>
> wrote:
>>
>> In all case, KDE API said that KCategoryDrawer V1 is obsolete and
>> unmaintained.
>>
>> At least V2 need to be used instead, or better V3.
>>
>> I propose to patch code to use V1 for old KDE < 4.4, V2 for KDE 4.4.x,
>> V3 for KDE > 4.5
>>
>> What do you think about ?
>
> Well since the newest digiKam needs at least KDE 4.5 libs (kdegraphics), I'd
> say it's useless to use add this, because you'll always need 4.5, no?  I
> mean you won't compile it without kdegraphics-4.5 installed and you won't
> install this package without the rest of KDE 4.5...
> Marty
> _______________________________________________
> Digikam-devel mailing list
> [hidden email]
> https://mail.kde.org/mailman/listinfo/digikam-devel
>
>
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel
Reply | Threaded
Open this post in threaded view
|

Re: KCategoryDrawer and memory leak...

Marcel Wiesweg

> Marcel,
>
> Why we use Q_DECLARE_PRIVATE() macro under imagedelegate.h ?
>
> This macro is used only internally in Qt4 and is not documented :
>
> http://www.qtcentre.org/threads/14928-Q_declare_private%28%29
>
> Same for Q_D() macro of course used in imagedelegate.cpp...

Well we need the exact same functionality as this macro provides - syntactic
sugar for a d-pointer shared by the subclass - so I used it and it works.
As you see it's used in nearly any Qt class, so I doubt they'll change it. If
they do, we can define it ourselves.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel