From 15f49f16085b5adb65ec49f8979d4aac0369ce99 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Henrik=20Grubbstr=C3=B6m=20=28Grubba=29?=
<grubba@grubba.org>
Date: Thu, 13 Feb 2020 12:42:44 +0100
Subject: [PATCH] I/O [NT]: Keep track of processes using ConPTY slaves.
Due to the way that ptys are implemented on NT, the master side
read pipe will not receive an EOF when all clients have terminated
(since the ConPTY process is still running).
We now keep track of the processes and kill the ConPTY when all
the processes using the slave side are gone. Note that this is
NOT 100% (or even 80%) safe. Indirect processes may still be
running, or processes may terminate after we have already entered
a blocking read().
A feature request has been registered with the Windows Terminal
team (https://github.com/microsoft/terminal/issues/4564).
---
src/fdlib.c | 31 ++++++++++++++++++++++++-
src/fdlib.h | 5 +++-
src/signal_handler.c | 55 ++++++++++++++++++++++++++++++++++++++++++++
src/signal_handler.h | 4 ++++
4 files changed, 93 insertions(+), 2 deletions(-)
diff --git a/src/fdlib.c b/src/fdlib.c
index ff2f0bb7e2..41367bc6a7 100644
--- a/src/fdlib.c
+++ b/src/fdlib.c
@@ -268,12 +268,22 @@ PMOD_EXPORT void set_errno_from_win32_error (unsigned long err)
#include "ntlibfuncs.h"
+void free_pty(struct my_pty *pty)
+{
+ if (sub_ref(pty)) return;
+ free(pty);
+}
+
/* NB: fd_mutex MUST be held at entry. */
static void close_pty(struct my_pty *pty)
{
struct my_pty *other;
+ struct pid_status *pid;
- if (sub_ref(pty)) return;
+ if (--pty->fd_refs) {
+ sub_ref(pty);
+ return;
+ }
CloseHandle(pty->write_pipe);
CloseHandle(pty->read_pipe);
@@ -289,6 +299,11 @@ static void close_pty(struct my_pty *pty)
pty->other = NULL;
}
+ while ((pid = pty->clients)) {
+ pid->clients = pid_status_unlink_pty(pid);
+ }
+
+ free_pty(pty);
}
#define ISSEPARATOR(a) ((a) == '\\' || (a) == '/')
@@ -2152,6 +2167,15 @@ PMOD_EXPORT ptrdiff_t debug_fd_read(FD fd, void *to, ptrdiff_t len)
{
struct my_pty *pty = (struct my_pty *)handle;
handle = pty->read_pipe;
+
+ if (pty->conpty && !pty->other && !check_pty_clients(pty)) {
+ /* Master side, no local slave and all known clients are dead.
+ *
+ * Terminate the ConPTY so that we can get an EOF.
+ */
+ Pike_NT_ClosePseudoConsole(pty->conpty);
+ pty->conpty = 0;
+ }
}
/* FALLTHRU */
@@ -2626,6 +2650,8 @@ PMOD_EXPORT FD debug_fd_dup(FD from)
if (fd < 0) return -1;
add_ref(pty);
+ pty->fd_refs++;
+
release_fd(fd);
return fd;
}
@@ -2677,6 +2703,7 @@ PMOD_EXPORT FD debug_fd_dup2(FD from, FD to)
* or freed by close_pty().
*/
add_ref(pty);
+ pty->fd_refs++;
x = h;
} else if(!DuplicateHandle(p, h, p, &x, 0, 0, DUPLICATE_SAME_ACCESS)) {
release_fd(from);
@@ -2799,10 +2826,12 @@ PMOD_EXPORT int debug_fd_openpty(int *master, int *slave,
}
add_ref(master_pty);
+ master_pty->fd_refs++;
master_pty->read_pipe = INVALID_HANDLE_VALUE;
master_pty->write_pipe = INVALID_HANDLE_VALUE;
master_pty->other = slave_pty;
add_ref(slave_pty);
+ slave_pty->fd_refs++;
slave_pty->read_pipe = INVALID_HANDLE_VALUE;
slave_pty->write_pipe = INVALID_HANDLE_VALUE;
slave_pty->other = master_pty;
diff --git a/src/fdlib.h b/src/fdlib.h
index 74bfdf3ce0..b632ff734f 100644
--- a/src/fdlib.h
+++ b/src/fdlib.h
@@ -171,6 +171,7 @@ struct winsize {
/* Prototypes begin here */
PMOD_EXPORT void set_errno_from_win32_error (unsigned long err);
+void free_pty(struct my_pty *pty);
int fd_to_handle(int fd, int *type, HANDLE *handle, int exclusive);
void release_fd(int fd);
PMOD_EXPORT char *debug_fd_info(int fd);
@@ -277,9 +278,11 @@ struct my_fd_set_s
struct my_pty
{
- INT32 refs; /* Number of references from da_handle[]. */
+ INT32 refs; /* Total number of references. */
+ INT32 fd_refs; /* Number of references from da_handle[]. */
HPCON conpty; /* Only valid for master side. */
struct my_pty *other; /* Other end (if any), NULL otherwise. */
+ struct pid_status *clients; /* List of client processes. */
HANDLE read_pipe; /* Pipe that supports read(). */
HANDLE write_pipe; /* Pipe that supports write(). */
};
diff --git a/src/signal_handler.c b/src/signal_handler.c
index 243feb493c..0400aa5ee7 100644
--- a/src/signal_handler.c
+++ b/src/signal_handler.c
@@ -1254,6 +1254,8 @@ struct pid_status
INT_TYPE pid;
#ifdef __NT__
HANDLE handle;
+ struct pid_status *next_pty_client; /* PTY client chain. */
+ struct my_pty *pty; /* PTY master, refcounted. */
#else
INT_TYPE sig;
INT_TYPE flags;
@@ -1279,6 +1281,48 @@ static void init_pid_status(struct object *UNUSED(o))
#endif
}
+#ifdef __NT__
+struct pid_status *pid_status_unlink_pty(struct pid_status *pid)
+{
+ struct pid_status *ret;
+ if (!pid) return NULL;
+ ret = pid->next_pty_client;
+ pid->next_pty_client = NULL;
+ if (pid->pty) {
+ free_pty(pid->pty);
+ pid->pty = NULL;
+ }
+ return ret;
+}
+
+static void pid_status_link_pty(struct pid_status *pid, struct my_pty *pty)
+{
+ add_ref(pty);
+ pid->pty = pty;
+ pid->next_pty_client = pty->clients;
+ pty->clients = pid;
+}
+
+int check_pty_clients(struct my_pty *pty)
+{
+ struct pid_status *pid;
+ while ((pid = pty->clients)) {
+ DWORD status;
+ /* FIXME: RACE: pid->handle my be INVALID_HANDLE_VALUE if
+ * the process is about to be started.
+ */
+ if ((pid->pid == -1) && (pid->handle == INVALID_HANDLE_VALUE)) return 1;
+ if (GetExitCodeProcess(pid->handle, &status) &&
+ (status == STILL_ACTIVE)) {
+ return 1;
+ }
+ /* Process has terminated. Unlink and check the next. */
+ pty->clients = pid_status_unlink_pty(pid);
+ }
+ return 0;
+}
+#endif /* __NT__ */
+
static void exit_pid_status(struct object *UNUSED(o))
{
#ifdef USE_PID_MAPPING
@@ -1291,6 +1335,16 @@ static void exit_pid_status(struct object *UNUSED(o))
#endif
#ifdef __NT__
+ if (THIS->pty) {
+ struct my_pty *pty = THIS->pty;
+ struct pid_status *pidptr = &pty->clients;
+ while (*pidptr && (*pidptr != THIS)) {
+ pidptr = &(*pidptr)->next_pty_client;
+ }
+ if (*pidptr) {
+ *pidptr = pid_status_unlink_pty(THIS);
+ }
+ }
CloseHandle(THIS->handle);
#endif
}
@@ -2187,6 +2241,7 @@ static HANDLE get_inheritable_handle(struct mapping *optional,
* pty for stdin, stdout and stderr wins
* (see above).
*/
+ pid_status_link_pty(pty->other);
*conpty = pty->other->conpty;
release_fd(fd);
return ret;
diff --git a/src/signal_handler.h b/src/signal_handler.h
index ce1a37aa7f..8a2c0d1245 100644
--- a/src/signal_handler.h
+++ b/src/signal_handler.h
@@ -14,6 +14,10 @@ struct sigdesc;
void my_signal(int sig, sigfunctype fun);
PMOD_EXPORT void check_signals(struct callback *foo, void *bar, void *gazonk);
void set_default_signal_handler(int signum, void (*func)(INT32));
+#ifdef __NT__
+struct pid_status *pid_status_unlink_pty(struct pid_status *pid);
+int check_pty_clients(struct my_pty *pty);
+#endif
void process_started(pid_t pid);
void process_done(pid_t pid, const char *from);
struct wait_data;
--
GitLab