<table><tr><td style="">rjvbb added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D6376" rel="noreferrer">View Revision</a></tr></table><br /><div><div><p>A priori this should be fine, and it might even address the long standing bug by leaving more time for Phonon objects to "do their thing". It might be an idea though to include a <tt style="background: #ebebeb; font-size: 13px;">qCDebug()</tt> probe that outputs the number of items left for auto-cleanup in <tt style="background: #ebebeb; font-size: 13px;">m_reusablePhonons</tt>.</p>

<p>But what's the official stance on deleting objects (widgets) that have a parent and are thus capable of auto-cleanup? AFAIK one can still delete them explicitly, and if they're reparented during that process they should no longer be included in the cleanup step performed by their original parent's dtor.</p>

<p>IOW, if you're right, isn't there a bug to address in <tt style="background: #ebebeb; font-size: 13px;">Phonon::MediaObject</tt>?</p>

<p>There can also be another reason *in release builds* for double frees (which your change will probably catch, too): <tt style="background: #ebebeb; font-size: 13px;">NotifyByAudio::notify()</tt> does</p>

<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">Q_ASSERT(!m_notifications.value(m));
m_notifications.insert(m, notification);</pre></div>

<p>and <tt style="background: #ebebeb; font-size: 13px;">NotifyByAudio::finishNotification()</tt> does</p>

<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">    m_notifications.remove(m);
//...
    m_reusablePhonons.append(m);</pre></div>

<p>Release builds do not check if <tt style="background: #ebebeb; font-size: 13px;">m_notifications</tt> already contains the object being added. It seems extremely unlikely that this would ever happen, but apparently the code author thought it could.</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R289 KNotifications</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D6376" rel="noreferrer">https://phabricator.kde.org/D6376</a></div></div><br /><div><strong>To: </strong>cullmann, Frameworks<br /><strong>Cc: </strong>rjvbb<br /></div>