[prev in list] [next in list] [prev in thread] [next in thread]
List: konsole-devel
Subject: Re: Review Request 129984: Prevent misdetection of EOF on Linux
From: Kurt Hindenburg <kurt.hindenburg () gmail ! com>
Date: 2017-03-08 2:04:22
Message-ID: 20170308020422.13092.89535 () mimi ! kde ! org
[Download RAW message or body]
--===============4823119506918845884==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129984/#review102758
-----------------------------------------------------------
Since this changes kpty, it needs to get reviewed on https://phabricator.kde.org/ - \
can you please open a request there?
- Kurt Hindenburg
On March 4, 2017, 10:56 p.m., Peter Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129984/
> -----------------------------------------------------------
>
> (Updated March 4, 2017, 10:56 p.m.)
>
>
> Review request for Konsole and Kurt Hindenburg.
>
>
> Bugs: 372991
> https://bugs.kde.org/show_bug.cgi?id=372991
>
>
> Repository: kpty
>
>
> Description
> -------
>
> On Linux, even if slave activity is detected on a master fd, FIONREAD
> can still return zero available bytes in the buffer. Previously this
> would prevent further signals from being emitted even if new IO arrived
> (since the readNotifier is disabled) which in its turn prevented the
> Konsole output from being updated.
>
> A race condition in the kernel is suspected, the first known broken
> kernel is v4.1.10-89-g5eb491ba5d06.
>
> BUG: 372991
>
>
> Diffs
> -----
>
> src/kptydevice.cpp 22233a505d32d988795839cb592098c38f7fab33
>
> Diff: https://git.reviewboard.kde.org/r/129984/diff/
>
>
> Testing
> -------
>
> Tested in conjunction with konsole v16.12.1-60-g151215a9 with this debug patch \
> applied: ```
> diff --git a/src/kptydevice.cpp b/src/kptydevice.cpp
> index 92b443b..4dfcd02 100644
> --- a/src/kptydevice.cpp
> +++ b/src/kptydevice.cpp
> @@ -273,6 +273,7 @@ struct KPtyDevicePrivate : public KPtyPrivate {
> KRingBuffer writeBuffer;
> };
>
> +#include <cstdio>
> bool KPtyDevicePrivate::_k_canRead()
> {
> Q_Q(KPtyDevice);
> @@ -310,6 +311,9 @@ bool KPtyDevicePrivate::_k_canRead()
> // select() will trigger again anyway if an EOF condition was found, and
> // only then we will accept it.
> if (!available && !maybeEof) {
> + FILE *fp = fopen("/tmp/EOF", "a");
> + fprintf(fp, "Maybe EOF!?\n");
> + fclose(fp);
> maybeEof = true;
> return true;
> } else if (available) {
>
> ```
>
> Indeed, the /tmp/EOF file is being written and the Konsole output no longer \
> "hangs".
> Closing a tab (via EOF in bash or closing the tab) somehow does not trigger this \
> debug print, perhaps the watcher is disabled elsewhere. (This is an observation and \
> not a problem.)
>
> Thanks,
>
> Peter Wu
>
>
--===============4823119506918845884==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 7bit
<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/129984/">https://git.reviewboard.kde.org/r/129984/</a>
</td>
</tr>
</table>
<br />
<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;">Since \
this changes kpty, it needs to get reviewed on https://phabricator.kde.org/ - can you \
please open a request there?</p></pre> <br />
<p>- Kurt Hindenburg</p>
<br />
<p>On March 4th, 2017, 10:56 p.m. UTC, Peter Wu 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 Konsole and Kurt Hindenburg.</div>
<div>By Peter Wu.</div>
<p style="color: grey;"><i>Updated March 4, 2017, 10:56 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="https://bugs.kde.org/show_bug.cgi?id=372991">372991</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kpty
</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;">On Linux, even if slave activity is detected on a master fd, FIONREAD \
can still return zero available bytes in the buffer. Previously this would prevent \
further signals from being emitted even if new IO arrived (since the readNotifier is \
disabled) which in its turn prevented the Konsole output from being updated.
A race condition in the kernel is suspected, the first known broken
kernel is v4.1.10-89-g5eb491ba5d06.
BUG: 372991</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;">Tested in conjunction with konsole \
v16.12.1-60-g151215a9 with this debug patch applied:</p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><div \
class="codehilite" style="background: #f8f8f8"><pre style="line-height: \
125%"><span></span><span style="color: #000080; font-weight: bold">diff --git \
a/src/kptydevice.cpp b/src/kptydevice.cpp</span> <span style="color: #000080; \
font-weight: bold">index 92b443b..4dfcd02 100644</span> <span style="color: \
#A00000">--- a/src/kptydevice.cpp</span> <span style="color: #00A000">+++ \
b/src/kptydevice.cpp</span> <span style="color: #800080; font-weight: bold">@@ -273,6 \
+273,7 @@ struct KPtyDevicePrivate : public KPtyPrivate {</span> KRingBuffer \
writeBuffer; };
<span style="color: #00A000">+#include <cstdio></span>
bool KPtyDevicePrivate::_k_canRead()
{
Q_Q(KPtyDevice);
<span style="color: #800080; font-weight: bold">@@ -310,6 +311,9 @@ bool \
KPtyDevicePrivate::_k_canRead()</span>
// select() will trigger again anyway if an EOF condition was found, and
// only then we will accept it.
if (!available && !maybeEof) {
<span style="color: #00A000">+ FILE *fp = fopen("/tmp/EOF", \
"a");</span> <span style="color: #00A000">+ fprintf(fp, \
"Maybe EOF!?\n");</span> <span style="color: #00A000">+ \
fclose(fp);</span> maybeEof = true;
return true;
} else if (available) {
</pre></div>
</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Indeed, the /tmp/EOF file is being written and the \
Konsole output no longer "hangs".</p> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">Closing a tab (via EOF \
in bash or closing the tab) somehow does not trigger this debug print, perhaps the \
watcher is disabled elsewhere. (This is an observation and not a problem.)</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>src/kptydevice.cpp <span style="color: \
grey">(22233a505d32d988795839cb592098c38f7fab33)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/129984/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
--===============4823119506918845884==--
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic