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

List:       kfm-devel
Subject:    Re: FW: Bug#1812: KFM memory leak: updated patch for khtmlw.
From:       "Bjarni R. Einarsson" <bre () netverjar ! is>
Date:       1999-08-31 16:16:28
[Download RAW message or body]

On Tue, Aug 31, 1999 at 05:53:30PM +0200, Waldo Bastian wrote:
> On Tue, 31 Aug 1999, you wrote:
> > Sorry if this doesn't thread correctly, I'm pasting from
> > http://www.lists.kde.org/, since I'm not actually subscribed to kfm-devel
> > (I'm just a power user, not a KDE developer). :-)
> 
> No problem. Do you want CC's?

Yes please.  That makes quoting much easier. :-)

I understand your resistance to my patch - it's a bit bigger than you'd
like, right before release time, and you aren't sure what it is fixing.  
You want to know which is doing what, I guess.  

So... my patch explained:

The patch implements the following changes.

	1. pCache cleanup code
	2. cosmetics
	3. A GIF/Gif98a typo fixed	
	4. The movieCache one-liner.
	5. Minor paranoia.

1. Consists of the following changes:

    a) A global definition (PCACHESIZE) replacing the constant 503.

    b) Use of pCache->setAutoDelete(TRUE) where appropriate.
       A destructor for the HTMLCachedImage type, which deletes the pixmap.
       Brute force cache-flushing, via pCache->count() and pCache->clear()

    c) The "cached" boolean variable in the HTMLImage type is ignored,
       destroying the pixmap whether it is cached or not.  This is a good
       thing, because the pixmap is already implicitly shared by Qt, and
       /must/ always be deleted by this destructor, for b) to work.
       This means the "cached" variable isn't really used.

    d) I added checks of whether pixmap was actually unassigned before
       calling pixmap = new QPixmap.

I think that if you're correct that the pCache is never used on web pages,
that d) or c) might be the one-liner you are looking for.  

But.. I feel compelled to harp on this some more - how can you be sure that
everybody uses cacheImage() the right way?  The changes in b) are very
straightforward, and should cause no adverse effects other than a minor
performance hit - if that.  cacheImage() is available to anyone using the
HTML widget, so fixing this is protecting us from abuse by future KDE 1.1.x
compatible programs which may not "do the right thing", no matter what kfm
is doing today.


2. Is just in the way.	

3. Makes handling of GIF images consistant throughout the program, only
   implementing movie-related stuff on GIF89a images.
   
4. The one-liner everyone seems happy with. :-)

5. I changed a "--refcount == 0" comparison to "--refcount <= 0", just
   to be on the safe side.  It probably doesn't help, probably doesn't
   matter, but it made me feel better.


Does this help? :-)

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

                Blasphemer, Blasphemer, Blasphemer!

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