<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/127038/">https://git.reviewboard.kde.org/r/127038/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 12th, 2016, 1:05 a.m. CET, <b>Aleix Pol Gonzalez</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;">Just a warning to stderr might be a bit hard to guess?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">In fact it might be useful to add a property to the NativeAppJob that we can simply set when configuring the CTest job to skip this check instead.</p></pre>
</blockquote>
<p>On February 12th, 2016, 10:08 a.m. CET, <b>Maciej Cencora</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;">Would you care to explain what is this feature all about?</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;">KDevelop just used to spawn a process, which made testing hard sometimes because you had to remember to shut down applications. This was especially bad when debugging a dbus service. The solution for that was to add this message box, but that's entirely for user-triggered executables but for tests it shouldn't happen.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Maybe the messagebox is just on the wrong layer.</p></pre>
<br />
<p>- Aleix</p>
<br />
<p>On February 11th, 2016, 9:46 a.m. CET, Maciej Cencora 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 KDevelop.</div>
<div>By Maciej Cencora.</div>
<p style="color: grey;"><i>Updated Feb. 11, 2016, 9:46 a.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;">If we happen to enter the if branch, QMessageBox is shown and local processing loop gets invoked.
If a job that was on currentJobs() list finishes before user closes the message box, the job will be deleted and on next iteration of the foreach loop, findNativeJob will crash trying to perform qobject_cast on deleted object.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">currentJobList(job#1, job#2)
enter loop:
- first loop iteration (process job#1)
- duplication detected
- show QMessageBox
- enter local event loop
- job#2 finishes
- user closes the QMessageBox
- QMessageBox returns
- next loop iteration (process job#2)
- findNativeJob(job#2)
- qobject_cast on deleted object -> CRASH</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">While attached patch fixes the crash, it does not fix a problem of running multiple test suites in parallel (when starting one after another manually by user).
I don't know why we allow only one job per configuration. Is this loop needed at all?
I need your help on where to go from this.</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;">To reproduce this issue:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">1) create empty cmake project with two tests:
cmake_minimum_required(VERSION 2.6)
project(unit_test_crash)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">enable_testing()</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">find_program(SLEEP sleep)
find_program(ECHO echo)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">add_test(unit_test_crash ${SLEEP} 5)
add_test(unit_test_crash2 ${ECHO} "Test 2 finished")</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">2) show the Unit Tests view
3) start the test "unit_test_crash" (by single left mouse click on it)
4) while the first test is running, start the second test</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/execute/nativeappjob.cpp <span style="color: grey">(fcc0a38)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/127038/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>