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

List:       kde-bugs-dist
Subject:    Bug#1812: KFM memory leak: Further diagnosis, and a patch for khtmlw
From:       "Bjarni R. Einarsson" <bre () netverjar ! is>
Date:       1999-08-30 12:40:07
[Download RAW message or body]

Hi,

I spent the weekend banging on KDE 1.1.2pre3, looking for the cause
of the memory leaks I'm experiencing.

(Martin:  see http://bugs.kde.org/db/18/1812.html)

Using kfm-1.1.2pre with my latest patch, I now see the following 
behavior:

	1) Reloading pages with only text on them causes no memory
	   usage increase.

	2) Reloading pages with images on them /sometimes/ causes
	   memory usage to increase.  Most of it in the X server,
	   some in kfm itself.  This also applies to pages browsed
	   using the file manager as well (HTML view selected).

	3) Flipping back and forth between previously visited pages
	   causes no memory increase, unless the page has to be
	   reloaded (it's not in cache).  This needs to be tested 
	   better - I didn't have a long enough history to be sure.

The X11 memory used by my patched version of 1.1.2 is measurably
better than it was under KDE 1.1.1's kfm - but the difference is
very, very small.

This implies to me that either my patch doesn't work (but I think
it does :-) or that there is still a memory leak, not in the HTML
widget's pixmap caching code, but elsewhere.

The shared nature of QPixmap means that the same pixmap can be
"leaked" in multiple places - any instance of QPixmap that
references the shared memory in the X server can potentially become
orphaned and keep the memory from getting freed.

It is a Qt 1.4x bug that this shared memory is not released when
the application exits.  Sloppy...

My patch is included below - please test it, pass it on and tell me
what you think. :-) If it looks OK to you guys, I really think it
should be included for the 1.1.2 release.

I'm going to continue my efforts to find leaks, and will probably be
studying KFM's html cache next.  This is my first foray into the KDE
code, so any and all help would be appreciated.

-- 
Bjarni R. Einarsson                           PGP: 02764305, B7A3AB89
 bre@netverjar.is           -><-           http://www.mmedia.is/~bre/

              These questions and answers are false.


["khtmlw.memleak.patch.4" (text/plain)]

Common subdirectories: khtmlw.org/CVS and khtmlw/CVS
diff -u khtmlw.org/htmlobj.cpp khtmlw/htmlobj.cpp
--- khtmlw.org/htmlobj.cpp	Mon Jan 18 10:57:54 1999
+++ khtmlw/htmlobj.cpp	Sat Aug 28 15:05:18 1999
@@ -50,6 +50,12 @@
 QDict<HTMLCachedImage>* HTMLImage::pCache = 0L;
 int HTMLObject::objCount = 0;
 
+// Testing implied that this number was just fine - impact on
+// performance was negligable.  Conserving memory is good (tm).
+// It would be best if this could be tuned via. some advanced
+// configuration dialog.
+#define PCACHESIZE 73
+
 //-----------------------------------------------------------------------------
 
 HTMLObject::HTMLObject()
@@ -1239,12 +1245,19 @@
 
 //-----------------------------------------------------------------------------
 
+
 HTMLCachedImage::HTMLCachedImage( const char *_filename )
 {
     pixmap = 0;
     filename = _filename;
 }
 
+HTMLCachedImage::~HTMLCachedImage( )
+{
+    if ( pixmap ) delete pixmap;
+    pixmap = 0;
+}
+
 QPixmap* HTMLCachedImage::getPixmap()
 {
     if ( !pixmap )
@@ -1262,7 +1275,8 @@
 	// yet been initialized. Better be careful.
 	if( !pCache )
 	{
-		pCache = new QDict<HTMLCachedImage>( 503, true, false );
+		pCache = new QDict<HTMLCachedImage>( PCACHESIZE, true, false );
+	        pCache->setAutoDelete(TRUE);
 		return 0l;
 	}
 
@@ -1279,10 +1293,16 @@
 {
 	// Since this method is static, it is possible that pCache has not
 	// yet been initialized. Better be careful.
-	if( !pCache )
-		pCache = new QDict<HTMLCachedImage>( 503, true, false );
+	if( !pCache ) {
+		pCache = new QDict<HTMLCachedImage>( PCACHESIZE, true, false );
+	        pCache->setAutoDelete(TRUE);
+	}
 
-	pCache->insert( _filename, new HTMLCachedImage( _filename ) );
+	// Brute force garbage collection.  So we take a 
+	// performance hit - this should plug the leak!
+	if (pCache->count() > PCACHESIZE/2) pCache->clear();
+	
+	pCache->replace( _filename, new HTMLCachedImage( _filename ) );
 }
 
 HTMLImage::HTMLImage( KHTMLWidget *widget, const char *_filename,
@@ -1290,8 +1310,10 @@
 	int _max_width, int _width, int _height, int _percent, int bdr )
     : QObject(), HTMLObject()
 {
-    if ( pCache == 0 )
-	pCache = new QDict<HTMLCachedImage>( 503, true, false );;
+    if ( pCache == 0 ) {
+	pCache = new QDict<HTMLCachedImage>( PCACHESIZE, true, false );;
+        pCache->setAutoDelete(TRUE);
+    }
 
     pixmap = 0;
     movieCache = 0;
@@ -1371,7 +1393,7 @@
     }
 
     // Is the image available ?
-    if ( pixmap == 0 || pixmap->isNull() )
+    if ( !pixmap || pixmap->isNull() )
     {
 	if ( !predefinedWidth && !percent)
 	    width = 32;
@@ -1427,6 +1449,7 @@
     if ( p )
     {
       *pixmap = *p;
+      cached = true;
     }
     else
     {
@@ -1445,7 +1468,7 @@
   }
   
   // Is the image available ?
-  if ( pixmap == 0 || pixmap->isNull() )
+  if ( !pixmap || pixmap->isNull() )
   {
     if ( !predefinedWidth && !percent)
       width = 32;
@@ -1485,11 +1508,11 @@
   }
   else
   {
-    pixmap = new QPixmap();
+    if ( !pixmap ) pixmap = new QPixmap();
     pixmap->loadFromData( _buffer.buffer() );	    
     cached = false;
 
-    if ( pixmap == 0 || pixmap->isNull() )
+    if ( !pixmap || pixmap->isNull() )
       return false;
   }
   
@@ -1554,11 +1577,11 @@
     }
     else
     {
-	pixmap = new QPixmap();
+	if ( !pixmap ) pixmap = new QPixmap();
 	pixmap->load( _filename );	    
 	cached = false;
 
-	if ( pixmap == 0 || pixmap->isNull() )
+	if ( !pixmap || pixmap->isNull() )
 	    return;
 
 	init();
@@ -1596,7 +1619,7 @@
     if ( percent > 0 )
 	max_width = _max_width;
 
-    if ( pixmap == 0 || pixmap->isNull() )
+    if ( !pixmap || pixmap->isNull() )
 	return;
 
     if ( percent > 0 )
@@ -1816,8 +1839,9 @@
     // if ( !imageURL.isEmpty() && !pixmap )
     // htmlWidget->cancelRequestFile( this );
 
-    if ( pixmap && !cached )
+    if ( pixmap )
 	delete pixmap;
+
 #ifdef USE_QMOVIE
     if ( movie )
     {
diff -u khtmlw.org/htmlobj.h khtmlw/htmlobj.h
--- khtmlw.org/htmlobj.h	Sun Dec  6 19:55:48 1998
+++ khtmlw/htmlobj.h	Sat Aug 28 11:55:06 1999
@@ -571,7 +571,7 @@
 {
 public:
     HTMLCachedImage( const char * );
-    virtual ~HTMLCachedImage() { }
+    virtual ~HTMLCachedImage();
 
     QPixmap* getPixmap();
     const char *getFileName() { return filename.data(); }
@@ -669,9 +669,9 @@
     QString imageURL;
     
     KHTMLWidget *htmlWidget;
-    
-    static QDict<HTMLCachedImage>* pCache;
 
+    static QDict<HTMLCachedImage>* pCache;    
+   
     /*
      * Flag telling wether this image was found in the cache
      * If this flag is set, you may not delete the pixmap since the pixmap
diff -u khtmlw.org/htmltable.h khtmlw/htmltable.h
--- khtmlw.org/htmltable.h	Thu Aug 19 12:49:10 1999
+++ khtmlw/htmltable.h	Sun Aug 29 16:24:47 1999
@@ -79,7 +79,7 @@
 	{ HTMLClueV::print(_painter,_obj,_x,_y,_width,_height,_tx,_ty); }
 
     void link() { refcount++; }
-    void unlink() { if (--refcount == 0) delete this; }
+    void unlink() { if (--refcount <= 0) delete this; }
 
 protected:
     int rspan;
Only in khtmlw: htmltable.h~
Common subdirectories: khtmlw.org/test and khtmlw/test


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