mirror of
https://github.com/stablyai/orca.git
synced 2026-09-28 00:02:41 +00:00
fix(windows): synchronize native terminal table lifetime (#23257)
* fix(i18n): restore diff note draft catalog entries * fix(windows): synchronize native terminal table lifetime * fix(ci): preserve focused Playwright file selection * test(windows): retain PTY stress failure evidence * test: isolate hourly identity inputs from release version changes * test(ci): require focused commands on every platform Assert each golden platform step and its focused arguments before deduplicating Playwright discovery probes. --------- Co-authored-by: m4air <m4air@m4airs-Air.localdomain> Co-authored-by: Neil <neil@stably.ai>
This commit is contained in:
@@ -707,7 +707,7 @@ index 98733dc0cd752b554bd94e45904ca341ad141bba..3dd5ad9b3124dbd5ba7f76679b26650d
|
||||
// Stop processing immediately on unexpected error and log
|
||||
this._writeQueue.length = 0;
|
||||
diff --git a/src/win/conpty.cc b/src/win/conpty.cc
|
||||
index 7b286d3d644c26141df516929703aa6e129df4b2..4b06d18576c807c3d1181a7bd714140c6678cf86 100644
|
||||
index 7b286d3d64..5239ba4e40 100644
|
||||
--- a/src/win/conpty.cc
|
||||
+++ b/src/win/conpty.cc
|
||||
@@ -18,6 +18,7 @@
|
||||
@@ -718,7 +718,7 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4b06d18576c807c3d1181a7bd714140c
|
||||
#include <vector>
|
||||
#include <Windows.h>
|
||||
#include <strsafe.h>
|
||||
@@ -44,12 +45,40 @@ struct pty_baton {
|
||||
@@ -44,15 +45,37 @@ struct pty_baton {
|
||||
HANDLE hOut;
|
||||
HPCON hpc;
|
||||
|
||||
@@ -749,18 +749,25 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4b06d18576c807c3d1181a7bd714140c
|
||||
};
|
||||
|
||||
static std::vector<std::unique_ptr<pty_baton>> ptyHandles;
|
||||
+// Orca: guards the job accessors below, and PtyKill, against the exit watcher
|
||||
+// thread. It does NOT make the whole table safe -- PtyResize and PtyClear still
|
||||
+// read it unlocked, as they always have -- but it closes the window this patch
|
||||
+// opened, where the watcher can close hShell/hJob and free the baton between a
|
||||
+// lookup and its use.
|
||||
+// Handle VALUES are recycled aggressively, so an unguarded read could pass the
|
||||
+// shell-pid check against an unrelated process and terminate the wrong job.
|
||||
+// Orca: every table access shares the exit watcher's lock; handles can be recycled.
|
||||
+static std::mutex ptyJobMutex;
|
||||
static volatile LONG ptyCounter;
|
||||
|
||||
static pty_baton* get_pty_baton(int id) {
|
||||
@@ -102,8 +131,31 @@ void SetupExitCallback(Napi::Env env, Napi::Function cb, pty_baton* baton) {
|
||||
-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 +85,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 +125,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
|
||||
@@ -782,7 +789,7 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4b06d18576c807c3d1181a7bd714140c
|
||||
+ // NDEBUG would compile the call away and leak every baton.
|
||||
+ baton->shellExited = true;
|
||||
+ if (baton->consoleClosed) {
|
||||
+ const bool removed = remove_pty_baton(baton->id);
|
||||
+ const bool removed = remove_pty_baton_locked(baton->id);
|
||||
+ assert(removed);
|
||||
+ (void)removed;
|
||||
+ }
|
||||
@@ -794,7 +801,7 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4b06d18576c807c3d1181a7bd714140c
|
||||
|
||||
auto status = tsfn.BlockingCall(exit_event, callback); // In main thread
|
||||
switch (status) {
|
||||
@@ -242,6 +294,20 @@
|
||||
@@ -242,6 +288,20 @@ HRESULT CreateNamedPipesAndPseudoConsole(const Napi::CallbackInfo& info,
|
||||
return HRESULT_FROM_WIN32(GetLastError());
|
||||
}
|
||||
|
||||
@@ -815,15 +822,40 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4b06d18576c807c3d1181a7bd714140c
|
||||
static Napi::Value PtyStartProcess(const Napi::CallbackInfo& info) {
|
||||
Napi::Env env(info.Env());
|
||||
Napi::HandleScope scope(env);
|
||||
@@ -303,6 +369,7 @@
|
||||
@@ -301,8 +361,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<pty_baton>(ptyId, hIn, hOut, hpc));
|
||||
+ ptyHandles.back()->allowJobBreakaway = !usesCygwinRuntime(shellpath);
|
||||
- ptyHandles.emplace_back(
|
||||
- std::make_unique<pty_baton>(ptyId, hIn, hOut, hpc));
|
||||
+ auto baton = std::make_unique<pty_baton>(ptyId, hIn, hOut, hpc);
|
||||
+ baton->allowJobBreakaway = !usesCygwinRuntime(shellpath);
|
||||
+ std::lock_guard<std::mutex> guard(ptyJobMutex);
|
||||
+ ptyHandles.emplace_back(std::move(baton));
|
||||
} else {
|
||||
throw Napi::Error::New(env, "Cannot launch conpty");
|
||||
}
|
||||
@@ -409,6 +476,15 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) {
|
||||
@@ -350,11 +412,15 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) {
|
||||
const bool useConptyDll = info[4].As<Napi::Boolean>().Value();
|
||||
Napi::Function exitCallback = info[5].As<Napi::Function>();
|
||||
|
||||
- // 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<std::mutex> 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<wchar_t[]> mutableCommandline = std::make_unique<wchar_t[]>(cmdline.length() + 1);
|
||||
@@ -409,6 +475,15 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) {
|
||||
throw errorWithCode(info, "UpdateProcThreadAttribute failed");
|
||||
}
|
||||
|
||||
@@ -839,7 +871,7 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4b06d18576c807c3d1181a7bd714140c
|
||||
PROCESS_INFORMATION piClient{};
|
||||
fSuccess = !!CreateProcessW(
|
||||
nullptr,
|
||||
@@ -416,7 +492,10 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) {
|
||||
@@ -416,7 +491,10 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) {
|
||||
nullptr, // lpProcessAttributes
|
||||
nullptr, // lpThreadAttributes
|
||||
false, // bInheritHandles VERY IMPORTANT that this is false
|
||||
@@ -851,7 +883,7 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4b06d18576c807c3d1181a7bd714140c
|
||||
envArg, // lpEnvironment
|
||||
mutableCwd.get(), // lpCurrentDirectory
|
||||
&siEx.StartupInfo, // lpStartupInfo
|
||||
@@ -426,8 +505,48 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) {
|
||||
@@ -426,8 +504,48 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) {
|
||||
throw errorWithCode(info, "Cannot create process");
|
||||
}
|
||||
|
||||
@@ -902,25 +934,52 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4b06d18576c807c3d1181a7bd714140c
|
||||
if (useConptyDll && fLoadedDll)
|
||||
{
|
||||
PFNRELEASEPSEUDOCONSOLE const pfnReleasePseudoConsole = (PFNRELEASEPSEUDOCONSOLE)GetProcAddress(
|
||||
@@ -440,6 +559,8 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) {
|
||||
@@ -438,8 +556,12 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) {
|
||||
}
|
||||
}
|
||||
|
||||
// Update handle
|
||||
handle->hShell = piClient.hProcess;
|
||||
+ handle->shellPid = piClient.dwProcessId;
|
||||
+ handle->hJob = hJob;
|
||||
- // Update handle
|
||||
- handle->hShell = piClient.hProcess;
|
||||
+ {
|
||||
+ std::lock_guard<std::mutex> guard(ptyJobMutex);
|
||||
+ handle->hShell = piClient.hProcess;
|
||||
+ handle->shellPid = piClient.dwProcessId;
|
||||
+ handle->hJob = hJob;
|
||||
+ }
|
||||
|
||||
// Close the thread handle to avoid resource leak
|
||||
CloseHandle(piClient.hThread);
|
||||
@@ -544,27 +665,213 @@ static Napi::Value PtyKill(const Napi::CallbackInfo& info) {
|
||||
@@ -472,9 +594,10 @@ static Napi::Value PtyResize(const Napi::CallbackInfo& info) {
|
||||
SHORT rows = static_cast<SHORT>(info[2].As<Napi::Number>().Uint32Value());
|
||||
const bool useConptyDll = info[3].As<Napi::Boolean>().Value();
|
||||
|
||||
- const pty_baton* handle = get_pty_baton(id);
|
||||
+ std::lock_guard<std::mutex> 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 +636,10 @@ static Napi::Value PtyClear(const Napi::CallbackInfo& info) {
|
||||
return env.Undefined();
|
||||
}
|
||||
|
||||
- const pty_baton* handle = get_pty_baton(id);
|
||||
+ std::lock_guard<std::mutex> 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 +668,215 @@ static Napi::Value PtyKill(const Napi::CallbackInfo& info) {
|
||||
int id = info[0].As<Napi::Number>().Int32Value();
|
||||
const bool useConptyDll = info[1].As<Napi::Boolean>().Value();
|
||||
|
||||
- const pty_baton* handle = get_pty_baton(id);
|
||||
-
|
||||
- if (handle != nullptr) {
|
||||
- HANDLE hLibrary = LoadConptyDll(info, useConptyDll);
|
||||
- bool fLoadedDll = hLibrary != nullptr;
|
||||
- if (fLoadedDll)
|
||||
+ // 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
|
||||
@@ -934,7 +993,18 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4b06d18576c807c3d1181a7bd714140c
|
||||
+ (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
|
||||
@@ -945,7 +1015,7 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4b06d18576c807c3d1181a7bd714140c
|
||||
+ bool owed = false;
|
||||
+ {
|
||||
+ std::lock_guard<std::mutex> guard(ptyJobMutex);
|
||||
+ pty_baton* handle = get_pty_baton(id);
|
||||
+ 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) {
|
||||
@@ -965,9 +1035,9 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4b06d18576c807c3d1181a7bd714140c
|
||||
+ hShellDup = nullptr;
|
||||
+ TerminateProcess(handle->hShell, 1);
|
||||
+ }
|
||||
+ }
|
||||
}
|
||||
+ if (handle->shellExited) {
|
||||
+ const bool removed = remove_pty_baton(id);
|
||||
+ const bool removed = remove_pty_baton_locked(id);
|
||||
+ assert(removed);
|
||||
+ (void)removed;
|
||||
+ }
|
||||
@@ -979,19 +1049,11 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4b06d18576c807c3d1181a7bd714140c
|
||||
+ // drained, and the watcher must be able to take the lock while it does.
|
||||
+ if (owed) {
|
||||
+ if (pfnClosePseudoConsole)
|
||||
{
|
||||
- PFNCLOSEPSEUDOCONSOLE const pfnClosePseudoConsole = (PFNCLOSEPSEUDOCONSOLE)GetProcAddress(
|
||||
- (HMODULE)hLibrary,
|
||||
- useConptyDll ? "ConptyClosePseudoConsole" : "ClosePseudoConsole");
|
||||
- if (pfnClosePseudoConsole)
|
||||
- {
|
||||
- pfnClosePseudoConsole(handle->hpc);
|
||||
- }
|
||||
- }
|
||||
+ {
|
||||
+ pfnClosePseudoConsole(hpc);
|
||||
}
|
||||
- if (useConptyDll) {
|
||||
- TerminateProcess(handle->hShell, 1);
|
||||
+ pfnClosePseudoConsole(hpc);
|
||||
+ }
|
||||
+ if (hShellDup != nullptr) {
|
||||
+ TerminateProcess(hShellDup, 1);
|
||||
+ CloseHandle(hShellDup);
|
||||
@@ -999,8 +1061,8 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4b06d18576c807c3d1181a7bd714140c
|
||||
}
|
||||
|
||||
return env.Undefined();
|
||||
+}
|
||||
+
|
||||
}
|
||||
|
||||
+/**
|
||||
+ * Orca: confirm a baton really is the pty the caller means.
|
||||
+ *
|
||||
@@ -1032,7 +1094,7 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4b06d18576c807c3d1181a7bd714140c
|
||||
+ // 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<std::mutex> guard(ptyJobMutex);
|
||||
+ const pty_baton* handle = get_pty_baton(info[0].As<Napi::Number>().Int32Value());
|
||||
+ const pty_baton* handle = get_pty_baton_locked(info[0].As<Napi::Number>().Int32Value());
|
||||
+ if (!ownsShell(handle, info[1].As<Napi::Number>().Uint32Value())) {
|
||||
+ return Napi::Boolean::New(env, false);
|
||||
+ }
|
||||
@@ -1064,7 +1126,7 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4b06d18576c807c3d1181a7bd714140c
|
||||
+ // 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<std::mutex> guard(ptyJobMutex);
|
||||
+ const pty_baton* handle = get_pty_baton(info[0].As<Napi::Number>().Int32Value());
|
||||
+ const pty_baton* handle = get_pty_baton_locked(info[0].As<Napi::Number>().Int32Value());
|
||||
+ if (!ownsShell(handle, info[1].As<Napi::Number>().Uint32Value())) {
|
||||
+ return env.Null();
|
||||
+ }
|
||||
@@ -1138,10 +1200,12 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4b06d18576c807c3d1181a7bd714140c
|
||||
+ }
|
||||
+ hHostJob = job;
|
||||
+ return Napi::Boolean::New(env, true);
|
||||
}
|
||||
|
||||
+}
|
||||
+
|
||||
/**
|
||||
@@ -577,6 +884,9 @@ Napi::Object init(Napi::Env env, Napi::Object exports) {
|
||||
* Init
|
||||
*/
|
||||
@@ -577,6 +887,9 @@ 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));
|
||||
|
||||
@@ -0,0 +1,156 @@
|
||||
'use strict'
|
||||
|
||||
const assert = require('node:assert/strict')
|
||||
const { createHash } = require('node:crypto')
|
||||
const { readFileSync, writeSync } = require('node:fs')
|
||||
const { dirname, resolve } = require('node:path')
|
||||
|
||||
function report(phase, details = {}) {
|
||||
writeSync(1, `${JSON.stringify({ phase, hostPid: process.pid, ...details })}\n`)
|
||||
}
|
||||
|
||||
async function exerciseTable() {
|
||||
assert.equal(process.platform, 'win32', 'This probe requires real Windows ConPTY')
|
||||
const rounds = Number(process.env.ORCA_PTY_TABLE_STRESS_ROUNDS ?? 8)
|
||||
assert.ok(Number.isInteger(rounds) && rounds > 0 && rounds <= 2000)
|
||||
const pty = require('node-pty')
|
||||
const nativePath = require.resolve('node-pty/lib/utils')
|
||||
const loaded = require(nativePath).loadNativeModule('conpty')
|
||||
const native = loaded.module
|
||||
const addonPath = resolve(dirname(nativePath), loaded.dir, 'conpty.node')
|
||||
report('native', {
|
||||
addonPath,
|
||||
sha256: createHash('sha256').update(readFileSync(addonPath)).digest('hex'),
|
||||
node: process.version,
|
||||
rounds
|
||||
})
|
||||
// Unlike production's fallback, this crash probe requires a host that permits nested jobs.
|
||||
const hostJobAssigned = native.assignCurrentProcessToJob()
|
||||
report('host-job-precondition', { assigned: hostJobAssigned })
|
||||
assert.equal(hostJobAssigned, true, 'Probe precondition: host must permit a crash-cleanup job')
|
||||
|
||||
const spawned = []
|
||||
function spawn(round, slot) {
|
||||
report('spawn', { round, slot })
|
||||
const proc = pty.spawn(process.env.ComSpec || 'cmd.exe', ['/d', '/q'], {
|
||||
cwd: process.cwd(),
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
useConptyDll: true
|
||||
})
|
||||
const record = { proc, output: '', exited: false, closed: false }
|
||||
const marker = `ORCA_PTY_READY_${spawned.length}`
|
||||
let resolveReady
|
||||
record.ready = new Promise((resolve) => {
|
||||
resolveReady = resolve
|
||||
})
|
||||
record.exit = new Promise((resolveExit) => {
|
||||
proc.onExit((event) => {
|
||||
record.exited = true
|
||||
resolveReady(false)
|
||||
resolveExit(event)
|
||||
})
|
||||
})
|
||||
proc.onData((chunk) => {
|
||||
record.output = (record.output + chunk).slice(-2048)
|
||||
if (record.output.includes(marker)) {
|
||||
resolveReady(true)
|
||||
}
|
||||
})
|
||||
spawned.push(record)
|
||||
// Escaping one letter keeps echoed input from satisfying the output marker.
|
||||
proc.write(`echo ${marker.replace('READY', 'REA^DY')}\r`)
|
||||
return record
|
||||
}
|
||||
|
||||
async function waitForReady(records) {
|
||||
let timer
|
||||
try {
|
||||
await Promise.race([
|
||||
Promise.all(
|
||||
records.map(async (record) => {
|
||||
assert.equal(await record.ready, true, `Shell exited before ready: ${record.output}`)
|
||||
})
|
||||
),
|
||||
new Promise((_, reject) => {
|
||||
timer = setTimeout(() => {
|
||||
const transcripts = records.map(({ proc, output }) => ({ pid: proc.pid, output }))
|
||||
reject(new Error(`PTY readiness timed out: ${JSON.stringify(transcripts)}`))
|
||||
}, 15_000)
|
||||
})
|
||||
])
|
||||
} finally {
|
||||
clearTimeout(timer)
|
||||
}
|
||||
for (const { proc } of records) {
|
||||
const members = native.listJobProcessIds(proc._pty, proc.pid)
|
||||
assert.ok(members?.includes(proc.pid), `Ready shell ${proc.pid} must retain its job`)
|
||||
}
|
||||
report('ready', { shellPids: records.map(({ proc }) => proc.pid) })
|
||||
}
|
||||
|
||||
function close(record, round, slot) {
|
||||
report('kill', { round, slot, shellPid: record.proc.pid })
|
||||
record.proc.kill()
|
||||
record.closed = true
|
||||
}
|
||||
|
||||
let failure
|
||||
try {
|
||||
const survivor = spawn(-1, -1)
|
||||
await waitForReady([survivor])
|
||||
const slots = []
|
||||
for (let round = 0; round < rounds; round += 1) {
|
||||
for (let slot = 0; slot < 3; slot += 1) {
|
||||
if (slots[slot]) {
|
||||
close(slots[slot], round, slot)
|
||||
}
|
||||
// The next lookup/insertion overlaps the previous shell's native exit watcher.
|
||||
slots[slot] = spawn(round, slot)
|
||||
report('resize-clear-list', { round, slot, shellPid: survivor.proc.pid })
|
||||
survivor.proc.resize(80 + (round % 2), 24)
|
||||
survivor.proc.clear()
|
||||
const members = native.listJobProcessIds(survivor.proc._pty, survivor.proc.pid)
|
||||
assert.ok(members?.includes(survivor.proc.pid), 'A live survivor must retain its job')
|
||||
}
|
||||
// A ready terminal runs kill/resize/clear immediately instead of deferring them.
|
||||
await waitForReady(slots)
|
||||
}
|
||||
assert.equal(survivor.exited, false, survivor.output)
|
||||
report('overlap-complete', { terminals: spawned.length })
|
||||
} catch (error) {
|
||||
failure = { error }
|
||||
} finally {
|
||||
for (const record of spawned) {
|
||||
if (!record.closed) {
|
||||
close(record, -1, -1)
|
||||
}
|
||||
}
|
||||
let timer
|
||||
try {
|
||||
await Promise.race([
|
||||
Promise.all(spawned.map((record) => record.exit)),
|
||||
new Promise((_, reject) => {
|
||||
timer = setTimeout(() => reject(new Error('PTY exit callbacks did not drain')), 15_000)
|
||||
})
|
||||
])
|
||||
} catch (error) {
|
||||
if (failure) {
|
||||
report('drain-error', { message: error.stack })
|
||||
} else {
|
||||
failure = { error }
|
||||
}
|
||||
} finally {
|
||||
clearTimeout(timer)
|
||||
}
|
||||
}
|
||||
if (failure) {
|
||||
throw failure.error
|
||||
}
|
||||
report('complete', { terminals: spawned.length })
|
||||
}
|
||||
|
||||
exerciseTable().catch((error) => {
|
||||
report('error', { message: error.stack })
|
||||
process.exitCode = 1
|
||||
})
|
||||
Generated
+3
-3
@@ -117,7 +117,7 @@ patchedDependencies:
|
||||
'@xterm/addon-webgl@0.20.0-beta.299': 94687e89a0115e6e6aa102837f986debdc029c091527ee5eb4a4e17ceaf9473e
|
||||
'@xterm/xterm@6.1.0-beta.303': dd0ccc59cd1ccf99f4d76e5aa2456da165fa0804dce19a833d7638bd07ffa393
|
||||
lint-staged@16.4.0: 7333b3837f80a7fbd045964db6d76ba4fc118e49134bdbabb00585b6b7b60673
|
||||
node-pty@1.1.0: 346cb29d33dd6eeb14910ff411c7584b0b2ff9a4b271c4b6d23c3b48e6548f74
|
||||
node-pty@1.1.0: 92c95cffab383d86b3b13a460c75a08074468ebf1f3911db8e874e083201f192
|
||||
|
||||
importers:
|
||||
|
||||
@@ -161,7 +161,7 @@ importers:
|
||||
version: 3.3.1
|
||||
node-pty:
|
||||
specifier: ^1.1.0
|
||||
version: 1.1.0(patch_hash=346cb29d33dd6eeb14910ff411c7584b0b2ff9a4b271c4b6d23c3b48e6548f74)
|
||||
version: 1.1.0(patch_hash=92c95cffab383d86b3b13a460c75a08074468ebf1f3911db8e874e083201f192)
|
||||
posthog-node:
|
||||
specifier: ^5.52.5
|
||||
version: 5.52.5
|
||||
@@ -13098,7 +13098,7 @@ snapshots:
|
||||
|
||||
node-int64@0.4.0: {}
|
||||
|
||||
node-pty@1.1.0(patch_hash=346cb29d33dd6eeb14910ff411c7584b0b2ff9a4b271c4b6d23c3b48e6548f74):
|
||||
node-pty@1.1.0(patch_hash=92c95cffab383d86b3b13a460c75a08074468ebf1f3911db8e874e083201f192):
|
||||
dependencies:
|
||||
node-addon-api: 7.1.1
|
||||
|
||||
|
||||
@@ -84,7 +84,7 @@ describe('node-pty patch: pseudoconsole close on the self-exit path', () => {
|
||||
[
|
||||
'+ baton->shellExited = true;',
|
||||
'+ if (baton->consoleClosed) {',
|
||||
'+ const bool removed = remove_pty_baton(baton->id);',
|
||||
'+ const bool removed = remove_pty_baton_locked(baton->id);',
|
||||
'+ assert(removed);',
|
||||
'+ (void)removed;',
|
||||
'+ }'
|
||||
@@ -138,7 +138,7 @@ describe('node-pty patch: pseudoconsole close on the self-exit path', () => {
|
||||
expect(ptyKillHunk).toContain(
|
||||
[
|
||||
'+ if (handle->shellExited) {',
|
||||
'+ const bool removed = remove_pty_baton(id);',
|
||||
'+ const bool removed = remove_pty_baton_locked(id);',
|
||||
'+ assert(removed);',
|
||||
'+ (void)removed;'
|
||||
].join('\n')
|
||||
|
||||
@@ -37,8 +37,12 @@ describeOnWindows('MSYS terminal job ownership', () => {
|
||||
})
|
||||
let output = ''
|
||||
let childPid: number | undefined
|
||||
let exit: { exitCode: number; signal?: number } | undefined
|
||||
proc.onExit((event) => {
|
||||
exit = event
|
||||
})
|
||||
proc.onData((chunk) => {
|
||||
output += chunk
|
||||
output = (output + chunk).slice(-32_768)
|
||||
const match = /MSYS_OWNED_CHILD=(\d+)/.exec(output)
|
||||
if (match) {
|
||||
childPid = Number(match[1])
|
||||
@@ -48,7 +52,14 @@ describeOnWindows('MSYS terminal job ownership', () => {
|
||||
proc.write(
|
||||
`${quotePosixShell(process.execPath.replace(/\\/g, '/'))} ${quotePosixShell(script.replace(/\\/g, '/'))}\r`
|
||||
)
|
||||
await vi.waitFor(() => expect(childPid).toBeDefined(), { timeout: 15_000 })
|
||||
try {
|
||||
await vi.waitFor(() => expect(childPid).toBeDefined(), { timeout: 15_000 })
|
||||
} catch (cause) {
|
||||
throw new Error(
|
||||
JSON.stringify({ shellPid: proc.pid, exit, jobPids: listPtyJobProcessIds(proc), output }),
|
||||
{ cause }
|
||||
)
|
||||
}
|
||||
expect(isAlive(childPid!)).toBe(true)
|
||||
expect(listPtyJobProcessIds(proc)).toContain(childPid)
|
||||
expect(terminatePtyJob(proc)).toBe('terminated')
|
||||
|
||||
@@ -3,6 +3,7 @@ import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { IPty } from 'node-pty'
|
||||
import { runProcess } from '../../shared/child-process/run-process'
|
||||
import {
|
||||
isPtyJobOwnershipAvailable,
|
||||
listPtyJobProcessIds,
|
||||
@@ -102,6 +103,21 @@ describeOnWindows('ConPTY job ownership', () => {
|
||||
expect(isPtyJobOwnershipAvailable()).toBe(true)
|
||||
})
|
||||
|
||||
it('keeps the native table intact while shell cleanup overlaps new terminals', async () => {
|
||||
const result = await runProcess({
|
||||
program: process.execPath,
|
||||
args: [join(process.cwd(), 'config', 'scripts', 'windows-pty-table-stress.cjs')],
|
||||
env: { ...process.env, ORCA_BACKGROUND_LAUNCH: '1' },
|
||||
timeoutMs: 90_000
|
||||
})
|
||||
const status = result.code === null ? 'null' : `0x${(result.code >>> 0).toString(16)}`
|
||||
expect(
|
||||
result,
|
||||
`Native host exited ${status} (${result.signal}); timedOut=${result.timedOut}\n${result.stdout}\n${result.stderr}`
|
||||
).toMatchObject({ code: 0, timedOut: false })
|
||||
expect(result.stdout).toContain('"phase":"complete"')
|
||||
}, 100_000)
|
||||
|
||||
it('counts a detached grandchild as part of the pane tree', async () => {
|
||||
const { proc, grandchildPid } = await spawnShellWithDetachedGrandchild()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user