Skip to content

fix: close pipe handles and free attribute list when conpty spawn fails #935

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
deepak1556 merged 1 commit into from
Jun 26, 2026
+37 −10
Merged

fix: close pipe handles and free attribute list when conpty spawn fails #935

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 37 additions & 10 deletions src/win/conpty.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ struct pty_baton {
HANDLE hOut;
HPCON hpc;

HANDLE hShell;
HANDLE hShell = nullptr;

pty_baton(int _id, HANDLE _hIn, HANDLE _hOut, HPCON _hpc) : id(_id), hIn(_hIn), hOut(_hOut), hpc(_hpc) {};
};
Expand Down Expand Up @@ -384,6 +384,14 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) {
ConnectNamedPipe(handle->hIn, nullptr);
ConnectNamedPipe(handle->hOut, nullptr);

// Close hIn/hOut on every exit so a throw doesn't leak the named-pipe server handles.
auto closePipeHandles = [handle]() {
CloseHandle(handle->hIn);
CloseHandle(handle->hOut);
handle->hIn = nullptr;
handle->hOut = nullptr;
};

// Attach the pseudoconsole to the client application we're creating
STARTUPINFOEXW siEx{0};
siEx.StartupInfo.cb = sizeof(STARTUPINFOEXW);
Expand All @@ -394,12 +402,14 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) {

SIZE_T size = 0;
InitializeProcThreadAttributeList(NULL, 1, 0, &size);
BYTE *attrList = new BYTE[size];
siEx.lpAttributeList = reinterpret_cast<PPROC_THREAD_ATTRIBUTE_LIST>(attrList);
std::unique_ptr<BYTE[]> attrList = std::make_unique<BYTE[]>(size);
siEx.lpAttributeList = reinterpret_cast<PPROC_THREAD_ATTRIBUTE_LIST>(attrList.get());

fSuccess = InitializeProcThreadAttributeList(siEx.lpAttributeList, 1, 0, &size);
if (!fSuccess) {
throw errorWithCode(info, "InitializeProcThreadAttributeList failed");
Napi::Error error = errorWithCode(info, "InitializeProcThreadAttributeList failed");
closePipeHandles();
throw error;
}
fSuccess = UpdateProcThreadAttribute(siEx.lpAttributeList,
0,
Expand All @@ -409,7 +419,10 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) {
NULL,
NULL);
if (!fSuccess) {
throw errorWithCode(info, "UpdateProcThreadAttribute failed");
Napi::Error error = errorWithCode(info, "UpdateProcThreadAttribute failed");
DeleteProcThreadAttributeList(siEx.lpAttributeList);
closePipeHandles();
throw error;
}

PROCESS_INFORMATION piClient{};
Expand All @@ -426,9 +439,14 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) {
&piClient // lpProcessInformation
);
if (!fSuccess) {
throw errorWithCode(info, "Cannot create process");
Napi::Error error = errorWithCode(info, "Cannot create process");
DeleteProcThreadAttributeList(siEx.lpAttributeList);
closePipeHandles();
throw error;
}

DeleteProcThreadAttributeList(siEx.lpAttributeList);

HANDLE hLibrary = LoadConptyDll(info, useConptyDll);
bool fLoadedDll = hLibrary != nullptr;
if (useConptyDll && fLoadedDll)
Expand All @@ -447,8 +465,7 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) {
// Close the thread handle to avoid resource leak
CloseHandle(piClient.hThread);
// Close the input read and output write handle of the pseudoconsole
CloseHandle(handle->hIn);
CloseHandle(handle->hOut);
closePipeHandles();

SetupExitCallback(env, exitCallback, handle);

Expand Down Expand Up @@ -550,7 +567,7 @@ static Napi::Value PtyKill(const Napi::CallbackInfo& info) {
const bool useConptyDll = info[1].As<Napi::Boolean>().Value();

std::lock_guard<std::mutex> lock(g_ptyHandlesMutex);
const pty_baton* handle = get_pty_baton(lock, id);
pty_baton* handle = get_pty_baton(lock, id);

if (handle != nullptr) {
HANDLE hLibrary = LoadConptyDll(info, useConptyDll);
Expand All @@ -565,7 +582,17 @@ static Napi::Value PtyKill(const Napi::CallbackInfo& info) {
pfnClosePseudoConsole(handle->hpc);
}
}
if (useConptyDll) {
// Defensive: if PtyConnect was never called (or failed before closing the
// pipe handles), release the named pipe server handles here.
if (handle->hIn != nullptr) {
CloseHandle(handle->hIn);
handle->hIn = nullptr;
}
if (handle->hOut != nullptr) {
CloseHandle(handle->hOut);
handle->hOut = nullptr;
}
if (useConptyDll && handle->hShell != nullptr) {
TerminateProcess(handle->hShell, 1);
}
}
Expand Down
Loading