[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