Package: pterm
Version: 0.70-6
Severity: minor
Tags: patch upstream
It looks superficiallu like #165887 is back:
[spirit ~]ls -l .putty/sessions
total 0
[spirit ~]pterm +ut&
[1] 11172
[spirit ~]echo $!
11172
[spirit ~]ps f
PID TTY STAT TIME COMMAND
[...]
11172 pts/11 SN 0:00 \_ pterm +ut
11175 pts/11 ZN 0:00 | \_ [pterm] <defunct>
11176 pts/21 SNs+ 0:00 | \_ -zsh
11233 pts/11 R+ 0:00 \_ ps f
[...]
But the `SIGCHLD' code is all there still. What gives? It turns out
that `pterm''s startup code is racy.
Let's walk through what happens.
1. We start up and enter `pty_pre_init' to split off the privileged
`utmp' helper process. We hang onto a pipe with which to
communicate with the helper, and carry on.
2. Stuff happens, and eventually we enter `pty_init'. Fairly early
on, we run this code:
/*
* Stamp utmp (that is, tell the utmp helper process to do so),
* or not.
*/
if (pty_utmp_helper_pipe >= 0) { /* if it's < 0, we can't anyway */
if (!conf_get_int(conf, CONF_stamp_utmp)) {
close(pty_utmp_helper_pipe); /* just let the child process
die */
pty_utmp_helper_pipe = -1;
} else {
[...]
}
}
Because I've set `+ut' on the command-line, `CONF_stamp_utmp' is
off, and we close our pipe to the helper, which is running this:
while (1) {
ret = read(pipefd[0], buffer, lenof(buffer));
if (ret <= 0) {
cleanup_utmp();
_exit(0);
} else
[...]
}
So the helper will quit as soon as it notices. When that happens,
the main `pterm' process gets `SIGCHLD', which is going to run
static void sigchld_handler(int signum)
{
if (write(pty_signal_pipe[1], "x", 1) <= 0)
/* not much we can do about it */;
}
Ahh! But what's `pty_signal_pipe[1]' at this point?
3. Well, it's funny you should ask that. It starts out as -1 because
/*
* The pty_signal_pipe, along with the SIGCHLD handler, must be
* process-global rather than session-specific.
*/
static int pty_signal_pipe[2] = { -1, -1 }; /* obviously bogus
initial val */
and is set at the /end/ of `pty_init':
if (pty_signal_pipe[0] < 0) {
if (pipe(pty_signal_pipe) < 0) {
perror("pipe");
exit(1);
}
cloexec(pty_signal_pipe[0]);
cloexec(pty_signal_pipe[1]);
}
So what goes wrong is that sometimes (mostly, in fact, at least on my
machine), the `SIGCHLD' comes before the signal pipe is established: the
`write' fails with `EBADF' because `pty_signal_pipe[1]' is still -1, but
this is ignored (though honestly we're in a signal handler at this point
and good options aren't readily available), and the child process is
left as a zombie.
The following patch fixes this for me.
--------------------------------------------------
diff --git a/unix/uxpty.c b/unix/uxpty.c
index 8be507d5..57ce02f6 100644
--- a/unix/uxpty.c
+++ b/unix/uxpty.c
@@ -757,6 +757,15 @@ static const char *pty_init(void *frontend, void
**backend_handle, Conf *conf,
pty->term_width = conf_get_int(conf, CONF_width);
pty->term_height = conf_get_int(conf, CONF_height);
+ if (pty_signal_pipe[0] < 0) {
+ if (pipe(pty_signal_pipe) < 0) {
+ perror("pipe");
+ exit(1);
+ }
+ cloexec(pty_signal_pipe[0]);
+ cloexec(pty_signal_pipe[1]);
+ }
+
if (pty->master_fd < 0)
pty_open_master(pty);
@@ -1008,14 +1017,6 @@ static const char *pty_init(void *frontend, void
**backend_handle, Conf *conf,
add234(ptys_by_pid, pty);
}
- if (pty_signal_pipe[0] < 0) {
- if (pipe(pty_signal_pipe) < 0) {
- perror("pipe");
- exit(1);
- }
- cloexec(pty_signal_pipe[0]);
- cloexec(pty_signal_pipe[1]);
- }
pty_uxsel_setup(pty);
*backend_handle = pty;
--------------------------------------------------
This also affects sid, and current upstream master. The necessary patch
is superficially different because the code has been refactored and has
grown new features in the meantime, but the basic problem is the same
even if the guts of `pty_init' are now in `pty_backend_create':
--------------------------------------------------
diff --git a/unix/uxpty.c b/unix/uxpty.c
index 346fdb55..09c5ecb0 100644
--- a/unix/uxpty.c
+++ b/unix/uxpty.c
@@ -890,6 +890,15 @@ Backend *pty_backend_create(
pty->fds[i].pty = pty;
}
+ if (pty_signal_pipe[0] < 0) {
+ if (pipe(pty_signal_pipe) < 0) {
+ perror("pipe");
+ exit(1);
+ }
+ cloexec(pty_signal_pipe[0]);
+ cloexec(pty_signal_pipe[1]);
+ }
+
pty->seat = seat;
pty->backend.vt = &pty_backend;
@@ -1250,14 +1259,6 @@ Backend *pty_backend_create(
add234(ptys_by_pid, pty);
}
- if (pty_signal_pipe[0] < 0) {
- if (pipe(pty_signal_pipe) < 0) {
- perror("pipe");
- exit(1);
- }
- cloexec(pty_signal_pipe[0]);
- cloexec(pty_signal_pipe[1]);
- }
pty_uxsel_setup(pty);
return &pty->backend;
--------------------------------------------------
This fixes the symptom in the common case. There's still a race here,
because the helper process might conceivably crash even before having
its input pipe closed, but I don't understand the way everything works
to suggest a proper fix for this case.
-- System Information:
Debian Release: 10.3
APT prefers stable
APT policy: (990, 'stable'), (500, 'stable-updates'), (500,
'oldstable-updates'), (500, 'testing'), (500, 'oldstable')
Architecture: amd64 (x86_64)
Foreign Architectures: i386
Kernel: Linux 5.4.0-4-amd64 (SMP w/8 CPU cores)
Locale: LANG=en_GB.UTF-8, LC_CTYPE=en_GB.UTF-8 (charmap=UTF-8),
LANGUAGE=en_GB.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: sysvinit (via /sbin/init)
Versions of packages pterm depends on:
ii libatk1.0-0 2.30.0-2
ii libc6 2.28-10
ii libcairo-gobject2 1.16.0-4
ii libcairo2 1.16.0-4
ii libgdk-pixbuf2.0-0 2.38.1+dfsg-1
ii libglib2.0-0 2.58.3-2+deb10u2
ii libgtk-3-0 3.24.5-1
ii libpango-1.0-0 1.42.4-7~deb10u1
ii libpangocairo-1.0-0 1.42.4-7~deb10u1
ii libx11-6 2:1.6.7-1
pterm recommends no packages.
Versions of packages pterm suggests:
ii putty-doc 0.70-6
-- no debconf information
