<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/128569/">https://git.reviewboard.kde.org/r/128569/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 2nd, 2016, 12:41 p.m. EDT, <b>Urs Wolfer</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;">Thanks, looks good. I think it could fix the referenced issue. I assume you were able to reproduce the described issue without the fix? If you cannot commit by yourself, please let me know.</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;">I'll commit it and close the bug. I couldn't reproduce the exactly, as I couldn't connect to my server (due to other reasons not related to krdc). But I saw the crash when it failed to connect, which I went after. The bug report looks similar, and the code behaviour matches what I expect caused the issue.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Do you prefer me to merge the branch to master? Or would you prefer to?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Thanks for the review!</p></pre>
<br />
<p>- Matthew</p>
<br />
<p>On August 1st, 2016, 1:28 p.m. EDT, Matthew Dawson 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 Utils and Urs Wolfer.</div>
<div>By Matthew Dawson.</div>
<p style="color: grey;"><i>Updated Aug. 1, 2016, 1:28 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=365054">365054</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
krdc
</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 xfreerdp sent a failure message then quit, the RDP plugin would first
show a message box with a relevant message. During that time, the inner
event loop would handle the xfreerdp process quitting, which would signal
krdc to cleanup the RDP plugin's resources. When the message box event loop
completed, the RdpView would have been destroyed, causing a crash.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Now the RdpView signals that the view is quitting as soon as the message box
is shown, and avoids trying to quit twice if another part signals a quit.
Once the message box is dismissed, the process continues as normal. This
adds the necessary mechanisms to handle this, as well as moving the message
box display code to connectionError, to avoid some duplication.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">BUG: 365054
REVIEW: 128569</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I've listed the above bug, as I suspect it is part of this issue, but I'm not sure. I'll take out the reference if desired.</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;">Before, when attempting a connection to an unreachable RDP server the app crashed after dismissing the dialogue box.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">After, it no longer does. Everything appears to still clean up.</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>rdp/rdpview.h <span style="color: grey">(0980c8d83d08b6e860af0d5b456a5b040f6aae80)</span></li>
<li>rdp/rdpview.cpp <span style="color: grey">(7f6081277919a41e35456d8069e5fc7cad23e9ad)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/128569/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>