dw::Image objects leak their mapKey
This shows up under valgrind as a leak when the HTML includes an <img usemap="...">. Patch attached. Regards, Jeremy Henty
On Wed, Jan 28, 2009 at 07:20:38PM +0000, Jeremy Henty wrote:
This shows up under valgrind as a leak when the HTML includes an <img usemap="...">. Patch attached.
In the second part of the patch, Why not: void Image::setUseMap (ImageMapsList *list, object::Object *key) { mapList = list; + if (mapKey && mapKey != key) + delete mapKey; mapKey = key; }
# HG changeset patch # User Jeremy Henty <onepoint@starurchin.org> # Date 1233170119 0 # Node ID 0f662866abb4ab30f50a2da66432eff7fda8ec7a # Parent ff6f4530c0f4dc880503d73faf5d2d828d577678 [mq]: fix-images-leak-mapkey
diff --git a/dw/image.cc b/dw/image.cc --- a/dw/image.cc +++ b/dw/image.cc @@ -140,6 +140,8 @@ delete altText; if (buffer) buffer->unref (); + if (mapKey) + delete mapKey; }
void Image::sizeRequestImpl (core::Requisition *requisition) @@ -401,7 +403,12 @@ void Image::setUseMap (ImageMapsList *list, object::Object *key) { mapList = list; - mapKey = key; + if (key != mapKey) { + object::Object *oldKey = mapKey; + mapKey = key; + if (oldKey) + delete oldKey; + } }
} // namespace dw
-- Cheers Jorge.- ______________________________________________________________________ This email has been scanned by the MessageLabs Email Security System. For more information please visit http://www.messagelabs.com/email ______________________________________________________________________
On Wed, Jan 28, 2009 at 05:48:20PM -0300, Jorge Arellano Cid wrote:
In the second part of the patch, Why not:
void Image::setUseMap (ImageMapsList *list, object::Object *key) { mapList = list; + if (mapKey && mapKey != key) + delete mapKey; mapKey = key; }
Because I'm ridiculously paranoid about data members pointing to free()-ed values for even a nanosecond. You decide whether Dillo needs to be similarly paranoid/ridiculous. :-) Regards, Jeremy Henty
On Wed, Jan 28, 2009 at 09:25:02PM +0000, Jeremy Henty wrote:
On Wed, Jan 28, 2009 at 05:48:20PM -0300, Jorge Arellano Cid wrote:
In the second part of the patch, Why not:
void Image::setUseMap (ImageMapsList *list, object::Object *key) { mapList = list; + if (mapKey && mapKey != key) + delete mapKey; mapKey = key; }
Because I'm ridiculously paranoid about data members pointing to free()-ed values for even a nanosecond. You decide whether Dillo needs to be similarly paranoid/ridiculous. :-)
Interesting. With multithreading, if mapKey was a shared resource, it could be a problem. In that case a mutex solves the problem, making the operation "atomic". AFAIS, with:
if (key != mapKey) { object::Object *oldKey = mapKey; mapKey = key; if (oldKey) delete oldKey; }
If oldKey was a shared resource, there's still the problem of being preempted in the middle of the destructor (delete oldKey). If another thread uses the object then, BUM! Again, a mutex solves the problem. The simpler version was committed. ;-) -- Cheers Jorge.- ______________________________________________________________________ This email has been scanned by the MessageLabs Email Security System. For more information please visit http://www.messagelabs.com/email ______________________________________________________________________
participants (2)
-
jcid＠dillo.org -
onepoint＠starurchin.org