diff --git a/src/fdlib.c b/src/fdlib.c index 41367bc6a793b492cdccaab1e86be94cf07e0a37..afa31abb3885a5630fd7a7633a66183bc6fef0e6 100644 --- a/src/fdlib.c +++ b/src/fdlib.c @@ -46,6 +46,7 @@ #endif #include "threads.h" +#include "signal_handler.h" /* Mutex protecting da_handle, fd_type, fd_busy and first_free_handle. */ static MUTEX_T fd_mutex; @@ -287,10 +288,6 @@ static void close_pty(struct my_pty *pty) CloseHandle(pty->write_pipe); CloseHandle(pty->read_pipe); - if (pty->conpty) { - /* Closing the master side. */ - Pike_NT_ClosePseudoConsole(pty->conpty); - } /* Unlink the pair. */ other = pty->other; @@ -299,8 +296,14 @@ static void close_pty(struct my_pty *pty) pty->other = NULL; } + if (pty->conpty) { + /* Closing the master side. */ + Pike_NT_ClosePseudoConsole(pty->conpty); + pty->conpty = 0; + } + while ((pid = pty->clients)) { - pid->clients = pid_status_unlink_pty(pid); + pty->clients = pid_status_unlink_pty(pid); } free_pty(pty); diff --git a/src/signal_handler.c b/src/signal_handler.c index 0400aa5ee74cc71e33d3a394723693a769deacdb..2742dad8e1e61d6393b293097d7a9db8af9f2c5c 100644 --- a/src/signal_handler.c +++ b/src/signal_handler.c @@ -1337,13 +1337,22 @@ static void exit_pid_status(struct object *UNUSED(o)) #ifdef __NT__ if (THIS->pty) { struct my_pty *pty = THIS->pty; - struct pid_status *pidptr = &pty->clients; + struct pid_status **pidptr = &pty->clients; while (*pidptr && (*pidptr != THIS)) { pidptr = &(*pidptr)->next_pty_client; } if (*pidptr) { *pidptr = pid_status_unlink_pty(THIS); } + if (!pty->clients && !pty->other && pty->conpty) { + /* Last client for this ConPTY has terminated, + * and our local copy of the slave has been closed. + * + * Close the ConPTY. + */ + Pike_NT_ClosePseudoConsole(pty->conpty); + pty->conpty = 0; + } } CloseHandle(THIS->handle); #endif @@ -2229,22 +2238,38 @@ static HANDLE get_inheritable_handle(struct mapping *optional, } else { /* Slave side. Get the conpty from the master side. * - * NB: It seems stdin, stdout and stderr are handled - * automatically by setting the conpty. The conpty - * apparently has duplicated handles of the original - * pipes, which most likely have been modified to - * actually look like ttys. We thus do NOT return - * the base pipe handle. + * NB: Stdin, stdout and stderr are handled automatically + * by setting the conpty. The conpty has duplicated + * handles of the original pipes, and in turn provides + * actual CONSOLE handles to the subprocess. We thus + * do NOT return the base pipe handle. * * BUGS: It is not possible to have multiple conpty * objects for the same process, so the first * pty for stdin, stdout and stderr wins * (see above). + * + * BUGS: As the conpty is a separate process that survives + * or subprocess terminating, we need to keep track + * of which master pty the process was bound to so + * that we can close the corresponding conpty + * when no one else will use it. */ - pid_status_link_pty(pty->other); + pid_status_link_pty(THIS, pty->other); *conpty = pty->other->conpty; release_fd(fd); - return ret; + + /* From the documentation for GetStdHandle(): + * + * If the existing value of the standard handle is NULL, or + * the existing value of the standard handle looks like a + * console pseudohandle, the handle is replaced with a + * console handle. + * + * This is not documented in conjunction with CreateProcess() or + * PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE, but it seems to work. + */ + return 0; } }