diff --git a/binding.gyp b/binding.gyp index 5f63978b07ab50aaf7523219a2170ec737a6b5db..bbd9e06136e8922f40b5779e35d4fc835f1479ab 100644 --- a/binding.gyp +++ b/binding.gyp @@ -1,13 +1,18 @@ { 'target_defaults': { 'dependencies': [ - " #include #include +#include #include +#include #include #include @@ -47,6 +49,39 @@ #include #endif +/* Orca: glibc 2.32-2.34 relocated pthread_sigmask/openpty/forkpty into libc + * under new symbol versions, so building on a newer glibc produces references + * (GLIBC_2.32/2.34) absent on Ubuntu 20.04 (glibc 2.31) and the app fails to + * launch. Pin these to the pre-merge version glibc still ships as a compat + * alias; the binding.gyp ldflags force libutil/libpthread into DT_NEEDED so + * those aliases are actually loaded on the target. */ +#if defined(__linux__) +# if defined(__x86_64__) +# define ORCA_GLIBC_COMPAT_VERSION "GLIBC_2.2.5" +# elif defined(__aarch64__) +# define ORCA_GLIBC_COMPAT_VERSION "GLIBC_2.17" +# endif +# ifdef ORCA_GLIBC_COMPAT_VERSION +__asm__(".symver openpty,openpty@" ORCA_GLIBC_COMPAT_VERSION); +__asm__(".symver forkpty,forkpty@" ORCA_GLIBC_COMPAT_VERSION); +__asm__(".symver pthread_sigmask,pthread_sigmask@" ORCA_GLIBC_COMPAT_VERSION); +/* glibc 2.42 versioned cfset*speed to take literal rates (B38400 == 38400U); + * the pinned compat versions accept only the legacy CBAUD code (named + * __B38400 in 2.42+ headers) and return EINVAL otherwise (#19420). */ +__asm__(".symver cfsetispeed,cfsetispeed@" ORCA_GLIBC_COMPAT_VERSION); +__asm__(".symver cfsetospeed,cfsetospeed@" ORCA_GLIBC_COMPAT_VERSION); +# ifdef __B38400 +# define ORCA_B38400 __B38400 +# else +# define ORCA_B38400 B38400 +# endif +static_assert(ORCA_B38400 == 0000017, "pinned cfset*speed take legacy CBAUD codes"); +# endif +#endif +#ifndef ORCA_B38400 +# define ORCA_B38400 B38400 +#endif + /* Some platforms name VWERASE and VDISCARD differently */ #if !defined(VWERASE) && defined(VWERSE) #define VWERASE VWERSE @@ -228,6 +263,9 @@ Napi::Value PtyGetProc(const Napi::CallbackInfo& info); static int pty_nonblock(int); +static int +pty_cloexec(int); + #if defined(__APPLE__) static char * pty_getproc(int); @@ -237,13 +275,23 @@ pty_getproc(int, char *); #endif #if defined(__APPLE__) || defined(__OpenBSD__) +struct pty_spawn_error { + const char* step; + int errnum; + std::string detail_name; + std::string detail_value; +}; + +static std::string +pty_format_spawn_error(const pty_spawn_error&); + static void pty_posix_spawn(char** argv, char** env, const struct termios *termp, const struct winsize *winp, int* master, pid_t* pid, - int* err); + pty_spawn_error* err); #endif struct DelBuf { @@ -345,8 +393,8 @@ Napi::Value PtyFork(const Napi::CallbackInfo& info) { term->c_cc[VSTATUS] = 20; #endif - cfsetispeed(term, B38400); - cfsetospeed(term, B38400); + cfsetispeed(term, ORCA_B38400); + cfsetospeed(term, ORCA_B38400); // helperPath std::string helper_path = info[9].As(); @@ -367,14 +415,18 @@ Napi::Value PtyFork(const Napi::CallbackInfo& info) { argv[i + 3] = strdup(arg.c_str()); } - int err = -1; - pty_posix_spawn(argv, env, term, &winp, &master, &pid, &err); - if (err != 0) { - throw Napi::Error::New(napiEnv, "posix_spawnp failed."); + pty_spawn_error spawn_error = { NULL, 0, "", "" }; + pty_posix_spawn(argv, env, term, &winp, &master, &pid, &spawn_error); + if (spawn_error.errnum != 0) { + std::string spawn_message = pty_format_spawn_error(spawn_error); + throw Napi::Error::New(napiEnv, spawn_message); } if (pty_nonblock(master) == -1) { throw Napi::Error::New(napiEnv, "Could not set master fd to nonblocking."); } + if (pty_cloexec(master) == -1) { + throw Napi::Error::New(napiEnv, "Could not set master fd to close-on-exec."); + } #else int argc = argv_.Length(); int argl = argc + 2; @@ -445,6 +497,9 @@ Napi::Value PtyFork(const Napi::CallbackInfo& info) { if (pty_nonblock(master) == -1) { throw Napi::Error::New(napiEnv, "Could not set master fd to nonblocking."); } + if (pty_cloexec(master) == -1) { + throw Napi::Error::New(napiEnv, "Could not set master fd to close-on-exec."); + } } #endif @@ -586,6 +641,23 @@ pty_nonblock(int fd) { return fcntl(fd, F_SETFL, flags | O_NONBLOCK); } +/** + * Orca: close-on-exec FD + * + * forkpty()/posix_openpt() have no atomic O_CLOEXEC, so a master left without + * FD_CLOEXEC is inherited by every later child of this process -- including + * later pty children -- which keeps its /dev/pts device and buffers alive long + * after its own session ends (#8362). + */ + +static int +pty_cloexec(int fd) { + int flags = fcntl(fd, F_GETFD); + if (flags == -1) return -1; + if (flags & FD_CLOEXEC) return 0; + return fcntl(fd, F_SETFD, flags | FD_CLOEXEC); +} + /** * pty_getproc * Taken from tmux. @@ -684,15 +756,73 @@ pty_getproc(int fd, char *tty) { #endif #if defined(__APPLE__) +static const char* +pty_errno_name(int errnum) { + switch (errnum) { + case E2BIG: return "E2BIG"; + case EACCES: return "EACCES"; + case EAGAIN: return "EAGAIN"; + case EMFILE: return "EMFILE"; + case ENFILE: return "ENFILE"; + case ENOENT: return "ENOENT"; + case ENOMEM: return "ENOMEM"; + default: return "errno"; + } +} + +static void +pty_set_spawn_error(pty_spawn_error* err, + const char* step, + int errnum, + const char* detail_name = NULL, + const char* detail_value = NULL) { + err->step = step; + err->errnum = errnum; + err->detail_name = detail_name ? detail_name : ""; + err->detail_value = detail_value ? detail_value : ""; +} + +static std::string +pty_format_spawn_error(const pty_spawn_error& err) { + char errno_buf[64]; + snprintf(errno_buf, sizeof(errno_buf), "%d", err.errnum); + + std::string message = "node-pty: "; + message += err.step ? err.step : "unknown"; + message += " failed: "; + message += pty_errno_name(err.errnum); + message += " (errno "; + message += errno_buf; + message += ", "; + message += strerror(err.errnum); + message += ")"; + + if (!err.detail_name.empty()) { + message += " - "; + message += err.detail_name; + message += "='"; + message += err.detail_value; + message += "'"; + } + + return message; +} + static void pty_posix_spawn(char** argv, char** env, const struct termios *termp, const struct winsize *winp, int* master, pid_t* pid, - int* err) { - int low_fds[3]; + pty_spawn_error* err) { + int low_fds[3] = {-1, -1, -1}; size_t count = 0; + int res = -1; + int slave = -1; + posix_spawn_file_actions_t acts; + bool acts_initialized = false; + posix_spawnattr_t attrs; + bool attrs_initialized = false; for (; count < 3; count++) { low_fds[count] = posix_openpt(O_RDWR); @@ -706,80 +836,118 @@ pty_posix_spawn(char** argv, char** env, POSIX_SPAWN_SETSID; *master = posix_openpt(O_RDWR); if (*master == -1) { - return; + pty_set_spawn_error(err, "posix_openpt", errno); + goto done; } - int res = grantpt(*master) || unlockpt(*master); + res = grantpt(*master); if (res == -1) { - return; + pty_set_spawn_error(err, "grantpt", errno); + goto done; + } + + res = unlockpt(*master); + if (res == -1) { + pty_set_spawn_error(err, "unlockpt", errno); + goto done; } // Use TIOCPTYGNAME instead of ptsname() to avoid threading problems. - int slave; char slave_pty_name[128]; res = ioctl(*master, TIOCPTYGNAME, slave_pty_name); if (res == -1) { - return; + pty_set_spawn_error(err, "ioctl_TIOCPTYGNAME", errno); + goto done; } slave = open(slave_pty_name, O_RDWR | O_NOCTTY); if (slave == -1) { - return; + pty_set_spawn_error(err, "open_slave", errno, "slave", slave_pty_name); + goto done; } if (termp) { res = tcsetattr(slave, TCSANOW, termp); if (res == -1) { - return; + pty_set_spawn_error(err, "tcsetattr", errno, "slave", slave_pty_name); + goto done; }; } if (winp) { res = ioctl(slave, TIOCSWINSZ, winp); if (res == -1) { - return; + pty_set_spawn_error(err, "ioctl_TIOCSWINSZ", errno, "slave", slave_pty_name); + goto done; } } - posix_spawn_file_actions_t acts; - posix_spawn_file_actions_init(&acts); + res = posix_spawn_file_actions_init(&acts); + if (res != 0) { + pty_set_spawn_error(err, "posix_spawn_file_actions_init", res); + goto done; + } + acts_initialized = true; posix_spawn_file_actions_adddup2(&acts, slave, STDIN_FILENO); posix_spawn_file_actions_adddup2(&acts, slave, STDOUT_FILENO); posix_spawn_file_actions_adddup2(&acts, slave, STDERR_FILENO); posix_spawn_file_actions_addclose(&acts, slave); posix_spawn_file_actions_addclose(&acts, *master); - posix_spawnattr_t attrs; - posix_spawnattr_init(&attrs); - *err = posix_spawnattr_setflags(&attrs, flags); - if (*err != 0) { + res = posix_spawnattr_init(&attrs); + if (res != 0) { + pty_set_spawn_error(err, "posix_spawnattr_init", res); + goto done; + } + attrs_initialized = true; + res = posix_spawnattr_setflags(&attrs, flags); + if (res != 0) { + pty_set_spawn_error(err, "posix_spawnattr_setflags", res); goto done; } sigset_t signal_set; /* Reset all signal the child to their default behavior */ sigfillset(&signal_set); - *err = posix_spawnattr_setsigdefault(&attrs, &signal_set); - if (*err != 0) { + res = posix_spawnattr_setsigdefault(&attrs, &signal_set); + if (res != 0) { + pty_set_spawn_error(err, "posix_spawnattr_setsigdefault", res); goto done; } /* Reset the signal mask for all signals */ sigemptyset(&signal_set); - *err = posix_spawnattr_setsigmask(&attrs, &signal_set); - if (*err != 0) { + res = posix_spawnattr_setsigmask(&attrs, &signal_set); + if (res != 0) { + pty_set_spawn_error(err, "posix_spawnattr_setsigmask", res); goto done; } do - *err = posix_spawn(pid, argv[0], &acts, &attrs, argv, env); - while (*err == EINTR); + res = posix_spawn(pid, argv[0], &acts, &attrs, argv, env); + while (res == EINTR); + if (res != 0) { + pty_set_spawn_error(err, "posix_spawn", res, "helper", argv[0]); + } done: - posix_spawn_file_actions_destroy(&acts); - posix_spawnattr_destroy(&attrs); + if (acts_initialized) { + posix_spawn_file_actions_destroy(&acts); + } + if (attrs_initialized) { + posix_spawnattr_destroy(&attrs); + } + if (slave != -1) { + close(slave); + } + if (err->errnum != 0 && *master != -1) { + close(*master); + *master = -1; + } - for (; count > 0; count--) { - close(low_fds[count]); + for (size_t i = 0; i <= count && i < 3; i++) { + if (low_fds[i] != -1) { + close(low_fds[i]); + } } } #endif diff --git a/src/unixTerminal.ts b/src/unixTerminal.ts index 98733dc0cd752b554bd94e45904ca341ad141bba..3dd5ad9b3124dbd5ba7f76679b26650dad503ec6 100644 --- a/src/unixTerminal.ts +++ b/src/unixTerminal.ts @@ -306,6 +306,14 @@ interface IWriteTask { offset: number; } +/** + * Orca: upstream retries EAGAIN with `setImmediate`, which re-attempts within microseconds + * and pins a core on the daemon thread when a reader stops draining. 1ms keeps ~97% of that + * CPU saving; longer delays cost 2-3x delivery time to a reader that drains in bursts (an + * agent). Re-measure both before changing. + */ +const EAGAIN_RETRY_DELAY_MS = 1; + /** * A custom write stream that writes directly to a file descriptor with proper * handling of backpressure and errors. This avoids some event loop exhaustion @@ -314,20 +322,28 @@ interface IWriteTask { class CustomWriteStream implements IDisposable { private readonly _writeQueue: IWriteTask[] = []; - private _writeImmediate: NodeJS.Immediate | undefined; + private _writeImmediate: NodeJS.Timeout | undefined; constructor( - private readonly _fd: number, + private _fd: number, private readonly _encoding: BufferEncoding ) { } dispose(): void { - clearImmediate(this._writeImmediate); + clearTimeout(this._writeImmediate); this._writeImmediate = undefined; + // Orca: retire this stream's own copy of the master fd and drop what has + // not shipped, so nothing queued here reaches a reused descriptor. + this._fd = -1; + this._writeQueue.length = 0; } write(data: string | Buffer): void { + if (this._fd < 0) { + return; + } + // Writes are put in a queue and processed asynchronously in order to handle // backpressure from the kernel buffer. const buffer = typeof data === 'string' @@ -345,7 +361,8 @@ class CustomWriteStream implements IDisposable { private _processWriteQueue(): void { this._writeImmediate = undefined; - if (this._writeQueue.length === 0) { + // Orca: an in-flight fs.write can re-enter here after dispose(). + if (this._fd < 0 || this._writeQueue.length === 0) { return; } @@ -357,9 +374,9 @@ class CustomWriteStream implements IDisposable { fs.write(this._fd, task.buffer, task.offset, (err, written) => { if (err) { if ('code' in err && err.code === 'EAGAIN') { - // `setImmediate` is used to yield to the event loop and re-attempt - // the write later. - this._writeImmediate = setImmediate(() => this._processWriteQueue()); + // Paced, not `setImmediate`: a stalled reader keeps this branch EAGAIN-ing, and an + // immediate re-attempt turns the retry into a busy-loop on the daemon thread. + this._writeImmediate = setTimeout(() => this._processWriteQueue(), EAGAIN_RETRY_DELAY_MS); } else { // Stop processing immediately on unexpected error and log this._writeQueue.length = 0; diff --git a/src/utils.ts b/src/utils.ts index 81a70c77a18e25c50b26f0d80aa0e8d8c2672805..4bc8d760fe00e208f211570930cd163b77901810 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -10,8 +10,8 @@ export function assign(target: any, ...sources: any[]): any { export function loadNativeModule(name: string): {dir: string, module: any} { - // Check build, debug, and then prebuilds. - const dirs = ['build/Release', 'build/Debug', `prebuilds/${process.platform}-${process.arch}`]; + // Check Orca's per-ABI build (published into package dirs shared across worktrees), build, debug, and then prebuilds. + const dirs = [`build/orca-napi-${process.platform}-${process.arch}`, 'build/Release', 'build/Debug', `prebuilds/${process.platform}-${process.arch}`]; // Check relative to the parent dir for unbundled and then the current dir for bundled const relative = ['..', '.']; let lastError: unknown; diff --git a/src/win/conpty.cc b/src/win/conpty.cc index 7b286d3d64..5239ba4e40 100644 --- a/src/win/conpty.cc +++ b/src/win/conpty.cc @@ -18,6 +18,8 @@ #include #include #include +#include +#include #include #include #include @@ -44,15 +46,39 @@ struct pty_baton { HANDLE hOut; HPCON hpc; - HANDLE hShell; + HANDLE hShell = nullptr; + // Orca: the shell's pid, captured at spawn. The ownership guard compares + // against this rather than calling GetProcessId(hShell), because the exit + // watcher closes hShell on another thread -- reading it there is an + // invalid-handle operation, and under strict handle checks that is fatal. + DWORD shellPid = 0; + + // Orca: job object owning this pty's whole process tree. Null when the OS + // refused to create or assign one (an outer job without breakaway rights), + // in which case callers fall back to their pre-job behaviour. + HANDLE hJob = nullptr; + bool allowJobBreakaway = true; + + // Orca: teardown needs BOTH the shell's death and an explicit kill() before + // the baton can be freed, so each side records that it has run. Whichever + // arrives second frees it. Freeing on the shell's death alone -- what this + // file did before -- destroyed the only record of `hpc` while + // ClosePseudoConsole was still owed, which is why a self-exiting shell + // leaked its pseudoconsole and the console host it reaps (#18601 / F24). + bool shellExited = false; + bool consoleClosed = false; pty_baton(int _id, HANDLE _hIn, HANDLE _hOut, HPCON _hpc) : id(_id), hIn(_hIn), hOut(_hOut), hpc(_hpc) {}; }; static std::vector> ptyHandles; +// Orca: every table access shares the exit watcher's lock; handles can be recycled. +static std::mutex ptyJobMutex; +// Orca: 0 = no per-terminal ceiling (the default); set once at daemon start. +static std::atomic ptyActiveProcessLimit{0}; static volatile LONG ptyCounter; -static pty_baton* get_pty_baton(int id) { +static pty_baton* get_pty_baton_locked(int id) { auto it = std::find_if(ptyHandles.begin(), ptyHandles.end(), [id](const auto& ptyHandle) { return ptyHandle->id == id; }); @@ -62,7 +88,7 @@ static pty_baton* get_pty_baton(int id) { return nullptr; } -static bool remove_pty_baton(int id) { +static bool remove_pty_baton_locked(int id) { auto it = std::remove_if(ptyHandles.begin(), ptyHandles.end(), [id](const auto& ptyHandle) { return ptyHandle->id == id; }); @@ -102,8 +128,31 @@ void SetupExitCallback(Napi::Env env, Napi::Function cb, pty_baton* baton) { // Get process exit code. GetExitCodeProcess(baton->hShell, (LPDWORD)(&exit_event->exit_code)); // Clean up handles - CloseHandle(baton->hShell); - assert(remove_pty_baton(baton->id)); + // Orca: release the job once the shell is gone. Without kill-on-close this + // only frees the handle -- anything the user backgrounded is orphaned, as + // it was before this patch. + { + std::lock_guard guard(ptyJobMutex); + CloseHandle(baton->hShell); + baton->hShell = nullptr; + if (baton->hJob != nullptr) { + CloseHandle(baton->hJob); + baton->hJob = nullptr; + } + // Why inside the lock: erasing frees the baton the job accessors hold a + // pointer to. Note remove_pty_baton must not be an assert() argument -- + // NDEBUG would compile the call away and leak every baton. + baton->shellExited = true; + if (baton->consoleClosed) { + const bool removed = remove_pty_baton_locked(baton->id); + assert(removed); + (void)removed; + } + // Else PtyKill has not run yet and still owns hpc. It frees the baton. + } + // Why the lock ends here: BlockingCall below waits on the JS thread, and the + // JS thread can be waiting on ptyJobMutex inside PtyTerminateJob. Holding + // the lock across it deadlocks. Do not widen this scope. auto status = tsfn.BlockingCall(exit_event, callback); // In main thread switch (status) { @@ -242,6 +291,20 @@ HRESULT CreateNamedPipesAndPseudoConsole(const Napi::CallbackInfo& info, return HRESULT_FROM_WIN32(GetLastError()); } +// Cygwin and MSYS request breakaway for every child whenever the job allows it, +// so their shells need one that does not. The runtime DLL on the exe's search +// path is the signal; Git for Windows ships bash.exe in bin\ beside usr\bin\. +static bool usesCygwinRuntime(const std::wstring& shellpath) { + const size_t separator = shellpath.find_last_of(L"\\/"); + if (separator == std::wstring::npos) return false; + const std::wstring directory = shellpath.substr(0, separator + 1); + for (const wchar_t* dll : {L"msys-2.0.dll", L"cygwin1.dll"}) { + if (path_util::file_exists(directory + dll) || + path_util::file_exists(directory + L"..\\usr\\bin\\" + dll)) return true; + } + return false; +} + static Napi::Value PtyStartProcess(const Napi::CallbackInfo& info) { Napi::Env env(info.Env()); Napi::HandleScope scope(env); @@ -301,8 +364,10 @@ static Napi::Value PtyStartProcess(const Napi::CallbackInfo& info) { // We were able to instantiate a conpty const int ptyId = InterlockedIncrement(&ptyCounter); marshal.Set("pty", Napi::Number::New(env, ptyId)); - ptyHandles.emplace_back( - std::make_unique(ptyId, hIn, hOut, hpc)); + auto baton = std::make_unique(ptyId, hIn, hOut, hpc); + baton->allowJobBreakaway = !usesCygwinRuntime(shellpath); + std::lock_guard guard(ptyJobMutex); + ptyHandles.emplace_back(std::move(baton)); } else { throw Napi::Error::New(env, "Cannot launch conpty"); } @@ -350,11 +415,15 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) { const bool useConptyDll = info[4].As().Value(); Napi::Function exitCallback = info[5].As(); - // Fetch pty handle from ID and start process - pty_baton* handle = get_pty_baton(id); - if (!handle) { - throw Napi::Error::New(env, "Invalid pty handle"); + pty_baton* handle; + { + std::lock_guard guard(ptyJobMutex); + handle = get_pty_baton_locked(id); + if (!handle || handle->consoleClosed) { + throw Napi::Error::New(env, "Invalid pty handle"); + } } + // No watcher exists for this baton until SetupExitCallback below; pipe waits stay unlocked. // Prepare command line std::unique_ptr mutableCommandline = std::make_unique(cmdline.length() + 1); @@ -409,6 +478,15 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) { throw errorWithCode(info, "UpdateProcThreadAttribute failed"); } + // Orca: resolve the DLL BEFORE creating anything. It throws when conpty.dll + // is missing -- a real state, and one this branch hit during development -- + // and every throw between CreateProcessW and SetupExitCallback leaks the job, + // process and thread handles AND leaves an untracked shell tree running, + // once per attempt. Validating first means the only throw after creation is + // the resume failure, which cleans up after itself. + HANDLE hLibrary = LoadConptyDll(info, useConptyDll); + bool fLoadedDll = hLibrary != nullptr; + PROCESS_INFORMATION piClient{}; fSuccess = !!CreateProcessW( nullptr, @@ -416,7 +494,10 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) { nullptr, // lpProcessAttributes nullptr, // lpThreadAttributes false, // bInheritHandles VERY IMPORTANT that this is false - EXTENDED_STARTUPINFO_PRESENT | CREATE_UNICODE_ENVIRONMENT, // dwCreationFlags + // Orca: CREATE_SUSPENDED so the shell is inside its job before it can + // spawn anything. Assigning after the fact leaves a window in which a + // fast child escapes the job and outlives the pane. + EXTENDED_STARTUPINFO_PRESENT | CREATE_UNICODE_ENVIRONMENT | CREATE_SUSPENDED, // dwCreationFlags envArg, // lpEnvironment mutableCwd.get(), // lpCurrentDirectory &siEx.StartupInfo, // lpStartupInfo @@ -426,8 +507,54 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) { throw errorWithCode(info, "Cannot create process"); } - HANDLE hLibrary = LoadConptyDll(info, useConptyDll); - bool fLoadedDll = hLibrary != nullptr; + // Orca: own the tree with a handle instead of inferring it later from a + // parent-pid walk. A pid walk cannot survive pid reuse and cannot see a + // descendant that reparented, which is why detached agent children outlived + // their pane and held the worktree directory open. + // + // Deliberately WITHOUT JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE. Measured on + // Windows 11: with that flag, closing the handle when the shell exits also + // kills whatever the user left running, so typing `exit` in a pane reaped a + // `start /b` server that used to survive. This job exists to make an + // EXPLICIT teardown exact, not to redefine what a clean exit means. + HANDLE hJob = CreateJobObjectW(nullptr, nullptr); + if (hJob != nullptr) { + // Native shells retain explicit breakaway for installers and updaters. + // Cygwin/MSYS shells take it automatically for ordinary children whenever + // this flag is present, so they get strict per-PTY membership instead. + // Explicit breakaway requests inside such a pane are consequently denied; + // ordinary backgrounding and clean shell exit remain supported. + JOBOBJECT_EXTENDED_LIMIT_INFORMATION jobLimits{}; + jobLimits.BasicLimitInformation.LimitFlags = + handle->allowJobBreakaway ? JOB_OBJECT_LIMIT_BREAKAWAY_OK : 0; + // Orca: an opt-in ceiling; forks past it fail inside this terminal's tree only. + const uint32_t activeProcessLimit = ptyActiveProcessLimit.load(); + if (activeProcessLimit > 0) { + jobLimits.BasicLimitInformation.LimitFlags |= JOB_OBJECT_LIMIT_ACTIVE_PROCESS; + jobLimits.BasicLimitInformation.ActiveProcessLimit = activeProcessLimit; + } + if (!SetInformationJobObject(hJob, JobObjectExtendedLimitInformation, &jobLimits, sizeof(jobLimits)) || + !AssignProcessToJobObject(hJob, piClient.hProcess)) { + // Why tolerate failure: an outer job without JOB_OBJECT_LIMIT_BREAKAWAY_OK + // (some EDR and container hosts) refuses the assignment. The pty must + // still start; ownership just degrades to the older best-effort path. + CloseHandle(hJob); + hJob = nullptr; + } + } + // Safe to run now: either it is in the job, or we accepted that it is not. + if (ResumeThread(piClient.hThread) == static_cast(-1)) { + // Why fatal: a shell left suspended produces a pane that never prints and + // never exits, which is far harder to diagnose than a failed spawn. + if (hJob != nullptr) { + CloseHandle(hJob); + } + TerminateProcess(piClient.hProcess, 1); + CloseHandle(piClient.hProcess); + CloseHandle(piClient.hThread); + throw errorWithCode(info, "Cannot resume process"); + } + if (useConptyDll && fLoadedDll) { PFNRELEASEPSEUDOCONSOLE const pfnReleasePseudoConsole = (PFNRELEASEPSEUDOCONSOLE)GetProcAddress( @@ -438,8 +565,12 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) { } } - // Update handle - handle->hShell = piClient.hProcess; + { + std::lock_guard guard(ptyJobMutex); + handle->hShell = piClient.hProcess; + handle->shellPid = piClient.dwProcessId; + handle->hJob = hJob; + } // Close the thread handle to avoid resource leak CloseHandle(piClient.hThread); @@ -472,9 +603,10 @@ static Napi::Value PtyResize(const Napi::CallbackInfo& info) { SHORT rows = static_cast(info[2].As().Uint32Value()); const bool useConptyDll = info[3].As().Value(); - const pty_baton* handle = get_pty_baton(id); + std::lock_guard guard(ptyJobMutex); + const pty_baton* handle = get_pty_baton_locked(id); - if (handle != nullptr) { + if (handle != nullptr && !handle->consoleClosed) { HANDLE hLibrary = LoadConptyDll(info, useConptyDll); bool fLoadedDll = hLibrary != nullptr; if (fLoadedDll) @@ -513,9 +645,10 @@ static Napi::Value PtyClear(const Napi::CallbackInfo& info) { return env.Undefined(); } - const pty_baton* handle = get_pty_baton(id); + std::lock_guard guard(ptyJobMutex); + const pty_baton* handle = get_pty_baton_locked(id); - if (handle != nullptr) { + if (handle != nullptr && !handle->consoleClosed) { HANDLE hLibrary = LoadConptyDll(info, useConptyDll); bool fLoadedDll = hLibrary != nullptr; if (fLoadedDll) @@ -544,29 +677,229 @@ static Napi::Value PtyKill(const Napi::CallbackInfo& info) { int id = info[0].As().Int32Value(); const bool useConptyDll = info[1].As().Value(); - const pty_baton* handle = get_pty_baton(id); + // Orca: resolve the DLL BEFORE touching any baton state, for the same reason + // PtyConnect does it before creating anything. LoadConptyDll throws when + // conpty.dll is missing, and a throw after consoleClosed was set would strand + // the pseudoconsole permanently: the retry would find the work already + // claimed and do nothing. Only the useConptyDll path can throw here; the + // other returns kernel32. + HANDLE hLibrary = LoadConptyDll(info, useConptyDll); + PFNCLOSEPSEUDOCONSOLE pfnClosePseudoConsole = nullptr; + if (hLibrary != nullptr) { + pfnClosePseudoConsole = (PFNCLOSEPSEUDOCONSOLE)GetProcAddress( + (HMODULE)hLibrary, + useConptyDll ? "ConptyClosePseudoConsole" : "ClosePseudoConsole"); + } - if (handle != nullptr) { - HANDLE hLibrary = LoadConptyDll(info, useConptyDll); - bool fLoadedDll = hLibrary != nullptr; - if (fLoadedDll) - { - PFNCLOSEPSEUDOCONSOLE const pfnClosePseudoConsole = (PFNCLOSEPSEUDOCONSOLE)GetProcAddress( - (HMODULE)hLibrary, - useConptyDll ? "ConptyClosePseudoConsole" : "ClosePseudoConsole"); - if (pfnClosePseudoConsole) - { - pfnClosePseudoConsole(handle->hpc); + // Orca: the baton now outlives the shell, so this runs on a self-exited pty + // too -- that is the whole point. Take what we need under the lock: the + // watcher thread nulls hShell the moment the shell dies, and TerminateProcess + // on a handle it just closed is an invalid-handle operation. Duplicating + // rather than reordering keeps upstream's close-then-terminate sequence. + HPCON hpc = nullptr; + HANDLE hShellDup = nullptr; + bool owed = false; + { + std::lock_guard guard(ptyJobMutex); + pty_baton* handle = get_pty_baton_locked(id); + // Why the consoleClosed check: a second kill() would otherwise close the + // same pseudoconsole twice. Upstream relied on the baton being gone. + if (handle != nullptr && !handle->consoleClosed) { + hpc = handle->hpc; + owed = true; + handle->consoleClosed = true; + // Null hShell means a self-exited pty, where there is nothing to kill. + if (useConptyDll && handle->hShell != nullptr) { + if (!DuplicateHandle(GetCurrentProcess(), handle->hShell, GetCurrentProcess(), + &hShellDup, 0, FALSE, DUPLICATE_SAME_ACCESS)) { + // Why terminate here instead of skipping: a failed duplication leaves + // hShellDup null, which is indistinguishable from the self-exit case, + // and skipping would leave the shell RUNNING after its pane closed -- + // a worse outcome than the leak this all exists to fix. hShell is + // valid under this lock and TerminateProcess does not block, so the + // only cost is that this rare path kills before the console closes. + hShellDup = nullptr; + TerminateProcess(handle->hShell, 1); + } } + if (handle->shellExited) { + const bool removed = remove_pty_baton_locked(id); + assert(removed); + (void)removed; + } + // Else the shell is still running and the watcher frees the baton. + } + } + + // Why outside the lock: ClosePseudoConsole blocks until the conout side has + // drained, and the watcher must be able to take the lock while it does. + if (owed) { + if (pfnClosePseudoConsole) + { + pfnClosePseudoConsole(hpc); } - if (useConptyDll) { - TerminateProcess(handle->hShell, 1); + if (hShellDup != nullptr) { + TerminateProcess(hShellDup, 1); + CloseHandle(hShellDup); } } return env.Undefined(); } +/** + * Orca: confirm a baton really is the pty the caller means. + * + * The winpty backend mints its own `pty` ids from a separate counter, and the + * JS layer stores both in the same field -- so a winpty terminal's id can + * collide with a live ConPTY baton here and terminate an unrelated pane's whole + * process tree. Matching the shell pid makes the id unforgeable. + */ +static bool ownsShell(const pty_baton* handle, DWORD expectedShellPid) { + return handle != nullptr && handle->hJob != nullptr && expectedShellPid != 0 && + handle->shellPid == expectedShellPid; +} + +/** + * Orca: kill this pty's entire tree in one syscall. + * + * Replaces "scrape the process table, walk parent pids, hope none were + * recycled, then taskkill /T /F". Returns false when no job was assigned so + * the caller knows to fall back rather than assume the tree is gone. + */ +static Napi::Value PtyTerminateJob(const Napi::CallbackInfo& info) { + Napi::Env env(info.Env()); + Napi::HandleScope scope(env); + + if (info.Length() != 2 || !info[0].IsNumber() || !info[1].IsNumber()) { + throw Napi::Error::New(env, "Usage: pty.terminateJob(id, shellPid)"); + } + + // Held across the lookup AND the Win32 call: the watcher thread can otherwise + // close these handles and free the baton in between. + std::lock_guard guard(ptyJobMutex); + const pty_baton* handle = get_pty_baton_locked(info[0].As().Int32Value()); + if (!ownsShell(handle, info[1].As().Uint32Value())) { + return Napi::Boolean::New(env, false); + } + return Napi::Boolean::New(env, !!TerminateJobObject(handle->hJob, 1)); +} + +/** + * Orca: the pids still alive in this pty's tree, straight from the kernel. + * + * Descendant liveness for a tree that is still tracked, including children that + * detached from the console. Once the shell exits the watcher nulls hJob, which + * ownsShell rejects, so this returns null rather than an empty list -- null + * means "no answer", never "they died". (The baton itself now outlives the + * shell, until kill() runs; hJob is what makes the answer null.) Also returns + * null when no job was assigned. + * + * Does not include the ConPTY console host: CreatePseudoConsole spawns it + * before this job exists, so it is not a member and ClosePseudoConsole is what + * reaps it. + */ +static Napi::Value PtyListJobProcessIds(const Napi::CallbackInfo& info) { + Napi::Env env(info.Env()); + Napi::HandleScope scope(env); + + if (info.Length() != 2 || !info[0].IsNumber() || !info[1].IsNumber()) { + throw Napi::Error::New(env, "Usage: pty.listJobProcessIds(id, shellPid)"); + } + + // Held across the lookup AND the Win32 call: the watcher thread can otherwise + // close these handles and free the baton in between. + std::lock_guard guard(ptyJobMutex); + const pty_baton* handle = get_pty_baton_locked(info[0].As().Int32Value()); + if (!ownsShell(handle, info[1].As().Uint32Value())) { + return env.Null(); + } + + // Grow until the buffer holds every pid: the count can change between calls, + // and a truncated list would read as "these children are gone". + DWORD capacity = 64; + for (int attempt = 0; attempt < 8; attempt++) { + const size_t bytes = sizeof(JOBOBJECT_BASIC_PROCESS_ID_LIST) + sizeof(ULONG_PTR) * capacity; + std::vector buffer(bytes, 0); + auto* list = reinterpret_cast(buffer.data()); + if (QueryInformationJobObject(handle->hJob, JobObjectBasicProcessIdList, list, static_cast(bytes), nullptr)) { + auto pids = Napi::Array::New(env, list->NumberOfProcessIdsInList); + for (DWORD i = 0; i < list->NumberOfProcessIdsInList; i++) { + pids.Set(i, Napi::Number::New(env, static_cast(list->ProcessIdList[i]))); + } + return pids; + } + if (GetLastError() != ERROR_MORE_DATA) { + return env.Null(); + } + capacity *= 4; + } + return env.Null(); +} + +/** + * Orca: put THIS process in a kill-on-close job, so its whole descendant tree + * dies with it. + * + * Why here and not per-pty: a per-pty job cannot carry KILL_ON_JOB_CLOSE, + * because its handle is released when the shell exits and that would reap + * whatever the user had backgrounded. This job's handle is released only when + * the process itself dies, so it reaps a crashed host without changing what a + * clean shell exit means. Children inherit job membership, so every pty the + * caller later spawns is covered without further work, and the per-pty jobs + * simply nest inside this one. + * + * The handle is deliberately never closed: it must outlive every caller, and + * process teardown is what releases it. + */ +static Napi::Value PtyAssignCurrentProcessToJob(const Napi::CallbackInfo& info) { + Napi::Env env(info.Env()); + Napi::HandleScope scope(env); + + // Why locked: two callers racing here would each create a job, put the + // process in both, and leak the first handle -- and since the handle is what + // keeps a kill-on-close job alive, a leaked one is never released. A worker + // thread with its own N-API env shares these statics, so "only JS calls it" + // is not a guarantee. + static HANDLE hHostJob = nullptr; + std::lock_guard guard(ptyJobMutex); + if (hHostJob != nullptr) { + return Napi::Boolean::New(env, true); + } + + HANDLE job = CreateJobObjectW(nullptr, nullptr); + if (job == nullptr) { + return Napi::Boolean::New(env, false); + } + JOBOBJECT_EXTENDED_LIMIT_INFORMATION limits{}; + // BREAKAWAY_OK for the same reason as the per-pty job: without it a child + // asking for CREATE_BREAKAWAY_FROM_JOB is refused outright. + limits.BasicLimitInformation.LimitFlags = + JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE | JOB_OBJECT_LIMIT_BREAKAWAY_OK; + if (!SetInformationJobObject(job, JobObjectExtendedLimitInformation, &limits, sizeof(limits)) || + !AssignProcessToJobObject(job, GetCurrentProcess())) { + // An outer job that forbids nesting refuses this; the caller degrades. + CloseHandle(job); + return Napi::Boolean::New(env, false); + } + hHostJob = job; + return Napi::Boolean::New(env, true); +} + +/** + * Orca: setActiveProcessLimit(limit) applies JOB_OBJECT_LIMIT_ACTIVE_PROCESS to + * every ConPTY job created afterwards. 0 turns it off. Existing jobs keep theirs. + */ +static Napi::Value PtySetActiveProcessLimit(const Napi::CallbackInfo& info) { + Napi::Env env(info.Env()); + Napi::HandleScope scope(env); + if (info.Length() != 1 || !info[0].IsNumber()) { + throw Napi::Error::New(env, "Usage: pty.setActiveProcessLimit(limit)"); + } + ptyActiveProcessLimit.store(info[0].As().Uint32Value()); + return Napi::Boolean::New(env, true); +} + /** * Init */ @@ -577,6 +910,10 @@ Napi::Object init(Napi::Env env, Napi::Object exports) { exports.Set("resize", Napi::Function::New(env, PtyResize)); exports.Set("clear", Napi::Function::New(env, PtyClear)); exports.Set("kill", Napi::Function::New(env, PtyKill)); + exports.Set("terminateJob", Napi::Function::New(env, PtyTerminateJob)); + exports.Set("listJobProcessIds", Napi::Function::New(env, PtyListJobProcessIds)); + exports.Set("assignCurrentProcessToJob", Napi::Function::New(env, PtyAssignCurrentProcessToJob)); + exports.Set("setActiveProcessLimit", Napi::Function::New(env, PtySetActiveProcessLimit)); return exports; }; diff --git a/lib/windowsConoutConnection.js b/lib/windowsConoutConnection.js --- a/lib/windowsConoutConnection.js +++ b/lib/windowsConoutConnection.js @@ -74,6 +74,15 @@ }; var scriptPath = __dirname.replace('node_modules.asar', 'node_modules.asar.unpacked'); this._worker = new worker_threads_1.Worker(path_1.join(scriptPath, 'worker/conoutSocketWorker.js'), { workerData: workerData }); + this._onFailed = new eventEmitter2_1.EventEmitter2(); + // Orca: without a listener a dead worker's error is rethrown process-wide. Report the exit + // instead, so a relay that dies before it connects still ends its terminal. + this._worker.on('error', function () { }); + this._worker.on('exit', function () { + if (!_this._isDisposed) { + _this._onFailed.fire(); + } + }); this._worker.on('message', function (message) { switch (message) { case 1 /* READY */: @@ -89,6 +98,11 @@ enumerable: false, configurable: true }); + Object.defineProperty(ConoutConnection.prototype, "onFailed", { + get: function () { return this._onFailed.event; }, + enumerable: false, + configurable: true + }); ConoutConnection.prototype.dispose = function () { if (!this._useConptyDll && this._isDisposed) { return; diff --git a/lib/windowsPtyAgent.js b/lib/windowsPtyAgent.js --- a/lib/windowsPtyAgent.js +++ b/lib/windowsPtyAgent.js @@ -75,6 +75,10 @@ this._conoutSocketWorker.onReady(function () { _this._conoutSocketWorker.connectSocket(_this._outSocket); }); + // Orca: surface a dead relay through the output socket's own error path. + this._conoutSocketWorker.onFailed(function () { + _this._outSocket.destroy(new Error('ConPTY output relay exited')); + }); this._outSocket.on('connect', function () { _this._outSocket.emit('ready_datapipe'); }); @@ -136,6 +140,9 @@ if (this._useConpty) { if (!this._useConptyDll) { this._inSocket.readable = false; + // The non-DLL path previously only flipped `readable`, leaving the + // conin PipeWrap alive until the host exited (#947). + this._inSocket.destroy(); this._outSocket.readable = false; this._getConsoleProcessList().then(function (consoleProcessList) { consoleProcessList.forEach(function (pid) { @@ -154,9 +161,10 @@ // Close the input write handle to signal the end of session. this._inSocket.destroy(); this._ptyNative.kill(this._pty, this._useConptyDll); - this._outSocket.on('data', function () { - _this._conoutSocketWorker.dispose(); - }); + // Orca: dispose unconditionally, as the non-DLL branch above does. + // Waiting for another 'data' event leaks the conout worker on every + // self-exiting shell, because no more data ever arrives (F24). + this._conoutSocketWorker.dispose(); } } else { @@ -182,6 +190,11 @@ var _this = this; return new Promise(function (resolve) { var agent = child_process_1.fork(path.join(__dirname, 'conpty_console_list_agent'), [_this._innerPid.toString()]); + // Orca: an unhandled fork or IPC error would be rethrown process-wide. + agent.on('error', function () { + clearTimeout(timeout); + resolve([_this._innerPid]); + }); agent.on('message', function (message) { clearTimeout(timeout); resolve(message.consoleProcessList); diff --git a/lib/windowsTerminal.js b/lib/windowsTerminal.js --- a/lib/windowsTerminal.js +++ b/lib/windowsTerminal.js @@ -50,6 +50,29 @@ // Create new termal. _this._agent = new windowsPtyAgent_1.WindowsPtyAgent(file, args, parsedEnv, cwd, _this._cols, _this._rows, false, opt.useConpty, opt.useConptyDll, opt.conptyInheritCursor); _this._socket = _this._agent.outSocket; + // Attach before readiness so a broken ConPTY output pipe cannot be unhandled. + var outputCloseEmitsExit = false; + var killedBeforeReady = false; + _this._socket.on('error', function () { + // Orca: retire only this terminal. Rethrowing an unlisted code (write UNKNOWN, ECONNRESET) + // killed every PTY in the process; consumer listeners still receive the error. + _this._close(); + if (_this._isReady || killedBeforeReady) { + return; + } + // Before the first output kill() stays deferred forever, so end the shell here. + killedBeforeReady = true; + try { + _this._agent.kill(); + } + catch (_a) { + // The shell may already be gone; the terminal is retired either way. + } + // Before the relay connects no 'close' handler reports exit. + if (!outputCloseEmitsExit) { + _this.emit('exit', _this._agent.exitCode); + } + }); // Not available until `ready` event emitted. _this._pid = _this._agent.innerPid; _this._fd = _this._agent.fd; @@ -57,6 +80,7 @@ // The forked windows terminal is not available until `ready` event is // emitted. _this._socket.on('ready_datapipe', function () { + outputCloseEmitsExit = true; // Run deferreds and set ready state once the first data event is received. _this._socket.once('data', function () { // Wait until the first data event is fired then we can run deferreds. @@ -76,23 +100,6 @@ _this._deferreds = []; } }); - // Shutdown if `error` event is emitted. - _this._socket.on('error', function (err) { - // Close terminal session. - _this._close(); - // EIO, happens when someone closes our child process: the only process - // in the terminal. - // node < 0.6.14: errno 5 - // node >= 0.6.14: read EIO - if (err.code) { - if (~err.code.indexOf('errno 5') || ~err.code.indexOf('EIO')) - return; - } - // Throw anything else. - if (_this.listeners('error').length < 2) { - throw err; - } - }); // Cleanup after the socket is closed. _this._socket.on('close', function () { _this.emit('exit', _this._agent.exitCode); @@ -103,6 +110,20 @@ _this._name = name; _this._readable = true; _this._writable = true; + // A ConPTY input-pipe error must retire only this terminal. Without a listener, Node promotes + // errors such as write EAGAIN to uncaughtException and kills every PTY in the daemon. + _this._agent.inSocket.on('error', function () { + if (!_this._writable) { + return; + } + _this._close(); + try { + _this._agent.kill(); + } + catch (_a) { + // The failing pipe may have raced process exit; the terminal is already unwritable. + } + }); _this._forwardEvents(); return _this; } @@ -196,4 +217,4 @@ return WindowsTerminal; }(terminal_1.Terminal)); exports.WindowsTerminal = WindowsTerminal; -//# sourceMappingURL=windowsTerminal.js.map \ No newline at end of file +//# sourceMappingURL=windowsTerminal.js.map diff --git a/lib/worker/conoutSocketWorker.js b/lib/worker/conoutSocketWorker.js --- a/lib/worker/conoutSocketWorker.js +++ b/lib/worker/conoutSocketWorker.js @@ -9,10 +9,33 @@ var conoutPipeName = worker_threads_1.workerData.conoutPipeName; var conoutSocket = new net_1.Socket(); conoutSocket.setEncoding('utf8'); +// Orca: an unhandled stream error crashes this worker, and Node rethrows it on the main thread as +// an uncaughtException that kills every PTY in the process (write EPIPE/UNKNOWN once the reader +// closed: #26877, #33350). Contain errors per terminal and keep draining ConPTY output. +var workerSockets = new Set(); +var drainConout = function () { + conoutSocket.resume(); +}; +conoutSocket.on('error', function () { + // Closing the reader ends this terminal instead of leaving it waiting for output. + workerSockets.forEach(function (workerSocket) { return workerSocket.destroy(); }); +}); conoutSocket.connect(conoutPipeName, function () { var server = net_1.createServer(function (workerSocket) { + if (conoutSocket.destroyed) { + workerSocket.destroy(); + return; + } + workerSockets.add(workerSocket); + workerSocket.on('error', function () { }); conoutSocket.pipe(workerSocket); + // After pipe() so its unpipe-and-pause runs first; an undrained conout can block ClosePseudoConsole. + workerSocket.on('close', function () { + workerSockets.delete(workerSocket); + drainConout(); + }); }); + server.on('error', drainConout); server.listen(conout_1.getWorkerPipeName(conoutPipeName)); if (!worker_threads_1.parentPort) { throw new Error('worker_threads parentPort is null'); diff --git a/src/windowsConoutConnection.ts b/src/windowsConoutConnection.ts --- a/src/windowsConoutConnection.ts +++ b/src/windowsConoutConnection.ts @@ -35,6 +35,8 @@ private _onReady = new EventEmitter2(); public get onReady(): IEvent { return this._onReady.event; } + private _onFailed = new EventEmitter2(); + public get onFailed(): IEvent { return this._onFailed.event; } constructor( private _conoutPipeName: string, @@ -45,6 +47,14 @@ }; const scriptPath = __dirname.replace('node_modules.asar', 'node_modules.asar.unpacked'); this._worker = new Worker(join(scriptPath, 'worker/conoutSocketWorker.js'), { workerData }); + // Orca: without a listener a dead worker's error is rethrown process-wide. Report the exit + // instead, so a relay that dies before it connects still ends its terminal. + this._worker.on('error', () => {}); + this._worker.on('exit', () => { + if (!this._isDisposed) { + this._onFailed.fire(); + } + }); this._worker.on('message', (message: ConoutWorkerMessage) => { switch (message) { case ConoutWorkerMessage.READY: diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts --- a/src/windowsPtyAgent.ts +++ b/src/windowsPtyAgent.ts @@ -103,6 +103,10 @@ this._conoutSocketWorker.onReady(() => { this._conoutSocketWorker.connectSocket(this._outSocket); }); + // Orca: surface a dead relay through the output socket's own error path. + this._conoutSocketWorker.onFailed(() => { + this._outSocket.destroy(new Error('ConPTY output relay exited')); + }); this._outSocket.on('connect', () => { this._outSocket.emit('ready_datapipe'); }); @@ -143,6 +147,9 @@ if (this._useConpty) { if (!this._useConptyDll) { this._inSocket.readable = false; + // The non-DLL path previously only flipped `readable`, leaving the + // conin PipeWrap alive until the host exited (#947). + this._inSocket.destroy(); this._outSocket.readable = false; this._getConsoleProcessList().then(consoleProcessList => { consoleProcessList.forEach((pid: number) => { @@ -159,9 +166,10 @@ // Close the input write handle to signal the end of session. this._inSocket.destroy(); (this._ptyNative as IConptyNative).kill(this._pty, this._useConptyDll); - this._outSocket.on('data', () => { - this._conoutSocketWorker.dispose(); - }); + // Orca: dispose unconditionally, as the non-DLL branch above does. + // Waiting for another 'data' event leaks the conout worker on every + // self-exiting shell, because no more data ever arrives (F24). + this._conoutSocketWorker.dispose(); } } else { // Because pty.kill closes the handle, it will kill most processes by itself. @@ -185,6 +193,11 @@ private _getConsoleProcessList(): Promise { return new Promise(resolve => { const agent = fork(path.join(__dirname, 'conpty_console_list_agent'), [ this._innerPid.toString() ]); + // Orca: an unhandled fork or IPC error would be rethrown process-wide. + agent.on('error', () => { + clearTimeout(timeout); + resolve([ this._innerPid ]); + }); agent.on('message', message => { clearTimeout(timeout); resolve(message.consoleProcessList); diff --git a/src/windowsTerminal.ts b/src/windowsTerminal.ts --- a/src/windowsTerminal.ts +++ b/src/windowsTerminal.ts @@ -51,6 +51,29 @@ this._agent = new WindowsPtyAgent(file, args, parsedEnv, cwd, this._cols, this._rows, false, opt.useConpty, opt.useConptyDll, opt.conptyInheritCursor); this._socket = this._agent.outSocket; + // Attach before readiness so a broken ConPTY output pipe cannot be unhandled. + let outputCloseEmitsExit = false; + let killedBeforeReady = false; + this._socket.on('error', () => { + // Orca: retire only this terminal. Rethrowing an unlisted code (write UNKNOWN, ECONNRESET) + // killed every PTY in the process; consumer listeners still receive the error. + this._close(); + if (this._isReady || killedBeforeReady) { + return; + } + // Before the first output kill() stays deferred forever, so end the shell here. + killedBeforeReady = true; + try { + this._agent.kill(); + } catch { + // The shell may already be gone; the terminal is retired either way. + } + // Before the relay connects no 'close' handler reports exit. + if (!outputCloseEmitsExit) { + this.emit('exit', this._agent.exitCode); + } + }); + // Not available until `ready` event emitted. this._pid = this._agent.innerPid; this._fd = this._agent.fd; @@ -59,6 +82,7 @@ // The forked windows terminal is not available until `ready` event is // emitted. this._socket.on('ready_datapipe', () => { + outputCloseEmitsExit = true; // Run deferreds and set ready state once the first data event is received. this._socket.once('data', () => { @@ -82,25 +106,6 @@ } }); - // Shutdown if `error` event is emitted. - this._socket.on('error', err => { - // Close terminal session. - this._close(); - - // EIO, happens when someone closes our child process: the only process - // in the terminal. - // node < 0.6.14: errno 5 - // node >= 0.6.14: read EIO - if ((err).code) { - if (~(err).code.indexOf('errno 5') || ~(err).code.indexOf('EIO')) return; - } - - // Throw anything else. - if (this.listeners('error').length < 2) { - throw err; - } - }); - // Cleanup after the socket is closed. this._socket.on('close', () => { this.emit('exit', this._agent.exitCode); @@ -114,6 +119,19 @@ this._readable = true; this._writable = true; + // A ConPTY input-pipe error must retire only this terminal. Without a listener, Node promotes + // errors such as write EAGAIN to uncaughtException and kills every PTY in the daemon. + this._agent.inSocket.on('error', () => { + if (!this._writable) { + return; + } + this._close(); + try { + this._agent.kill(); + } catch { + // The failing pipe may have raced process exit; the terminal is already unwritable. + } + }); this._forwardEvents(); } diff --git a/src/worker/conoutSocketWorker.ts b/src/worker/conoutSocketWorker.ts --- a/src/worker/conoutSocketWorker.ts +++ b/src/worker/conoutSocketWorker.ts @@ -10,10 +10,33 @@ const conoutSocket = new Socket(); conoutSocket.setEncoding('utf8'); +// Orca: an unhandled stream error crashes this worker, and Node rethrows it on the main thread as +// an uncaughtException that kills every PTY in the process (write EPIPE/UNKNOWN once the reader +// closed: #26877, #33350). Contain errors per terminal and keep draining ConPTY output. +const workerSockets = new Set(); +const drainConout = (): void => { + conoutSocket.resume(); +}; +conoutSocket.on('error', () => { + // Closing the reader ends this terminal instead of leaving it waiting for output. + workerSockets.forEach(workerSocket => workerSocket.destroy()); +}); conoutSocket.connect(conoutPipeName, () => { const server = createServer(workerSocket => { + if (conoutSocket.destroyed) { + workerSocket.destroy(); + return; + } + workerSockets.add(workerSocket); + workerSocket.on('error', () => {}); conoutSocket.pipe(workerSocket); + // After pipe() so its unpipe-and-pause runs first; an undrained conout can block ClosePseudoConsole. + workerSocket.on('close', () => { + workerSockets.delete(workerSocket); + drainConout(); + }); }); + server.on('error', drainConout); server.listen(getWorkerPipeName(conoutPipeName)); if (!parentPort) { throw new Error('worker_threads parentPort is null');