diff --git a/.github/workflows/R-CMD-check.yaml b/.github/workflows/R-CMD-check.yaml index 9ce0693c..89ab8a21 100644 --- a/.github/workflows/R-CMD-check.yaml +++ b/.github/workflows/R-CMD-check.yaml @@ -8,6 +8,7 @@ on: push: branches: [main, master] pull_request: + workflow_dispatch: name: R-CMD-check.yaml diff --git a/NEWS.md b/NEWS.md index a70344d0..12558f43 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,12 +1,19 @@ # processx (development version) +* `process$new()` and `run()` now support `pty = TRUE` on Windows 10 version + 1809 and later, in addition to Unix. The Windows implementation uses the + ConPTY API (`CreatePseudoConsole`). The API is loaded dynamically so + processx continues to load on older Windows and emits a clear error if + `pty = TRUE` is requested on an unsupported version (#231). + * `run()` now supports `pty = TRUE` and `pty_options` to run a process in a - pseudo-terminal (PTY) on Unix. This causes the child to see a real terminal, - so programs that disable colour output or interactive behaviour when not - attached to a terminal will behave as if they are. `stderr` is merged into - `stdout` (the result's `$stderr` is always `NULL`). A file-based `stdin` - argument is also supported: its contents are fed to the process via the PTY - master, followed by an EOF signal (#230). + pseudo-terminal (PTY) on Unix and Windows (see above). This causes the + child to see a real terminal, so programs that disable colour output or + interactive behaviour when not attached to a terminal will behave as if + they are. `stderr` is merged into `stdout` (the result's `$stderr` is + always `NULL`). A file-based `stdin` argument is also supported: its + contents are fed to the process via the PTY master, followed by an EOF + signal (#230). * `process$new()` now supports `">>"` as a prefix for `stdout` and `stderr` file paths (e.g. `stdout = ">>output.log"`), which appends output to the diff --git a/R/initialize.R b/R/initialize.R index 3edf7bec..98dec236 100644 --- a/R/initialize.R +++ b/R/initialize.R @@ -78,9 +78,6 @@ process_initialize <- function( cleanup <- TRUE } - if (pty && os_type() != "unix") { - throw(new_error("`pty = TRUE` is only implemented on Unix")) - } if (pty && tolower(Sys.info()[["sysname"]]) == "sunos") { throw(new_error("`pty = TRUE` is not (yet) implemented on Solaris")) } diff --git a/R/io.R b/R/io.R index 97456717..e9091598 100644 --- a/R/io.R +++ b/R/io.R @@ -114,6 +114,27 @@ process_read_all_output <- function(self, private) { result <- "" while (self$is_incomplete_output()) { self$poll_io(-1) + # On Windows with pty=TRUE the IOCP loop forces timeout=0 once poll_pipe + # signals EOF (process exit), so poll_io(-1) returns immediately after the + # child exits regardless of the requested timeout. conhost.exe processes + # the child's final writes asynchronously; we must poll *only* the stdout + # connection (not the full process) to give conhost time to flush, and then + # call ClosePseudoConsole() explicitly so conhost closes its end of the pipe. + if (private$pty && .Platform$OS.type == "windows" && !self$is_alive()) { + con <- self$get_output_connection() + repeat { + p <- poll(list(con), 1000L)[[1]] + if (!identical(p, "ready")) break + result <- paste0(result, self$read_output()) + } + chain_call(c_processx_pty_close, private$status, + private$get_short_name()) + while (self$is_incomplete_output()) { + poll(list(con), -1L) + result <- paste0(result, self$read_output()) + } + return(result) + } result <- paste0(result, self$read_output()) } result diff --git a/R/run.R b/R/run.R index dba0dfc1..00dd7bed 100644 --- a/R/run.R +++ b/R/run.R @@ -132,13 +132,14 @@ #' `NULL` (no input) or a file path (whose contents are fed to the #' process via the PTY). #' @param pty Whether to use a pseudo-terminal (PTY) for the process. -#' This is only supported on Unix. When `TRUE`, stdout and stderr are -#' merged into a single stream (accessible via `$stdout` in the result), -#' and `$stderr` is always `NULL`. The process sees a real terminal, so -#' programs that disable colour or interactive features when not attached -#' to a terminal will behave as if they are. `stdout` and `stderr` must -#' be left at their defaults (`"|"`), and `stderr_to_stdout`, -#' `stderr_callback`, and `stderr_line_callback` must not be set. +#' Supported on Unix and on Windows 10 version 1809 or later (via +#' ConPTY). When `TRUE`, stdout and stderr are merged into a single +#' stream (accessible via `$stdout` in the result), and `$stderr` is +#' always `NULL`. The process sees a real terminal, so programs that +#' disable colour or interactive features when not attached to a terminal +#' will behave as if they are. `stdout` and `stderr` must be left at +#' their defaults (`"|"`), and `stderr_to_stdout`, `stderr_callback`, +#' and `stderr_line_callback` must not be set. #' @param pty_options Options for the PTY, a named list. See #' [default_pty_options()] for the available options and their defaults. #' @param ... Extra arguments are passed to `process$new()`, see @@ -566,7 +567,17 @@ run_manage <- function( stdin_remaining <- proc$write_input(stdin_remaining) } if (length(stdin_remaining) == 0L) { - proc$write_input(as.raw(c(0x04L, 0x04L))) + ## Signal EOF to the PTY. + ## Unix: two Ctrl+D (0x04) — first flushes the line buffer, + ## second triggers unconditional EOF. + ## Windows ConPTY: Ctrl+Z (0x1A) — the Windows CRT treats this + ## as EOF when reading from a console in text mode. + eof_bytes <- if (.Platform$OS.type == "windows") { + as.raw(0x1aL) + } else { + as.raw(c(0x04L, 0x04L)) + } + proc$write_input(eof_bytes) stdin_eof_sent <- TRUE } } @@ -582,6 +593,24 @@ run_manage <- function( if (spinner) spin() } + ## Windows PTY: after process exit the IOCP timeout is forced to 0 (because + ## poll_pipe is at EOF), so proc$poll_io(-1) returns immediately regardless + ## of the timeout. conhost.exe processes the child's final writes + ## asynchronously; we must poll *only* the stdout connection (not the full + ## process) to give conhost time to flush. Then call ClosePseudoConsole() + ## so conhost closes its write end of the pipe, causing EOF on our read end. + ## Skip this if we already killed the process (timeout / forced kill). + if (pty && .Platform$OS.type == "windows" && has_stdout && !timeout_happened) { + con <- proc$get_output_connection() + repeat { + p <- poll(list(con), 1000L)[[1]] + if (!identical(p, "ready")) break + do_output() + } + priv <- get_private(proc) + chain_call(c_processx_pty_close, priv$status, priv$get_short_name()) + } + ## Needed to get the exit status "!DEBUG run() waiting to get exit status, process `proc$get_pid()`" proc$wait() diff --git a/man/run.Rd b/man/run.Rd index 78f8c067..c61383b3 100644 --- a/man/run.Rd +++ b/man/run.Rd @@ -141,13 +141,14 @@ strings. Line callbacks are not supported in binary mode.} the process has finished.} \item{pty}{Whether to use a pseudo-terminal (PTY) for the process. -This is only supported on Unix. When \code{TRUE}, stdout and stderr are -merged into a single stream (accessible via \verb{$stdout} in the result), -and \verb{$stderr} is always \code{NULL}. The process sees a real terminal, so -programs that disable colour or interactive features when not attached -to a terminal will behave as if they are. \code{stdout} and \code{stderr} must -be left at their defaults (\code{"|"}), and \code{stderr_to_stdout}, -\code{stderr_callback}, and \code{stderr_line_callback} must not be set.} +Supported on Unix and on Windows 10 version 1809 or later (via +ConPTY). When \code{TRUE}, stdout and stderr are merged into a single +stream (accessible via \verb{$stdout} in the result), and \verb{$stderr} is +always \code{NULL}. The process sees a real terminal, so programs that +disable colour or interactive features when not attached to a terminal +will behave as if they are. \code{stdout} and \code{stderr} must be left at +their defaults (\code{"|"}), and \code{stderr_to_stdout}, \code{stderr_callback}, +and \code{stderr_line_callback} must not be set.} \item{pty_options}{Options for the PTY, a named list. See \code{\link[=default_pty_options]{default_pty_options()}} for the available options and their defaults.} diff --git a/src/init.c b/src/init.c index 1b8214b7..5af3cee6 100644 --- a/src/init.c +++ b/src/init.c @@ -65,6 +65,7 @@ static const R_CallMethodDef callMethods[] = { { "processx_exec", (DL_FUNC) &processx_exec, 14 }, { "processx_wait", (DL_FUNC) &processx_wait, 3 }, { "processx_is_alive", (DL_FUNC) &processx_is_alive, 2 }, + { "processx_pty_close", (DL_FUNC) &processx_pty_close, 2 }, { "processx_get_exit_status", (DL_FUNC) &processx_get_exit_status, 2 }, { "processx_signal", (DL_FUNC) &processx_signal, 3 }, { "processx_interrupt", (DL_FUNC) &processx_interrupt, 2 }, diff --git a/src/processx.h b/src/processx.h index 711a4e15..72f5eae4 100644 --- a/src/processx.h +++ b/src/processx.h @@ -49,6 +49,7 @@ SEXP processx_exec(SEXP command, SEXP args, SEXP pty, SEXP pty_options, SEXP tree_id); SEXP processx_wait(SEXP status, SEXP timeout, SEXP name); SEXP processx_is_alive(SEXP status, SEXP name); +SEXP processx_pty_close(SEXP status, SEXP name); SEXP processx_get_exit_status(SEXP status, SEXP name); SEXP processx_signal(SEXP status, SEXP signal, SEXP name); SEXP processx_interrupt(SEXP status, SEXP name); diff --git a/src/unix/processx.c b/src/unix/processx.c index 8434ca4c..38739d84 100644 --- a/src/unix/processx.c +++ b/src/unix/processx.c @@ -697,6 +697,11 @@ static void processx__wait_cleanup(void *ptr) { * 7. We keep polling until the timeout expires or the process finishes. */ +/* No-op on Unix: ConPTY is Windows-only. */ +SEXP processx_pty_close(SEXP status, SEXP name) { + return R_NilValue; +} + SEXP processx_wait(SEXP status, SEXP timeout, SEXP name) { processx_handle_t *handle = R_ExternalPtrAddr(status); const char *cname = isNull(name) ? "???" : CHAR(STRING_ELT(name, 0)); diff --git a/src/win/processx-win.h b/src/win/processx-win.h index b17c8537..5b0896db 100644 --- a/src/win/processx-win.h +++ b/src/win/processx-win.h @@ -14,6 +14,7 @@ typedef struct processx_handle_s { processx_connection_t *pipes[3]; int cleanup; double create_time; + void *ptycon; /* ConPTY handle (HPCON), NULL if not using PTY */ } processx_handle_t; int processx__utf8_to_utf16_alloc(const char* s, WCHAR** ws_ptr); @@ -30,6 +31,10 @@ int processx__create_pipe(void *id, HANDLE* parent_pipe_ptr, HANDLE* child_pipe_ int processx__create_input_pipe(void *id, HANDLE* parent_pipe_ptr, HANDLE* child_pipe_ptr, const char *cname); +processx_connection_t *processx__create_connection( + HANDLE pipe_handle, const char *membername, SEXP private, + const char *encoding, BOOL async); + void processx__handle_destroy(processx_handle_t *handle); void processx__stdio_noinherit(BYTE* buffer); diff --git a/src/win/processx.c b/src/win/processx.c index 3e31837a..bade696c 100644 --- a/src/win/processx.c +++ b/src/win/processx.c @@ -6,6 +6,85 @@ #include +/* ConPTY support (Windows 10 version 1809+). + We load the API dynamically so processx still loads on older Windows. */ + +#ifndef HPCON +typedef VOID* HPCON; +#endif + +#ifndef PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE +/* ProcThreadAttributeValue(22, FALSE, TRUE, FALSE) */ +#define PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE 0x00020016 +#endif + +/* Fallback type and constant definitions for older MinGW headers (rtools40). + These are Vista+ (0x0600) features; the 32-bit rtools40 toolchain sets + _WIN32_WINNT below that threshold so the declarations are absent. + We cannot use #ifndef because typedefs do not create preprocessor macros. */ +#ifndef EXTENDED_STARTUPINFO_PRESENT +#define EXTENDED_STARTUPINFO_PRESENT 0x00080000 +#endif + +#if !defined(_WIN32_WINNT) || (_WIN32_WINNT < 0x0600) +typedef struct _PROC_THREAD_ATTRIBUTE_LIST *PPROC_THREAD_ATTRIBUTE_LIST, + *LPPROC_THREAD_ATTRIBUTE_LIST; +typedef struct _STARTUPINFOEXW { + STARTUPINFOW StartupInfo; + LPPROC_THREAD_ATTRIBUTE_LIST lpAttributeList; +} STARTUPINFOEXW, *LPSTARTUPINFOEXW; +#endif + +typedef HRESULT (WINAPI *pfn_CreatePseudoConsole)( + COORD, HANDLE, HANDLE, DWORD, HPCON *); +typedef void (WINAPI *pfn_ClosePseudoConsole)(HPCON); +typedef BOOL (WINAPI *pfn_InitializeProcThreadAttributeList)( + LPPROC_THREAD_ATTRIBUTE_LIST, DWORD, DWORD, PSIZE_T); +typedef BOOL (WINAPI *pfn_UpdateProcThreadAttribute)( + LPPROC_THREAD_ATTRIBUTE_LIST, DWORD, DWORD_PTR, PVOID, SIZE_T, + PVOID, PSIZE_T); +typedef VOID (WINAPI *pfn_DeleteProcThreadAttributeList)( + LPPROC_THREAD_ATTRIBUTE_LIST); + +static pfn_CreatePseudoConsole processx__CreatePseudoConsole = NULL; +static pfn_ClosePseudoConsole processx__ClosePseudoConsole = NULL; +static pfn_InitializeProcThreadAttributeList + processx__InitializeProcThreadAttributeList = NULL; +static pfn_UpdateProcThreadAttribute + processx__UpdateProcThreadAttribute = NULL; +static pfn_DeleteProcThreadAttributeList + processx__DeleteProcThreadAttributeList = NULL; +/* 0 = unchecked, 1 = available, -1 = not available */ +static int processx__pty_api_state = 0; + +static int processx__load_pty_api(void) { + if (processx__pty_api_state != 0) return processx__pty_api_state == 1; + + HMODULE hK32 = GetModuleHandleW(L"kernel32.dll"); + if (hK32) { + processx__CreatePseudoConsole = + (pfn_CreatePseudoConsole) GetProcAddress(hK32, "CreatePseudoConsole"); + processx__ClosePseudoConsole = + (pfn_ClosePseudoConsole) GetProcAddress(hK32, "ClosePseudoConsole"); + processx__InitializeProcThreadAttributeList = + (pfn_InitializeProcThreadAttributeList) + GetProcAddress(hK32, "InitializeProcThreadAttributeList"); + processx__UpdateProcThreadAttribute = + (pfn_UpdateProcThreadAttribute) + GetProcAddress(hK32, "UpdateProcThreadAttribute"); + processx__DeleteProcThreadAttributeList = + (pfn_DeleteProcThreadAttributeList) + GetProcAddress(hK32, "DeleteProcThreadAttributeList"); + } + + processx__pty_api_state = + (processx__CreatePseudoConsole && processx__ClosePseudoConsole && + processx__InitializeProcThreadAttributeList && + processx__UpdateProcThreadAttribute && + processx__DeleteProcThreadAttributeList) ? 1 : -1; + return processx__pty_api_state == 1; +} + static HANDLE processx__global_job_handle = NULL; static void processx__init_global_job_handle(void) { @@ -862,6 +941,10 @@ SEXP processx__make_handle(SEXP private, int cleanup) { void processx__handle_destroy(processx_handle_t *handle) { if (!handle) return; if (handle->child_stdio_buffer) free(handle->child_stdio_buffer); + if (handle->ptycon && processx__ClosePseudoConsole) { + processx__ClosePseudoConsole((HPCON) handle->ptycon); + handle->ptycon = NULL; + } free(handle); } @@ -877,6 +960,7 @@ SEXP processx_exec(SEXP command, SEXP args, SEXP pty, SEXP pty_options, const char *ctree_id = CHAR(STRING_ELT(tree_id, 0)); int err = 0; + int cpty = LOGICAL(pty)[0]; WCHAR *path; WCHAR *application_path = NULL, *application = NULL, *arguments = NULL, *cenv = NULL, *cwd = NULL; @@ -957,12 +1041,6 @@ SEXP processx_exec(SEXP command, SEXP args, SEXP pty, SEXP pty_options, result = PROTECT(processx__make_handle(private, ccleanup)); handle = R_ExternalPtrAddr(result); - int inherit_std = 0; - err = processx__stdio_create(handle, connections, - &handle->child_stdio_buffer, private, - cencoding, ccommand, &inherit_std); - if (err) { R_THROW_SYSTEM_ERROR_CODE(err, "setup stdio for '%s'", ccommand); } - application_path = processx__search_path(application, cwd, path); /* If a UNC Path, then we try to flip the forward slashes, if any. @@ -977,11 +1055,263 @@ SEXP processx_exec(SEXP command, SEXP args, SEXP pty, SEXP pty_options, if (!application_path) { R_ClearExternalPtr(result); - processx__stdio_destroy(handle->child_stdio_buffer); free(handle); R_THROW_ERROR("Command '%s' not found", ccommand); } + /* ------------------------------------------------------------------ */ + /* ConPTY branch (Windows 10 1809+, pty = TRUE) */ + /* ------------------------------------------------------------------ */ + + if (cpty) { + + if (!processx__load_pty_api()) { + R_ClearExternalPtr(result); + free(handle); + R_THROW_ERROR("PTY is not supported on this version of Windows " + "(requires Windows 10 version 1809 or later)"); + } + + /* Extract PTY options (order matches default_pty_options(): echo, rows, cols) */ + int pty_rows = INTEGER(VECTOR_ELT(pty_options, 1))[0]; + int pty_cols = INTEGER(VECTOR_ELT(pty_options, 2))[0]; + + /* stdin pipe: anonymous, synchronous. + parent writes to ptyin_write; ConPTY reads from ptyin_read (hInput). */ + HANDLE ptyin_read = NULL, ptyin_write = NULL; + { + SECURITY_ATTRIBUTES sa; + sa.nLength = sizeof(sa); + sa.lpSecurityDescriptor = NULL; + sa.bInheritHandle = FALSE; + if (!CreatePipe(&ptyin_read, &ptyin_write, &sa, 0)) { + R_ClearExternalPtr(result); + free(handle); + R_THROW_SYSTEM_ERROR("create PTY stdin pipe for '%s'", ccommand); + } + } + + /* stdout pipe: named + overlapped so the parent can read asynchronously. + processx__create_pipe() gives: + parent (ptyout_read) – overlapped server end, parent reads from here; + child (ptyout_child) – sync client end, ConPTY writes here (hOutput). */ + HANDLE ptyout_read = NULL, ptyout_child = NULL; + err = processx__create_pipe(handle, &ptyout_read, &ptyout_child, ccommand); + if (err) { + CloseHandle(ptyin_read); + CloseHandle(ptyin_write); + R_ClearExternalPtr(result); + free(handle); + R_THROW_SYSTEM_ERROR_CODE(err, "create PTY stdout pipe for '%s'", ccommand); + } + + /* Create the pseudo-console. + Background R processes (callr, R CMD check, processx::run with captured + stdout) have their standard handles set to PIPE handles, not to the + console handles CONIN$/CONOUT$. PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE + can only intercept writes that go to CONSOLE handles; writes to pipe + handles bypass the ConPTY entirely, causing child output to be lost. + Fix: ensure the parent's standard handles are CONSOLE handles + (CONIN$/CONOUT$) at the point CreateProcessW is called. The kernel + copies the parent's standard handles into the child; PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE + then intercepts those console writes and routes them through the ConPTY. + We restore the original pipe handles immediately after CreateProcessW so + that R's own I/O is unaffected. + AllocConsole (if the process has no console yet) both makes + CreatePseudoConsole work on Windows 10 (which needs \Device\ConDrv) and + sets the standard handles to CONIN$/CONOUT$ in one step. When the + process already has a console, AllocConsole returns FALSE and we open + CONIN$/CONOUT$ explicitly to set the standard handles. The hidden + console from AllocConsole persists for the process lifetime (harmless; + subsequent AllocConsole calls return FALSE). */ + COORD pty_size; + pty_size.X = (SHORT) pty_cols; + pty_size.Y = (SHORT) pty_rows; + HPCON hPC; + /* Save originals and open/set console handles for CreateProcessW. */ + HANDLE saved_in = GetStdHandle(STD_INPUT_HANDLE); + HANDLE saved_out = GetStdHandle(STD_OUTPUT_HANDLE); + HANDLE saved_err = GetStdHandle(STD_ERROR_HANDLE); + HANDLE hConIn = INVALID_HANDLE_VALUE; + HANDLE hConOut = INVALID_HANDLE_VALUE; + if (AllocConsole()) { + /* AllocConsole already set std handles to CONIN$/CONOUT$. */ + HWND ach = GetConsoleWindow(); + if (ach) ShowWindow(ach, SW_HIDE); + } else { + /* Process already has a console (interactive session or a previous + AllocConsole call). Std handles may still be pipe handles; open + CONIN$/CONOUT$ explicitly and install them as std handles. */ + hConIn = CreateFileA("CONIN$", GENERIC_READ | GENERIC_WRITE, + FILE_SHARE_READ | FILE_SHARE_WRITE, + NULL, OPEN_EXISTING, 0, NULL); + hConOut = CreateFileA("CONOUT$", GENERIC_READ | GENERIC_WRITE, + FILE_SHARE_READ | FILE_SHARE_WRITE, + NULL, OPEN_EXISTING, 0, NULL); + if (hConIn != INVALID_HANDLE_VALUE) SetStdHandle(STD_INPUT_HANDLE, hConIn); + if (hConOut != INVALID_HANDLE_VALUE) { + SetStdHandle(STD_OUTPUT_HANDLE, hConOut); + SetStdHandle(STD_ERROR_HANDLE, hConOut); + } + } + + /* Helper: restores original std handles and closes any console handles + we opened. Called on every error path and after CreateProcessW. */ +#define PROCESSX_PTY_RESTORE_STD_HANDLES() do { \ + SetStdHandle(STD_INPUT_HANDLE, saved_in); \ + SetStdHandle(STD_OUTPUT_HANDLE, saved_out); \ + SetStdHandle(STD_ERROR_HANDLE, saved_err); \ + if (hConIn != INVALID_HANDLE_VALUE) CloseHandle(hConIn); \ + if (hConOut != INVALID_HANDLE_VALUE) CloseHandle(hConOut); \ + } while (0) + + HRESULT hr = processx__CreatePseudoConsole( + pty_size, ptyin_read, ptyout_child, 0, &hPC); + /* ConPTY made its own internal copies; close the pipe ends we passed in */ + CloseHandle(ptyin_read); + CloseHandle(ptyout_child); + if (FAILED(hr)) { + PROCESSX_PTY_RESTORE_STD_HANDLES(); + CloseHandle(ptyin_write); + CloseHandle(ptyout_read); + R_ClearExternalPtr(result); + free(handle); + R_THROW_ERROR("CreatePseudoConsole failed for '%s' (HRESULT 0x%08lx)", + ccommand, (unsigned long) hr); + } + + /* Build the process-thread attribute list containing the ConPTY handle */ + SIZE_T attrlist_size = 0; + processx__InitializeProcThreadAttributeList(NULL, 1, 0, &attrlist_size); + LPPROC_THREAD_ATTRIBUTE_LIST lpAttrList = + (LPPROC_THREAD_ATTRIBUTE_LIST) malloc(attrlist_size); + if (!lpAttrList) { + PROCESSX_PTY_RESTORE_STD_HANDLES(); + processx__ClosePseudoConsole(hPC); + CloseHandle(ptyin_write); + CloseHandle(ptyout_read); + R_ClearExternalPtr(result); + free(handle); + R_THROW_ERROR("Out of memory when creating subprocess '%s'", ccommand); + } + if (!processx__InitializeProcThreadAttributeList( + lpAttrList, 1, 0, &attrlist_size)) { + PROCESSX_PTY_RESTORE_STD_HANDLES(); + free(lpAttrList); + processx__ClosePseudoConsole(hPC); + CloseHandle(ptyin_write); + CloseHandle(ptyout_read); + R_ClearExternalPtr(result); + free(handle); + R_THROW_SYSTEM_ERROR( + "InitializeProcThreadAttributeList for '%s'", ccommand); + } + if (!processx__UpdateProcThreadAttribute( + lpAttrList, 0, PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE, + hPC, sizeof(HPCON), NULL, NULL)) { + PROCESSX_PTY_RESTORE_STD_HANDLES(); + processx__DeleteProcThreadAttributeList(lpAttrList); + free(lpAttrList); + processx__ClosePseudoConsole(hPC); + CloseHandle(ptyin_write); + CloseHandle(ptyout_read); + R_ClearExternalPtr(result); + free(handle); + R_THROW_SYSTEM_ERROR( + "UpdateProcThreadAttribute for '%s'", ccommand); + } + + STARTUPINFOEXW siEx; + memset(&siEx, 0, sizeof(siEx)); + siEx.StartupInfo.cb = sizeof(STARTUPINFOEXW); + /* Do NOT set STARTF_USESTDHANDLES: the parent's current std handles + (CONIN$/CONOUT$, set above) are copied into the child by CreateProcessW + and then intercepted by PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE. */ + siEx.StartupInfo.dwFlags = STARTF_USESHOWWINDOW; + siEx.StartupInfo.wShowWindow = + options.windows_hide ? SW_HIDE : SW_SHOWDEFAULT; + siEx.lpAttributeList = lpAttrList; + + /* ConPTY requires EXTENDED_STARTUPINFO_PRESENT. + Do NOT use CREATE_NO_WINDOW or DETACHED_PROCESS — either flag + prevents the child from attaching to the pseudo-console. */ + DWORD pty_flags = CREATE_UNICODE_ENVIRONMENT | CREATE_SUSPENDED + | EXTENDED_STARTUPINFO_PRESENT; + if (!ccleanup) pty_flags |= CREATE_NEW_PROCESS_GROUP; + + err = CreateProcessW( + /* lpApplicationName = */ application_path, + /* lpCommandLine = */ arguments, + /* lpProcessAttributes = */ NULL, + /* lpThreadAttributes = */ NULL, + /* bInheritHandles = */ FALSE, + /* dwCreationFlags = */ pty_flags, + /* lpEnvironment = */ cenv, + /* lpCurrentDirectory = */ cwd, + /* lpStartupInfo = */ (LPSTARTUPINFOW) &siEx, + /* lpProcessInformation = */ &info); + + processx__DeleteProcThreadAttributeList(lpAttrList); + free(lpAttrList); + + /* Restore R's original standard handles now that the child exists. */ + PROCESSX_PTY_RESTORE_STD_HANDLES(); +#undef PROCESSX_PTY_RESTORE_STD_HANDLES + + if (!err) { + processx__ClosePseudoConsole(hPC); + CloseHandle(ptyin_write); + CloseHandle(ptyout_read); + R_ClearExternalPtr(result); + free(handle); + UNPROTECT(1); + R_THROW_SYSTEM_ERROR("create process '%s'", ccommand); + } + + handle->ptycon = (void *) hPC; + handle->hProcess = info.hProcess; + handle->dwProcessId = info.dwProcessId; + handle->create_time = processx__create_time(handle->hProcess); + + if (ccleanup) { + if (!processx__global_job_handle) processx__init_global_job_handle(); + if (!AssignProcessToJobObject(processx__global_job_handle, + info.hProcess)) { + DWORD derr = GetLastError(); + if (derr != ERROR_ACCESS_DENIED) { + R_THROW_SYSTEM_ERROR_CODE(derr, "Assign to job object '%s'", + ccommand); + } + } + } + + dwerr = ResumeThread(info.hThread); + if (dwerr == (DWORD) -1) { + R_THROW_SYSTEM_ERROR("resume thread for '%s'", ccommand); + } + CloseHandle(info.hThread); + + /* Create parent-side I/O connections */ + handle->pipes[0] = processx__create_connection( + ptyin_write, "stdin_pipe", private, cencoding, FALSE); + handle->pipes[1] = processx__create_connection( + ptyout_read, "stdout_pipe", private, cencoding, TRUE); + handle->pipes[2] = NULL; + + UNPROTECT(1); + return result; + } + + /* ------------------------------------------------------------------ */ + /* Normal (non-PTY) branch */ + /* ------------------------------------------------------------------ */ + + int inherit_std = 0; + err = processx__stdio_create(handle, connections, + &handle->child_stdio_buffer, private, + cencoding, ccommand, &inherit_std); + if (err) { R_THROW_SYSTEM_ERROR_CODE(err, "setup stdio for '%s'", ccommand); } + startup.cb = sizeof(startup); startup.lpReserved = NULL; startup.lpDesktop = NULL; @@ -1088,6 +1418,24 @@ void processx__collect_exit_status(SEXP status, DWORD exitcode) { processx_handle_t *handle = R_ExternalPtrAddr(status); handle->exitcode = exitcode; handle->collected = 1; + /* Do NOT call ClosePseudoConsole() here. ConPTY processes the child's + writes asynchronously: by the time GetExitCodeProcess() reports that + the child has exited, conhost may not have finished writing the child's + last output to our pipe. Calling ClosePseudoConsole() too early + causes that buffered output to be silently discarded. + The caller is responsible for draining the stdout pipe first (via + processx_pty_close()) after polling until no more data arrives. */ +} + +/* Called from R after the stdout pipe has been drained: signals conhost + to flush any remaining terminal output and close the pipe (EOF). */ +SEXP processx_pty_close(SEXP status, SEXP name) { + processx_handle_t *handle = R_ExternalPtrAddr(status); + if (handle && handle->ptycon && processx__ClosePseudoConsole) { + processx__ClosePseudoConsole((HPCON) handle->ptycon); + handle->ptycon = NULL; + } + return R_NilValue; } SEXP processx_wait(SEXP status, SEXP timeout, SEXP name) { diff --git a/tests/testthat/_snaps/Darwin/process.md b/tests/testthat/_snaps/Darwin/process.md index c1932b36..623f68a1 100644 --- a/tests/testthat/_snaps/Darwin/process.md +++ b/tests/testthat/_snaps/Darwin/process.md @@ -5,7 +5,7 @@ Condition Error: ! ! Native call to `processx_exec` failed - Caused by error in `chain_call(c_processx_exec, command, c(command, args), pty, pty_options, ...` at initialize.R:162:: + Caused by error in `chain_call(c_processx_exec, command, c(command, args), pty, pty_options, ...` at initialize.R::: ! cannot start processx process '/' (system error 2, No such file or directory) @unix/processx.c:628 (processx_exec) # working directory does not exist @@ -15,6 +15,6 @@ Condition Error: ! ! Native call to `processx_exec` failed - Caused by error in `chain_call(c_processx_exec, command, c(command, args), pty, pty_options, ...` at initialize.R:162:: + Caused by error in `chain_call(c_processx_exec, command, c(command, args), pty, pty_options, ...` at initialize.R::: ! cannot start processx process '/px' (system error 2, No such file or directory) @unix/processx.c:628 (processx_exec) diff --git a/tests/testthat/_snaps/Darwin/run.md b/tests/testthat/_snaps/Darwin/run.md index 53882cd0..b0e068bc 100644 --- a/tests/testthat/_snaps/Darwin/run.md +++ b/tests/testthat/_snaps/Darwin/run.md @@ -5,6 +5,6 @@ Condition Error: ! ! Native call to `processx_exec` failed - Caused by error in `chain_call(c_processx_exec, command, c(command, args), pty, pty_options, ...` at initialize.R:162:: + Caused by error in `chain_call(c_processx_exec, command, c(command, args), pty, pty_options, ...` at initialize.R::: ! cannot start processx process '/px' (system error 2, No such file or directory) @unix/processx.c:628 (processx_exec) diff --git a/tests/testthat/_snaps/Linux/process.md b/tests/testthat/_snaps/Linux/process.md index c1932b36..623f68a1 100644 --- a/tests/testthat/_snaps/Linux/process.md +++ b/tests/testthat/_snaps/Linux/process.md @@ -5,7 +5,7 @@ Condition Error: ! ! Native call to `processx_exec` failed - Caused by error in `chain_call(c_processx_exec, command, c(command, args), pty, pty_options, ...` at initialize.R:162:: + Caused by error in `chain_call(c_processx_exec, command, c(command, args), pty, pty_options, ...` at initialize.R::: ! cannot start processx process '/' (system error 2, No such file or directory) @unix/processx.c:628 (processx_exec) # working directory does not exist @@ -15,6 +15,6 @@ Condition Error: ! ! Native call to `processx_exec` failed - Caused by error in `chain_call(c_processx_exec, command, c(command, args), pty, pty_options, ...` at initialize.R:162:: + Caused by error in `chain_call(c_processx_exec, command, c(command, args), pty, pty_options, ...` at initialize.R::: ! cannot start processx process '/px' (system error 2, No such file or directory) @unix/processx.c:628 (processx_exec) diff --git a/tests/testthat/_snaps/Linux/run.md b/tests/testthat/_snaps/Linux/run.md index 53882cd0..b0e068bc 100644 --- a/tests/testthat/_snaps/Linux/run.md +++ b/tests/testthat/_snaps/Linux/run.md @@ -5,6 +5,6 @@ Condition Error: ! ! Native call to `processx_exec` failed - Caused by error in `chain_call(c_processx_exec, command, c(command, args), pty, pty_options, ...` at initialize.R:162:: + Caused by error in `chain_call(c_processx_exec, command, c(command, args), pty, pty_options, ...` at initialize.R::: ! cannot start processx process '/px' (system error 2, No such file or directory) @unix/processx.c:628 (processx_exec) diff --git a/tests/testthat/_snaps/Windows/process.md b/tests/testthat/_snaps/Windows/process.md index f85822c4..2aec025e 100644 --- a/tests/testthat/_snaps/Windows/process.md +++ b/tests/testthat/_snaps/Windows/process.md @@ -5,8 +5,8 @@ Condition Error: ! ! Native call to `processx_exec` failed - Caused by error in `chain_call(c_processx_exec, command, c(command, args), pty, pty_options, ...` at initialize.R:162:: - ! Command '/' not found @win/processx.c:982 (processx_exec) + Caused by error in `chain_call(c_processx_exec, command, c(command, args), pty, pty_options, ...` at initialize.R::: + ! Command '/' not found @win/processx.c:1059 (processx_exec) # working directory does not exist @@ -15,7 +15,7 @@ Condition Error: ! ! Native call to `processx_exec` failed - Caused by error in `chain_call(c_processx_exec, command, c(command, args), pty, pty_options, ...` at initialize.R:162:: + Caused by error in `chain_call(c_processx_exec, command, c(command, args), pty, pty_options, ...` at initialize.R::: ! create process '/px' (system error 267, The directory name is invalid. - ) @win/processx.c:1040 (processx_exec) + ) @win/processx.c:1370 (processx_exec) diff --git a/tests/testthat/_snaps/Windows/run.md b/tests/testthat/_snaps/Windows/run.md index 6c27e464..b8e1ef31 100644 --- a/tests/testthat/_snaps/Windows/run.md +++ b/tests/testthat/_snaps/Windows/run.md @@ -5,7 +5,7 @@ Condition Error: ! ! Native call to `processx_exec` failed - Caused by error in `chain_call(c_processx_exec, command, c(command, args), pty, pty_options, ...` at initialize.R:162:: + Caused by error in `chain_call(c_processx_exec, command, c(command, args), pty, pty_options, ...` at initialize.R::: ! create process '/px' (system error 267, The directory name is invalid. - ) @win/processx.c:1040 (processx_exec) + ) @win/processx.c:1370 (processx_exec) diff --git a/tests/testthat/helper.R b/tests/testthat/helper.R index f82ee493..b77c6260 100644 --- a/tests/testthat/helper.R +++ b/tests/testthat/helper.R @@ -203,6 +203,10 @@ transform_column_number <- function(x) { sub("([.]R:[0-9]+:)[0-9]+", "\\1", x) } +transform_line_number <- function(x) { + sub("([.]R:[0-9]+:)[0-9]+", ".R::", x) +} + sysname <- function() { Sys.info()[["sysname"]] } diff --git a/tests/testthat/test-process.R b/tests/testthat/test-process.R index c591026a..4af0b132 100644 --- a/tests/testthat/test-process.R +++ b/tests/testthat/test-process.R @@ -19,7 +19,7 @@ test_that("non existing process", { expect_snapshot( error = TRUE, process$new(tempfile()), - transform = function(x) transform_column_number(transform_tempdir(x)), + transform = function(x) transform_line_number(transform_tempdir(x)), variant = sysname() ) ## This closes connections in finalizers @@ -79,7 +79,7 @@ test_that("working directory does not exist", { expect_snapshot( error = TRUE, process$new(px, wd = tempfile()), - transform = function(x) transform_column_number(transform_px(x)), + transform = function(x) transform_line_number(transform_px(x)), variant = sysname() ) ## This closes connections in finalizers diff --git a/tests/testthat/test-pty.R b/tests/testthat/test-pty.R index 3d49ac91..00169551 100644 --- a/tests/testthat/test-pty.R +++ b/tests/testthat/test-pty.R @@ -1,10 +1,48 @@ -test_that("fails in windows", { +test_that("pty works on windows", { skip_other_platforms("windows") - expect_error( - process$new("R", pty = TRUE), - "only implemented on Unix", - class = "error" - ) + skip_on_cran() + + px <- get_tool("px") + p <- process$new(px, c("outln", "hello", "sleep", "10"), pty = TRUE) + on.exit(p$kill(), add = TRUE) + expect_true(p$is_alive()) + + con <- p$get_output_connection() + out <- "" + repeat { + pr <- poll(list(con), 2000L)[[1]] + if (!identical(pr, "ready")) break + out <- paste0(out, p$read_output()) + if (grepl("hello", out, fixed = TRUE)) break + } + expect_match(out, "hello") +}) + +test_that("pty write_input works on windows", { + skip_other_platforms("windows") + skip_on_cran() + + # Use px cat instead of cmd.exe: px reads stdin and echoes to stdout, + # no banner to flush, no cmd.exe overhead or security-software interference. + px <- get_tool("px") + p <- process$new(px, c("cat", ""), pty = TRUE) + on.exit(p$kill(), add = TRUE) + expect_true(p$is_alive()) + + con <- p$get_output_connection() + + p$write_input("hello\r\n") + # Poll the stdout connection directly (not the full process) to avoid the + # poll_pipe-forces-timeout-0 effect when the process has already exited. + # Loop to skip VT init sequences that ConPTY writes before any child output. + out <- "" + repeat { + pr <- poll(list(con), 2000L)[[1]] + if (!identical(pr, "ready")) break + out <- paste0(out, p$read_output()) + if (grepl("hello", out, fixed = TRUE)) break + } + expect_match(out, "hello") }) test_that("pty works", { diff --git a/tests/testthat/test-run.R b/tests/testthat/test-run.R index a6b2e98b..310380bb 100644 --- a/tests/testthat/test-run.R +++ b/tests/testthat/test-run.R @@ -80,7 +80,7 @@ test_that("working directory does not exist", { expect_snapshot( error = TRUE, run(px, wd = tempfile()), - transform = function(x) transform_column_number(transform_px(x)), + transform = function(x) transform_line_number(transform_px(x)), variant = sysname() ) gc() @@ -230,11 +230,23 @@ test_that("binary=TRUE with stdout_callback receives raw chunks", { test_that("binary=TRUE errors with line callbacks", { px <- get_tool("px") - expect_snapshot(error = TRUE, - run(px, "out", encoding = "binary", stdout_line_callback = function(x, ...) x) + expect_snapshot( + error = TRUE, + run( + px, + "out", + encoding = "binary", + stdout_line_callback = function(x, ...) x + ) ) - expect_snapshot(error = TRUE, - run(px, "out", encoding = "binary", stderr_line_callback = function(x, ...) x) + expect_snapshot( + error = TRUE, + run( + px, + "out", + encoding = "binary", + stderr_line_callback = function(x, ...) x + ) ) }) @@ -248,6 +260,16 @@ test_that("pty=TRUE collects merged output in stdout", { expect_null(res$stderr) }) +test_that("pty=TRUE collects merged output in stdout (windows)", { + skip_other_platforms("windows") + skip_on_cran() + + px <- get_tool("px") + res <- run(px, c("outln", "hello pty"), pty = TRUE) + expect_match(res$stdout, "hello pty") + expect_null(res$stderr) +}) + test_that("pty=TRUE works with stdout_callback", { skip_other_platforms("unix") skip_on_os("solaris") @@ -255,7 +277,24 @@ test_that("pty=TRUE works with stdout_callback", { chunks <- character() res <- run( - "echo", "hello", + "echo", + "hello", + pty = TRUE, + stdout_callback = function(x, ...) chunks <<- c(chunks, x) + ) + expect_match(paste(chunks, collapse = ""), "hello") + expect_null(res$stderr) +}) + +test_that("pty=TRUE works with stdout_callback (windows)", { + skip_other_platforms("windows") + skip_on_cran() + + px <- get_tool("px") + chunks <- character() + res <- run( + px, + c("outln", "hello"), pty = TRUE, stdout_callback = function(x, ...) chunks <<- c(chunks, x) ) @@ -267,18 +306,19 @@ test_that("pty=TRUE errors on incompatible arguments", { skip_on_cran() expect_snapshot(error = TRUE, run("echo", pty = TRUE, stdout = NULL)) expect_snapshot(error = TRUE, run("echo", pty = TRUE, stderr = NULL)) - expect_snapshot(error = TRUE, + expect_snapshot( + error = TRUE, run("echo", pty = TRUE, stderr_to_stdout = TRUE) ) - expect_snapshot(error = TRUE, + expect_snapshot( + error = TRUE, run("echo", pty = TRUE, stderr_callback = function(x, ...) x) ) - expect_snapshot(error = TRUE, + expect_snapshot( + error = TRUE, run("echo", pty = TRUE, stderr_line_callback = function(x, ...) x) ) - expect_snapshot(error = TRUE, - run("echo", pty = TRUE, stdin = "|") - ) + expect_snapshot(error = TRUE, run("echo", pty = TRUE, stdin = "|")) }) test_that("pty=TRUE with file stdin feeds content to the process", {