<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<meta http-equiv="Content-Style-Type" content="text/css" />
<meta name="generator" content="pandoc" />
<title></title>
<style type="text/css">code{white-space: pre;}</style>
</head>
<body>
<p>Hi Rémi,</p>
<p>On 2017-03-01 16:16, Rémi Denis-Courmont wrote:</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> On March 1, 2017 10:52:03 AM GMT+02:00, "Filip Roséen" <filip@atch.se> wrote:</code></pre>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code>PictureFlowSoftwareRenderer::PictureFlowSoftwareRenderer():
PictureFlowSoftwareRenderer::~PictureFlowSoftwareRenderer()
{
buffer = QImage();
+
+ for( QHash<QString, QImage*>::const_iterator
+ it = cache.constBegin(); it != cache.constEnd(); ++it )
+ {
+ delete it.value();
+ }
+
cache.clear();
delete blankSurface;
}</code></pre>
</blockquote>
<pre><code> I do not know if the patch is valid or not. But "indirect" leak
implies that the container is leaked. (And so, that a double free
could potentially occur if the container leak is fixed.)</code></pre>
</blockquote>
<ol style="list-style-type: decimal">
<li><p>The container is a direct member of <code>PictureFlowSoftwareRenderer</code>, and as <code>PictureFlowSoftwareRenderer::~PictureFlowSoftwareRenderer()</code> is called, so will the destructor of the container.</p></li>
<li><p>The destructor of <code>QHash<QString, QImage*></code> will not take care of cleaning up after <code>T*</code>.</p></li>
</ol>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> Isn't there a "direct" leak?</code></pre>
</blockquote>
<p>AFAICT, not related to the leak which this patch is set to address, below is the full log (from a session where the current commit is reverted).</p>
<ul>
<li>https://gist.github.com/anonymous/80ad4c56d6d3222a2a1937a7e9f5ca5b</li>
</ul>
<p>I think there is some part of either the “qt plugin” responsible for the image handling in question, or the <code>QPainter</code> itself (with automatic storage duration in our implementaiton), that stores a pointer to the <code>QImage</code> for which it is created, without considering it to have ownership of said pointer (somehow fooling the sanitizer into thinking that the leak is indirect).</p>
<p>Best Regards,<br />
Filip</p>
</body>
</html>