<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/120150/">https://git.reviewboard.kde.org/r/120150/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 12th, 2014, 7:31 p.m. CEST, <b>René J.V. Bertin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">So, if I activate malloc/free guarding too early (in PatchReviewPlugin::setPatch, when called with a "real" patch, not at plugin load time), I get this:</p>
<h3 style="font-size: 100%;text-rendering: inherit;padding: 0;white-space: normal;margin: 0;line-height: inherit;">starting malloc guarding</h3>
<h3 style="font-size: 100%;text-rendering: inherit;padding: 0;white-space: normal;margin: 0;line-height: inherit;">PatchReviewPlugin::setPatch() deleting previous patch LocalPatchSource(0x1237b3830) "Custom Patch" with file KUrl("") basedir KUrl("")</h3>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">setting new patch "VCS Diff" with file KUrl("file:///private/var/folders/W2/W27-dhO62RWW-U+BYnQ9i++++TY/-Tmp-/kde-bertin/kdevelopC890942.patch") basedir KUrl("file:///Volumes/Debian/Users/bertin/work/src/new/KDE/kdevelop/kdevplatform-git-1.7.60/") <br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
kdevelop.bin(89094,0x7fff70affcc0) malloc: <strong style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;"><em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;"> error for object 0x138a80f10: Non-aligned pointer being freed (2)<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
</em></strong> set a breakpoint in malloc_error_break to debug</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Program received signal SIGABRT, Aborted.<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
0x00007fff85f470b6 in __kill ()<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
(gdb) bt</p>
<h1 style="font-size: 100%;text-rendering: inherit;padding: 0;white-space: normal;margin: 0;line-height: inherit;">0 0x00007fff85f470b6 in __kill ()</h1>
<h1 style="font-size: 100%;text-rendering: inherit;padding: 0;white-space: normal;margin: 0;line-height: inherit;">1 0x00007fff85fe79f6 in abort ()</h1>
<h1 style="font-size: 100%;text-rendering: inherit;padding: 0;white-space: normal;margin: 0;line-height: inherit;">2 0x00007fff85fd662d in szone_error ()</h1>
<h1 style="font-size: 100%;text-rendering: inherit;padding: 0;white-space: normal;margin: 0;line-height: inherit;">3 0x00007fff827358d4 in -[NSViewHierarchyLock dealloc] ()</h1>
<h1 style="font-size: 100%;text-rendering: inherit;padding: 0;white-space: normal;margin: 0;line-height: inherit;">4 0x00007fff8273443c in -[NSWindow dealloc] ()</h1>
<h1 style="font-size: 100%;text-rendering: inherit;padding: 0;white-space: normal;margin: 0;line-height: inherit;">5 0x0000000101ed6be2 in -[QCocoaPanel dealloc] ()</h1>
<h1 style="font-size: 100%;text-rendering: inherit;padding: 0;white-space: normal;margin: 0;line-height: inherit;">6 0x0000000101ed12e0 in QWidget::destroy ()</h1>
<h1 style="font-size: 100%;text-rendering: inherit;padding: 0;white-space: normal;margin: 0;line-height: inherit;">7 0x0000000101f89222 in QWidget::~QWidget ()</h1>
<h1 style="font-size: 100%;text-rendering: inherit;padding: 0;white-space: normal;margin: 0;line-height: inherit;">8 0x000000012926f985 in ProjectTreeView::popupContextMenu (this=<value temporarily unavailable, due to optimizations>, pos=<value temporarily unavailable, due to optimizations>) at /Volumes/Debian/MP6/var/macports/build/_Volumes_Debian_MP6_site-ports_kde_kdevplatform-git/kdevplatform-git/work/kdevplatform-git-1.7.60/plugins/projectmanagerview/projecttreeview.cpp:316</h1>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Previous frame inner to this frame (gdb could not unwind past this frame)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">In other words, I expose a situation that either doesn't occur or is handled silently without malloc guarding, and it gets triggered because apparently the popup menu's breakdown happens concurrently with the patchreview toolview's initialisation.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Is there a place in patchreview.cpp where I can assume that the popup menu through which I get to the toolview is completely gone (and any malloc/free errors exposed are in the plugin code)?</p></pre>
</blockquote>
<p>On September 13th, 2014, 6:27 p.m. CEST, <b>Milian Wolff</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This looks like there is an issue with deleting the context menu widget, probably due to nested eventloop madness and such.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">We might need to replace the stack-allocated QMenu's with heap-allocated ones and replace the blocking <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">QMenu::exec()</code> with the non-blocking <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">QMenu::popup()</code>. This removes the nested eventloop.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">If you feel like doing that cleanup, it would be greatly appreciated. Generally though, it might make more sense to do this in the frameworks branch as you'll need to change quite a bit of code, I guess.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">And to reiterate: None of the above is OS-X specific. You might just be "lucky" in seeing a reliable crash which just randomly happens to work on Linux, thus noone saw the issues.</p></pre>
</blockquote>
<p>On September 13th, 2014, 7:03 p.m. CEST, <b>René J.V. Bertin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">madness is indeed the word, I keep having to force my mind around the implications of using that blocking call. Yes, the context menu is supposed to be released before we're really out of the function, but no, the patch review thingy's activation doesn't wait for that.<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
I'm not sure if I've already mentioned it on here, but the "frameworks branch" (as in the KF5 version) is still a while off on OS X, and my quick attempt at installing Project Neon5 to build kdevelop under KF5 on Linux didn't get very far. So much as I'd appreciate to do <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">that cleanup</em>, it'll have to wait a bit :)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Unless there's an intermediate solution. Is it not possible to set the context menu to return the activated item, or the slot to trigger, so as to defer that at least until after the menu's event loop has exited? I know it's an oldfashioned and maybe even uncool approach, but this one ought to make debugging easier ;)</p></pre>
</blockquote>
<p>On September 13th, 2014, 7:16 p.m. CEST, <b>Milian Wolff</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">You can try to make the connection to the QAction a Qt::QueuedConnection. Maybe that helps.</p></pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Is this what you had in mind, in vcspluginhelper.cpp?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">void createActions(VcsPluginHelper* parent) {<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
// ...<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
diffToBaseAction = new KAction(KIcon("text-x-patch"), i18n("Show Differences..."), parent);<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
revertAction = new KAction(KIcon("archive-remove"), i18n("Revert"), parent);<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
historyAction = new KAction(KIcon("view-history"), i18n("History..."), parent);<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
annotationAction = new KAction(KIcon("user-properties"), i18n("Annotation..."), parent);<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
diffForRevAction = new KAction(KIcon("text-x-patch"), i18n("Show Diff..."), parent);<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
diffForRevGlobalAction = new KAction(KIcon("text-x-patch"), i18n("Show Diff (all files)..."), parent);<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
// ...<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
connect(diffToBaseAction, SIGNAL(triggered()), parent, SLOT(diffToBase()), Qt::QueuedConnection);<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
connect(revertAction, SIGNAL(triggered()), parent, SLOT(revert()));<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
connect(historyAction, SIGNAL(triggered()), parent, SLOT(history()), Qt::QueuedConnection);<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
connect(annotationAction, SIGNAL(triggered()), parent, SLOT(annotation()), Qt::QueuedConnection);<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
connect(diffForRevAction, SIGNAL(triggered()), parent, SLOT(diffForRev()), Qt::QueuedConnection);<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
//...<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
}</p></pre>
<br />
<p>- René J.V.</p>
<br />
<p>On September 12th, 2014, 10:30 p.m. CEST, René J.V. Bertin wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KDE Software on Mac OS X, KDevelop and Olivier Goffart.</div>
<div>By René J.V. Bertin.</div>
<p style="color: grey;"><i>Updated Sept. 12, 2014, 10:30 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdevplatform
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">In https://reviewboard.kde.org/r/120081/ I proposed an (accepted) approach to prevent kdevelop from crashing after closing the patch review ("git/show differences") toolview. I had another of those crashes after heavier-than-usual perusal of the toolview, despite the previous patch. The attached patch replaces all <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">delete</code>s of <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">QObject</code> derived class instances with <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">deleteLater()</code>.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">kdevplatform git/kde4-legacy on KDE/MacPorts OS X 10.6.8 . </p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>plugins/patchreview/patchreview.cpp <span style="color: grey">(18b63db)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/120150/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>