[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: got it! (Re: Kicker memory leaks)
From:       Josef Weidendorfer <Josef.Weidendorfer () gmx ! de>
Date:       2002-05-08 22:56:40
[Download RAW message or body]

Hi,

a follow-up:
I looked into the KDirWatch problem with PanelBrowserMenu again,
because the KDirWatch instance should have been deleted even
before my patch, and I found the real problem:
NO PanelBrowserMenu instance is deleted at all, my patch changes
nothing about the memory leak (OK: the menu regeneration are
a lot less now).
Reason: The manual of
 QPopupMenu::insertItem(*,*,QPopupMenu)
that is used for inserting new menus states, that inserted submenus
aren't deleted when clearing the menu! Thus, submenus exist as long
as the parent exists, and as the top parent is kicker itself, the QObject
cleanup happens only at kicker termination; all the submenus which are
regenerated on a KDirWatch event, are NEVER deleted till termination!

Attached patch:
=2D PanelBrowserMenu::slotClear reimplements KPanelMenu::slotClear
  and correctly delete the submenus (Question: should submenu
  deletion handling be done in KPanelMenu, as this deletion bug probably
  exists for the K-Menu too ?!??).
=2D KDirWatch events simply call slotClear; this is much better than
  regenerating menus that aren't visible at all. The menus are always
  regenerated if needed before they are shown (slotAboutToShow calls
  initialize() !)
=2D Menus should NOT be changed when shown. When shown, a change
  is marked in a dirty flag. When the menu is shown the next time,
  the menu is updated before (in initialize()).
=2D Start KDirWatch as late as possible (in initialize()), and remove
  it as early as possible (in slotClear())

This leads to a much saner behaviour now. Note that opening a
quickbrowser on /proc/self or on /tmp while compiling rendered
kicker useless before this change.

Please check and commit to BRANCH and HEAD again :-)

Cheers,
Josef


On Wednesday 08 May 2002 18:27, Matthias Elter wrote:
> On Wednesday 08 May 2002 17:54, Josef Weidendorfer wrote:
> > Can somebody check and apply to BRANCH and HEAD?
> > (Perhaps even for KDE 3.0.1)
>
> Done, thanks.
>
> Matthias

["kicker.diff" (text/x-diff)]

? ~kicker.diff
Index: ui/browser_mnu.cpp
===================================================================
RCS file: /home/kde/kdebase/kicker/ui/browser_mnu.cpp,v
retrieving revision 1.37
diff -u -3 -p -r1.37 browser_mnu.cpp
--- ui/browser_mnu.cpp	2002/05/08 16:27:37	1.37
+++ ui/browser_mnu.cpp	2002/05/08 22:57:47
@@ -54,25 +54,33 @@ PanelBrowserMenu::PanelBrowserMenu(QStri
     : KPanelMenu(path, parent, name)
     , _mimecheckTimer(0)
     , _startid(startid)
+    , _dirty(false)
 {
+    _subMenus.setAutoDelete(true);
     _lastpress = QPoint(-1, -1);
     setAcceptDrops(true); // Should depend on permissions of path.
 
     // we are not interested for dirty events on files inside the
-    // directory (see reinitializeIfNeeded)
+    // directory (see slotClearIfNeeded)
     connect( &_dirWatch, SIGNAL(dirty(const QString&)),
-             this, SLOT(reinitializeIfNeeded(const QString&)) );
+             this, SLOT(slotClearIfNeeded(const QString&)) );
     connect( &_dirWatch, SIGNAL(created(const QString&)),
-             this, SLOT(reinitialize()) );
+             this, SLOT(slotClear()) );
     connect( &_dirWatch, SIGNAL(deleted(const QString&)),
-             this, SLOT(reinitialize()) );
-    _dirWatch.addDir( path );
+             this, SLOT(slotClear()) );
+
+    kdDebug() << "PanelBrowserMenu Constructor " << path << endl;
+}
+
+PanelBrowserMenu::~PanelBrowserMenu()
+{
+    kdDebug() << "PanelBrowserMenu Destructor " << path() << endl;
 }
 
-void PanelBrowserMenu::reinitializeIfNeeded(const QString& p)
+void PanelBrowserMenu::slotClearIfNeeded(const QString& p)
 {
-  if (p == path())
-    reinitialize();
+    if (p == path())
+        slotClear();
 }
 
 
@@ -80,9 +88,24 @@ void PanelBrowserMenu::initialize()
 {
     _lastpress = QPoint(-1, -1);
 
+    // don't change menu if already visible
+    if (isVisible())
+        return;
+
+    if (_dirty) {
+        // directory content changed while menu was visible
+        slotClear();
+        setInitialized(false);
+        _dirty = false;
+    }
+
     if (initialized()) return;
     setInitialized(true);
 
+    // start watching if not already done
+    if (!_dirWatch.contains(path()))
+        _dirWatch.addDir( path() );
+
     // setup icon map
     initIconMap();
 
@@ -254,7 +277,7 @@ void PanelBrowserMenu::initialize()
             ++it;
             if( it.current() ) {
                 insertSeparator();
-                insertItem(CICON("folder_open"), i18n("More..."), new \
PanelBrowserMenu(path(), this, 0, run_id)); +                \
append(CICON("folder_open"), i18n("More..."), new PanelBrowserMenu(path(), this, 0, \
run_id));  }
             break;
         }
@@ -324,6 +347,8 @@ void PanelBrowserMenu::append(const QPix
 
     // insert submenu
     insertItem(pixmap, newTitle, subMenu);
+    // remember submenu for later deletion
+    _subMenus.append(subMenu);
 }
 
 void PanelBrowserMenu::mousePressEvent(QMouseEvent *e)
@@ -437,6 +462,21 @@ void PanelBrowserMenu::slotMimeCheck()
         changeItem(id, CICON(icon), file);
 }
 
+void PanelBrowserMenu::slotClear()
+{
+    // no need to watch any further
+    if (_dirWatch.contains(path()))
+        _dirWatch.removeDir( path() );
+
+    // don't change menu if already visible
+    if (isVisible()) {
+        _dirty = true;
+        return;
+    }
+    KPanelMenu::slotClear();
+    _subMenus.clear(); // deletes submenus
+}
+
 void PanelBrowserMenu::initIconMap()
 {
     if(_icons) return;
@@ -455,3 +495,4 @@ void PanelBrowserMenu::initIconMap()
     _icons->insert("exec", SmallIcon("exec"));
     _icons->insert("chardevice", SmallIcon("chardevice"));
 }
+
Index: ui/browser_mnu.h
===================================================================
RCS file: /home/kde/kdebase/kicker/ui/browser_mnu.h,v
retrieving revision 1.14
diff -u -3 -p -r1.14 browser_mnu.h
--- ui/browser_mnu.h	2002/05/08 16:27:37	1.14
+++ ui/browser_mnu.h	2002/05/08 22:57:47
@@ -25,6 +25,7 @@ CONNECTION WITH THE SOFTWARE OR THE USE 
 #define __browser_mnu_h__
 
 #include <qmap.h>
+#include <qptrlist.h>
 #include <kpanelmenu.h>
 #include <kdirwatch.h>
 
@@ -34,6 +35,7 @@ class PanelBrowserMenu : public KPanelMe
 
 public:
     PanelBrowserMenu(QString path, QWidget *parent = 0, const char *name = 0, int \
startid = 0); +  ~PanelBrowserMenu();
 
     void append(const QPixmap &pixmap, const QString &title, const QString \
                &filename, bool mimecheck);
     void append(const QPixmap &pixmap, const QString &title, PanelBrowserMenu \
*subMenu); @@ -46,14 +48,14 @@ protected slots:
     void slotOpenTerminal();
     void slotOpenFileManager();
     void slotMimeCheck();
-    void reinitializeIfNeeded(const QString&);
+    void slotClearIfNeeded(const QString&);
+    void slotClear();
 
 protected:
     void mousePressEvent(QMouseEvent *);
     void mouseMoveEvent(QMouseEvent *);
     void dropEvent(QDropEvent *ev);
     void dragEnterEvent(QDragEnterEvent *ev);
-
     void initIconMap();
 
     QPoint             _lastpress;
@@ -61,10 +63,12 @@ protected:
     QMap<int, bool>    _mimemap;
     QTimer            *_mimecheckTimer;
     KDirWatch          _dirWatch;
+    QPtrList<PanelBrowserMenu> _subMenus;
 
     int                _startid;
     bool               _showhidden;
     int                _maxentries;
+    bool               _dirty;
 
     static QMap<QString, QPixmap> *_icons;
 };



[prev in list] [next in list] [prev in thread] [next in thread]