From c84d987682f246d9b01872c8d7b4899f25f1cf47 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Wed, 15 Aug 2018 10:45:02 -0700 Subject: [PATCH 01/55] Begin prototyping with conpty support this is the minimum viable "it works" build - it's dirty, it relies on knowing features are coming in the RS5 SDK, and is super fragile. --- examples/fork/index.js | 11 +- scripts/install.js | 2 +- src/win/pty.cc | 267 +++++++++++++++++++++++++++++++++++++++-- src/windowsPtyAgent.ts | 7 +- 4 files changed, 274 insertions(+), 13 deletions(-) diff --git a/examples/fork/index.js b/examples/fork/index.js index b7f4c51fa..fa1450200 100644 --- a/examples/fork/index.js +++ b/examples/fork/index.js @@ -12,12 +12,17 @@ var ptyProcess = pty.spawn(shell, [], { }); ptyProcess.on('data', function(data) { - console.log(data); + // console.log(data); + process.stdout.write(data); }); ptyProcess.write('ls\r'); -ptyProcess.resize(100, 40); -ptyProcess.write('ls\r'); +// ptyProcess.resize(100, 40); +// ptyProcess.write('ls\r'); + +setTimeout(() => { + ptyProcess.write('ls\r'); +}, 2000); setTimeout(() => { ptyProcess.kill() diff --git a/scripts/install.js b/scripts/install.js index 416c1adf4..b6058cfc8 100644 --- a/scripts/install.js +++ b/scripts/install.js @@ -4,7 +4,7 @@ const os = require('os'); const path = require('path'); const spawn = require('child_process').spawn; -const p = spawn(os.platform() === 'win32' ? 'node-gyp.cmd' : 'node-gyp', ['rebuild'], { +const p = spawn(os.platform() === 'win32' ? 'node-gyp.cmd' : 'node-gyp', ['rebuild', '--debug'], { cwd: path.join(__dirname, '..'), stdio: 'inherit' }); diff --git a/src/win/pty.cc b/src/win/pty.cc index 77af9e423..7b958dbcf 100644 --- a/src/win/pty.cc +++ b/src/win/pty.cc @@ -16,9 +16,11 @@ #include #include #include - +#include +#include #include "path_util.h" - +#include "..\..\deps\winpty\src\shared\GenRandom.h" +#include "..\..\deps\winpty\src\shared\StringBuilder.h" /** * Misc */ @@ -111,9 +113,139 @@ static NAN_METHOD(PtyGetProcessList) { info.GetReturnValue().Set(result); } +HANDLE hIn, hOut, hpc; + +// Returns a new server named pipe. It has not yet been connected. +bool createDataServerPipe(bool write, std::wstring kind, HANDLE *hRead, HANDLE *hWrite, std::wstring &name) +{ + name = L"\\\\.\\pipe\\conpty2-" + kind; + + // const DWORD winOpenMode = ((openMode & OpenMode::Reading) ? PIPE_ACCESS_INBOUND : 0) | ((openMode & OpenMode::Writing) ? PIPE_ACCESS_OUTBOUND : 0) | FILE_FLAG_FIRST_PIPE_INSTANCE | FILE_FLAG_OVERLAPPED; + const DWORD winOpenMode = PIPE_ACCESS_INBOUND | PIPE_ACCESS_OUTBOUND | FILE_FLAG_FIRST_PIPE_INSTANCE/* | FILE_FLAG_OVERLAPPED */; + // const auto sd = createPipeSecurityDescriptorOwnerFullControl(); + // ASSERT(sd && "error creating data pipe SECURITY_DESCRIPTOR"); + SECURITY_ATTRIBUTES sa = {}; + sa.nLength = sizeof(sa); + // sa.lpSecurityDescriptor = sd.get(); + *hRead = CreateNamedPipeW( + name.c_str(), + /*dwOpenMode=*/winOpenMode, + /*dwPipeMode=*/PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_WAIT, + /*nMaxInstances=*/1, + /*nOutBufferSize=*/0, + /*nInBufferSize=*/0, + /*nDefaultTimeOut=*/30000, + &sa); + + // *hWrite = CreateFileW(name.c_str(), GENERIC_READ|GENERIC_WRITE, 0, 0, OPEN_EXISTING, 0, 0); + wprintf(L"%s:%x\n", name.c_str(), *hRead); + // Start an asynchronous connection attempt. + // m_connectEvent = createEvent(); + + // HANDLE ret = CreateEventW(nullptr, TRUE, FALSE, nullptr); + // OVERLAPPED m_connectOver = {}; + // memset(&m_connectOver, 0, sizeof(m_connectOver)); + // m_connectOver.hEvent = ret; + // BOOL success = ConnectNamedPipe(*hRead, &m_connectOver); + // printf("ConnectNamedPipe: %d\n", success); + // const auto err = GetLastError(); + // printf("ConnectNamedPipe: %d\n", err); + + // if (!success && err == ERROR_PIPE_CONNECTED) + // { + // success = TRUE; + // } + // if (success) + // { + // // TRACE("Server pipe [%s] connected", utf8FromWide(name).c_str()); + // // m_connectEvent.dispose(); + // CloseHandle(ret); + // // startPipeWorkers(); + // } + // else if (err != ERROR_IO_PENDING) + // { + // // ASSERT(false && "ConnectNamedPipe call failed"); + // // return false; + // } + + // ConnectNamedPipe(*hRead, nullptr); + // return success; + return true; + // NamedPipe &pipe = createNamedPipe(); + // pipe.openServerPipe( + // name.c_str(), + // write ? NamedPipe::OpenMode::Writing + // : NamedPipe::OpenMode::Reading, + // write ? 8192 : 0, + // write ? 0 : 256); + // if (!write) + // { + // pipe.setReadBufferSize(64 * 1024); + // } + // return pipe; +} + +typedef bool (*PFNCREATEPSEUDOCONSOLE)(COORD c, HANDLE hin, HANDLE hout, DWORD dwFlags, void* phpcon); + +HRESULT CreatePseudoConsoleAndHandles(COORD size, + _In_ DWORD dwFlags, + _Out_ HANDLE *phInput, + _Out_ HANDLE *phOutput, + _Out_ void *phPC, std::wstring& inName, std::wstring& outName) +{ + + HANDLE hLibrary = LoadLibraryExW(L"kernel32.dll", 0, 0); + bool fLoadedDll = hLibrary != nullptr; + bool success = false; + if (fLoadedDll) + { + PFNCREATEPSEUDOCONSOLE const pfnCreate = (PFNCREATEPSEUDOCONSOLE)GetProcAddress((HMODULE)hLibrary, "CreatePseudoConsole"); + if (pfnCreate) + { + if (phPC == NULL || phInput == NULL || phOutput == NULL) + { + return E_INVALIDARG; + } + + HANDLE outPipeOurSide; + HANDLE inPipeOurSide; + // HANDLE outPipePseudoConsoleSide; + // HANDLE inPipePseudoConsoleSide; + success = createDataServerPipe(true, L"in", &hIn, &inPipeOurSide, inName); + printf("createDataServerPipe0 %d;\n", success); + success = createDataServerPipe(true, L"out", &hOut, &outPipeOurSide, outName); + printf("createDataServerPipe1 %d;\n", success); + + return pfnCreate(size, hIn, hOut, dwFlags, phPC); + } + else{ + + printf("Failed to load PFNCREATEPSEUDOCONSOLE\n"); + } + + } + + else + { + printf("Failed to load kernel32.dll\n"); + } + + return E_FAIL; + +} + static NAN_METHOD(PtyStartProcess) { Nan::HandleScope scope; + // HANDLE hpc; + HANDLE hIn, hOut; + std::wstring inName, outName, str_cmdline; + BOOL fSuccess = FALSE; + std::unique_ptr mutableCommandline; + PROCESS_INFORMATION _piClient{}; + + DWORD dwExit = 0; + if (info.Length() != 7 || !info[0]->IsString() || !info[1]->IsString() || @@ -168,6 +300,19 @@ static NAN_METHOD(PtyStartProcess) { int rows = info[5]->Int32Value(); bool debug = info[6]->ToBoolean()->IsTrue(); + fSuccess = CreatePseudoConsoleAndHandles({(SHORT)cols, (SHORT)rows}, 0, &hIn, &hOut, (void *)&hpc, inName, outName); + if (fSuccess) + { + wprintf(L"succeeded:CreatePseudoConsoleAndHandles\n"); + } + else + { + auto gle = GetLastError(); + wprintf(L"CreatePseudoConsoleAndHandles GLE:%d\n", gle); + } + + + // Enable/disable debugging SetEnvironmentVariable(WINPTY_DBG_VARIABLE, debug ? "1" : NULL); // NULL = deletes variable @@ -221,13 +366,15 @@ static NAN_METHOD(PtyStartProcess) { marshal->Set(Nan::New("pty").ToLocalChecked(), Nan::New(InterlockedIncrement(&ptyCounter))); marshal->Set(Nan::New("fd").ToLocalChecked(), Nan::New(-1)); { - LPCWSTR coninPipeName = winpty_conin_name(pc); - std::wstring coninPipeNameWStr(coninPipeName); - std::string coninPipeNameStr(coninPipeNameWStr.begin(), coninPipeNameWStr.end()); + // LPCWSTR coninPipeName = winpty_conin_name(pc); + // std::wstring coninPipeNameWStr(coninPipeName); + std::string coninPipeNameStr(inName.begin(), inName.end()); + // marshal->Set(Nan::New("conin").ToLocalChecked(), Nan::New(coninPipeNameStr).ToLocalChecked()); marshal->Set(Nan::New("conin").ToLocalChecked(), Nan::New(coninPipeNameStr).ToLocalChecked()); - LPCWSTR conoutPipeName = winpty_conout_name(pc); - std::wstring conoutPipeNameWStr(conoutPipeName); - std::string conoutPipeNameStr(conoutPipeNameWStr.begin(), conoutPipeNameWStr.end()); + // LPCWSTR conoutPipeName = winpty_conout_name(pc); + // std::wstring conoutPipeNameWStr(conoutPipeName); + std::string conoutPipeNameStr(outName.begin(), outName.end()); + // marshal->Set(Nan::New("conout").ToLocalChecked(), Nan::New(conoutPipeNameStr).ToLocalChecked()); marshal->Set(Nan::New("conout").ToLocalChecked(), Nan::New(conoutPipeNameStr).ToLocalChecked()); } info.GetReturnValue().Set(marshal); @@ -240,6 +387,109 @@ static NAN_METHOD(PtyStartProcess) { delete cwd; } +static NAN_METHOD(PtyConnect) { + std::wstring str_cmdline; + BOOL fSuccess = FALSE; + std::unique_ptr mutableCommandline; + PROCESS_INFORMATION _piClient{}; + BOOL success = ConnectNamedPipe(hIn, nullptr); + // if (success) + // { + // wprintf(L"succeeded:ConnectNamedPipe\n"); + // } + // else + // { + // wprintf(L"GLE:ConnectNamedPipe%d\n", GetLastError()); + // } + success = ConnectNamedPipe(hOut, nullptr); + // if (success) + // { + // wprintf(L"succeeded:ConnectNamedPipe\n"); + // } + // else + // { + // wprintf(L"GLE:ConnectNamedPipe%d\n", GetLastError()); + // } + + auto foo = ProcThreadAttributeValue(22, FALSE, TRUE, FALSE); + + STARTUPINFOEXW siEx; + siEx = {0}; + siEx.StartupInfo.cb = sizeof(STARTUPINFOEXW); + size_t size; + InitializeProcThreadAttributeList(NULL, 1, 0, &size); + BYTE *attrList = new BYTE[size]; + printf("size:%d", size); + siEx.lpAttributeList = reinterpret_cast(attrList); + + fSuccess = InitializeProcThreadAttributeList(siEx.lpAttributeList, 1, 0, (PSIZE_T)&size); + // if (fSuccess) + // { + // wprintf(L"succeeded:InitializeProcThreadAttributeList\n"); + // } + // else + // { + // auto gle = GetLastError(); + // wprintf(L"InitializeProcThreadAttributeList GLE:%d\n", gle); + // } + + fSuccess = UpdateProcThreadAttribute(siEx.lpAttributeList, + 0, + foo, + hpc, + sizeof(void*), + NULL, + NULL); + if (fSuccess) + { + wprintf(L"succeeded:UpdateProcThreadAttribute\n"); + } + else + { + auto gle = GetLastError(); + wprintf(L"UpdateProcThreadAttribute GLE:%d\n", gle); + } + + // str_cmdline = cmdline; + // str_cmdline = L"c:\\windows\\system32\\cmd.exe"; + str_cmdline = L"powershell.exe"; + mutableCommandline = std::make_unique(str_cmdline.length() + 1); + // THROW_IF_NULL_ALLOC(mutableCommandline); + + HRESULT hr = StringCchCopyW(mutableCommandline.get(), str_cmdline.length() + 1, str_cmdline.c_str()); + // THROW_IF_FAILED(hr); + // DebugBreak(); + + fSuccess = !!CreateProcessW( + nullptr, + mutableCommandline.get(), + nullptr, // lpProcessAttributes + nullptr, // lpThreadAttributes + false, // bInheritHandles + EXTENDED_STARTUPINFO_PRESENT, // dwCreationFlags + nullptr, // lpEnvironment + nullptr, // lpCurrentDirectory + &siEx.StartupInfo, // lpStartupInfo + &_piClient // lpProcessInformation + ); + // if (fSuccess) + // { + // wprintf(L"succeeded:CreateProcessW\n"); + // } + // else + // { + // wprintf(L"GLE:CreateProcessW%d\n", GetLastError()); + // } + // wprintf(L"_piClient.hProcess %x\n", _piClient.hProcess); + // wprintf(L"_piClient.hThread %x\n", _piClient.hThread); + // wprintf(L"_piClient.dwProcessId %d\n", _piClient.dwProcessId); + // wprintf(L"_piClient.dwThreadId %d\n", _piClient.dwThreadId); + // WaitForSingleObject(_piClient.hProcess, 2000); + // GetExitCodeProcess(_piClient.hProcess, &dwExit); + // wprintf(L"_piClient.dwExit %d\n", dwExit); + +} + static NAN_METHOD(PtyResize) { Nan::HandleScope scope; @@ -302,6 +552,7 @@ static NAN_METHOD(PtyKill) { extern "C" void init(v8::Handle target) { Nan::HandleScope scope; Nan::SetMethod(target, "startProcess", PtyStartProcess); + Nan::SetMethod(target, "connect", PtyConnect); Nan::SetMethod(target, "resize", PtyResize); Nan::SetMethod(target, "kill", PtyKill); Nan::SetMethod(target, "getExitCode", PtyGetExitCode); diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts index dd32776e5..4d97f6df6 100644 --- a/src/windowsPtyAgent.ts +++ b/src/windowsPtyAgent.ts @@ -7,7 +7,7 @@ import * as path from 'path'; import { Socket } from 'net'; import { ArgvOrCommandLine } from './types'; -const pty = require(path.join('..', 'build', 'Release', 'pty.node')); +const pty = require(path.join('..', 'build', 'Debug', 'pty.node')); /** * Agent. Internal class. @@ -77,6 +77,11 @@ export class WindowsPtyAgent { this._inSocket.setEncoding('utf8'); this._inSocket.connect(term.conin); // TODO: Wait for ready event? + + // TODO: Do we *need* to timeout here or wait for the sockets to connect? + // or can we do this synchronously like this? + pty.connect(); + } public resize(cols: number, rows: number): void { From b10d40d162215e8ae80acba6fbf671c1f325f859 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 16 Aug 2018 14:22:04 -0700 Subject: [PATCH 02/55] General cleanup Clean out dead code, actually define some magic, leave some todo's in areas that need help --- src/win/pty.cc | 233 ++++++++++++++++--------------------------------- 1 file changed, 77 insertions(+), 156 deletions(-) diff --git a/src/win/pty.cc b/src/win/pty.cc index 7b958dbcf..f96995a2b 100644 --- a/src/win/pty.cc +++ b/src/win/pty.cc @@ -28,6 +28,18 @@ extern "C" void init(v8::Handle); #define WINPTY_DBG_VARIABLE TEXT("WINPTYDBG") +#ifndef PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE +// Taken from the RS5 Windows SDK, but redefined here in case we're targeting <= 17134 +#define PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE \ + ProcThreadAttributeValue(22, FALSE, TRUE, FALSE) + +typedef VOID* HPCON; +typedef HRESULT (*PFNCREATEPSEUDOCONSOLE)(COORD c, HANDLE hIn, HANDLE hOut, DWORD dwFlags, HPCON* phpcon); +typedef HRESULT (*PFNRESIZEPSEUDOCONSOLE)(HPCON hpc, COORD newSize); +typedef void (*PFNCLOSEPSEUDOCONSOLE)(HPCON hpc); + +#endif + /** * winpty */ @@ -113,21 +125,24 @@ static NAN_METHOD(PtyGetProcessList) { info.GetReturnValue().Set(result); } -HANDLE hIn, hOut, hpc; +// TODO these should probably not be globals +HANDLE hIn, hOut; +HPCON hpc; // Returns a new server named pipe. It has not yet been connected. -bool createDataServerPipe(bool write, std::wstring kind, HANDLE *hRead, HANDLE *hWrite, std::wstring &name) +bool createDataServerPipe(bool write, std::wstring kind, HANDLE* hServer, std::wstring &name) { + *hServer = INVALID_HANDLE_VALUE; + + // TODO generate unique names for each pipe name = L"\\\\.\\pipe\\conpty2-" + kind; - // const DWORD winOpenMode = ((openMode & OpenMode::Reading) ? PIPE_ACCESS_INBOUND : 0) | ((openMode & OpenMode::Writing) ? PIPE_ACCESS_OUTBOUND : 0) | FILE_FLAG_FIRST_PIPE_INSTANCE | FILE_FLAG_OVERLAPPED; const DWORD winOpenMode = PIPE_ACCESS_INBOUND | PIPE_ACCESS_OUTBOUND | FILE_FLAG_FIRST_PIPE_INSTANCE/* | FILE_FLAG_OVERLAPPED */; - // const auto sd = createPipeSecurityDescriptorOwnerFullControl(); - // ASSERT(sd && "error creating data pipe SECURITY_DESCRIPTOR"); + SECURITY_ATTRIBUTES sa = {}; sa.nLength = sizeof(sa); - // sa.lpSecurityDescriptor = sd.get(); - *hRead = CreateNamedPipeW( + + *hServer = CreateNamedPipeW( name.c_str(), /*dwOpenMode=*/winOpenMode, /*dwPipeMode=*/PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_WAIT, @@ -137,66 +152,19 @@ bool createDataServerPipe(bool write, std::wstring kind, HANDLE *hRead, HANDLE * /*nDefaultTimeOut=*/30000, &sa); - // *hWrite = CreateFileW(name.c_str(), GENERIC_READ|GENERIC_WRITE, 0, 0, OPEN_EXISTING, 0, 0); - wprintf(L"%s:%x\n", name.c_str(), *hRead); - // Start an asynchronous connection attempt. - // m_connectEvent = createEvent(); - - // HANDLE ret = CreateEventW(nullptr, TRUE, FALSE, nullptr); - // OVERLAPPED m_connectOver = {}; - // memset(&m_connectOver, 0, sizeof(m_connectOver)); - // m_connectOver.hEvent = ret; - // BOOL success = ConnectNamedPipe(*hRead, &m_connectOver); - // printf("ConnectNamedPipe: %d\n", success); - // const auto err = GetLastError(); - // printf("ConnectNamedPipe: %d\n", err); - - // if (!success && err == ERROR_PIPE_CONNECTED) - // { - // success = TRUE; - // } - // if (success) - // { - // // TRACE("Server pipe [%s] connected", utf8FromWide(name).c_str()); - // // m_connectEvent.dispose(); - // CloseHandle(ret); - // // startPipeWorkers(); - // } - // else if (err != ERROR_IO_PENDING) - // { - // // ASSERT(false && "ConnectNamedPipe call failed"); - // // return false; - // } - - // ConnectNamedPipe(*hRead, nullptr); - // return success; - return true; - // NamedPipe &pipe = createNamedPipe(); - // pipe.openServerPipe( - // name.c_str(), - // write ? NamedPipe::OpenMode::Writing - // : NamedPipe::OpenMode::Reading, - // write ? 8192 : 0, - // write ? 0 : 256); - // if (!write) - // { - // pipe.setReadBufferSize(64 * 1024); - // } - // return pipe; + return *hServer != INVALID_HANDLE_VALUE; } -typedef bool (*PFNCREATEPSEUDOCONSOLE)(COORD c, HANDLE hin, HANDLE hout, DWORD dwFlags, void* phpcon); - -HRESULT CreatePseudoConsoleAndHandles(COORD size, - _In_ DWORD dwFlags, - _Out_ HANDLE *phInput, - _Out_ HANDLE *phOutput, - _Out_ void *phPC, std::wstring& inName, std::wstring& outName) +HRESULT CreateNamedPipesAndPseudoConsole(COORD size, + DWORD dwFlags, + HANDLE *phInput, + HANDLE *phOutput, + HPCON* phPC, + std::wstring& inName, + std::wstring& outName) { - HANDLE hLibrary = LoadLibraryExW(L"kernel32.dll", 0, 0); bool fLoadedDll = hLibrary != nullptr; - bool success = false; if (fLoadedDll) { PFNCREATEPSEUDOCONSOLE const pfnCreate = (PFNCREATEPSEUDOCONSOLE)GetProcAddress((HMODULE)hLibrary, "CreatePseudoConsole"); @@ -207,38 +175,35 @@ HRESULT CreatePseudoConsoleAndHandles(COORD size, return E_INVALIDARG; } - HANDLE outPipeOurSide; - HANDLE inPipeOurSide; - // HANDLE outPipePseudoConsoleSide; - // HANDLE inPipePseudoConsoleSide; - success = createDataServerPipe(true, L"in", &hIn, &inPipeOurSide, inName); - printf("createDataServerPipe0 %d;\n", success); - success = createDataServerPipe(true, L"out", &hOut, &outPipeOurSide, outName); - printf("createDataServerPipe1 %d;\n", success); - - return pfnCreate(size, hIn, hOut, dwFlags, phPC); + bool success = createDataServerPipe(true, L"in", phInput, inName); + if (!success) + { + return HRESULT_FROM_WIN32(GetLastError()); + } + success = createDataServerPipe(false, L"out", phOutput, outName); + if (!success) + { + return HRESULT_FROM_WIN32(GetLastError()); + } + return pfnCreate(size, *phInput, *phOutput, dwFlags, phPC); } - else{ - - printf("Failed to load PFNCREATEPSEUDOCONSOLE\n"); + else + { + // Failed to find CreatePseudoConsole in kernel32. This is likely because + // the user is not running a build of Windows that supports that API. + // We should fall back to winpty in this case. + return HRESULT_FROM_WIN32(GetLastError()); } - - } - - else - { - printf("Failed to load kernel32.dll\n"); } - return E_FAIL; - + // Failed to find kernel32. This is realy unlikely - honestly no idea how + // this is even possible to hit. But if it does happen, fall back to winpty. + return HRESULT_FROM_WIN32(GetLastError()); } static NAN_METHOD(PtyStartProcess) { Nan::HandleScope scope; - // HANDLE hpc; - HANDLE hIn, hOut; std::wstring inName, outName, str_cmdline; BOOL fSuccess = FALSE; std::unique_ptr mutableCommandline; @@ -300,19 +265,19 @@ static NAN_METHOD(PtyStartProcess) { int rows = info[5]->Int32Value(); bool debug = info[6]->ToBoolean()->IsTrue(); - fSuccess = CreatePseudoConsoleAndHandles({(SHORT)cols, (SHORT)rows}, 0, &hIn, &hOut, (void *)&hpc, inName, outName); - if (fSuccess) + HRESULT hr = CreateNamedPipesAndPseudoConsole({(SHORT)cols, (SHORT)rows}, 0, &hIn, &hOut, &hpc, inName, outName); + + if (SUCCEEDED(hr)) { - wprintf(L"succeeded:CreatePseudoConsoleAndHandles\n"); + // We were able to instantiate a conpty, yay! + // TODO } else { - auto gle = GetLastError(); - wprintf(L"CreatePseudoConsoleAndHandles GLE:%d\n", gle); + // We weren't able to start conpty. Fall back to winpty. + // TODO } - - // Enable/disable debugging SetEnvironmentVariable(WINPTY_DBG_VARIABLE, debug ? "1" : NULL); // NULL = deletes variable @@ -369,12 +334,11 @@ static NAN_METHOD(PtyStartProcess) { // LPCWSTR coninPipeName = winpty_conin_name(pc); // std::wstring coninPipeNameWStr(coninPipeName); std::string coninPipeNameStr(inName.begin(), inName.end()); - // marshal->Set(Nan::New("conin").ToLocalChecked(), Nan::New(coninPipeNameStr).ToLocalChecked()); marshal->Set(Nan::New("conin").ToLocalChecked(), Nan::New(coninPipeNameStr).ToLocalChecked()); + // LPCWSTR conoutPipeName = winpty_conout_name(pc); // std::wstring conoutPipeNameWStr(conoutPipeName); std::string conoutPipeNameStr(outName.begin(), outName.end()); - // marshal->Set(Nan::New("conout").ToLocalChecked(), Nan::New(conoutPipeNameStr).ToLocalChecked()); marshal->Set(Nan::New("conout").ToLocalChecked(), Nan::New(conoutPipeNameStr).ToLocalChecked()); } info.GetReturnValue().Set(marshal); @@ -388,109 +352,62 @@ static NAN_METHOD(PtyStartProcess) { } static NAN_METHOD(PtyConnect) { + // If we're working with conpty's we need to call ConnectNamedPipe here AFTER + // the Socket has attempted to connect to the other end, then actually + // spawn the process here. + std::wstring str_cmdline; BOOL fSuccess = FALSE; std::unique_ptr mutableCommandline; PROCESS_INFORMATION _piClient{}; BOOL success = ConnectNamedPipe(hIn, nullptr); - // if (success) - // { - // wprintf(L"succeeded:ConnectNamedPipe\n"); - // } - // else - // { - // wprintf(L"GLE:ConnectNamedPipe%d\n", GetLastError()); - // } success = ConnectNamedPipe(hOut, nullptr); - // if (success) - // { - // wprintf(L"succeeded:ConnectNamedPipe\n"); - // } - // else - // { - // wprintf(L"GLE:ConnectNamedPipe%d\n", GetLastError()); - // } - - auto foo = ProcThreadAttributeValue(22, FALSE, TRUE, FALSE); + // Attach the pseudoconsole to the client application we're creating STARTUPINFOEXW siEx; siEx = {0}; siEx.StartupInfo.cb = sizeof(STARTUPINFOEXW); size_t size; InitializeProcThreadAttributeList(NULL, 1, 0, &size); BYTE *attrList = new BYTE[size]; - printf("size:%d", size); siEx.lpAttributeList = reinterpret_cast(attrList); fSuccess = InitializeProcThreadAttributeList(siEx.lpAttributeList, 1, 0, (PSIZE_T)&size); - // if (fSuccess) - // { - // wprintf(L"succeeded:InitializeProcThreadAttributeList\n"); - // } - // else - // { - // auto gle = GetLastError(); - // wprintf(L"InitializeProcThreadAttributeList GLE:%d\n", gle); - // } - fSuccess = UpdateProcThreadAttribute(siEx.lpAttributeList, 0, - foo, + PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE, hpc, - sizeof(void*), + sizeof(HPCON), NULL, NULL); - if (fSuccess) - { - wprintf(L"succeeded:UpdateProcThreadAttribute\n"); - } - else - { - auto gle = GetLastError(); - wprintf(L"UpdateProcThreadAttribute GLE:%d\n", gle); - } - // str_cmdline = cmdline; - // str_cmdline = L"c:\\windows\\system32\\cmd.exe"; + // You need to pass a MUTABLE commandline to CreateProcess, so convert our const wchar_t* here. str_cmdline = L"powershell.exe"; mutableCommandline = std::make_unique(str_cmdline.length() + 1); - // THROW_IF_NULL_ALLOC(mutableCommandline); - HRESULT hr = StringCchCopyW(mutableCommandline.get(), str_cmdline.length() + 1, str_cmdline.c_str()); - // THROW_IF_FAILED(hr); - // DebugBreak(); fSuccess = !!CreateProcessW( nullptr, mutableCommandline.get(), nullptr, // lpProcessAttributes nullptr, // lpThreadAttributes - false, // bInheritHandles + false, // bInheritHandles VERY IMPORTANT that this is false EXTENDED_STARTUPINFO_PRESENT, // dwCreationFlags nullptr, // lpEnvironment nullptr, // lpCurrentDirectory &siEx.StartupInfo, // lpStartupInfo &_piClient // lpProcessInformation ); - // if (fSuccess) - // { - // wprintf(L"succeeded:CreateProcessW\n"); - // } - // else - // { - // wprintf(L"GLE:CreateProcessW%d\n", GetLastError()); - // } - // wprintf(L"_piClient.hProcess %x\n", _piClient.hProcess); - // wprintf(L"_piClient.hThread %x\n", _piClient.hThread); - // wprintf(L"_piClient.dwProcessId %d\n", _piClient.dwProcessId); - // wprintf(L"_piClient.dwThreadId %d\n", _piClient.dwThreadId); - // WaitForSingleObject(_piClient.hProcess, 2000); - // GetExitCodeProcess(_piClient.hProcess, &dwExit); - // wprintf(L"_piClient.dwExit %d\n", dwExit); + + // TODO: return the information about the client application out to the caller? } static NAN_METHOD(PtyResize) { + + // TODO: If the pty is backed by conpty, call ResizePseudoConsole + // (using LoadLibrary/GetProcAddress to find ResizePseudoConsole if it exists) + Nan::HandleScope scope; if (info.Length() != 3 || @@ -521,6 +438,10 @@ static NAN_METHOD(PtyResize) { } static NAN_METHOD(PtyKill) { + + // TODO: If the pty is backed by conpty, call ClosePseudoConsole + // (using LoadLibrary/GetProcAddress to find ClosePseudoConsole if it exists) + Nan::HandleScope scope; if (info.Length() != 2 || From 671e0388a4d1f1b42ddb276263139c920deb059c Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 22 Aug 2018 14:23:49 -0700 Subject: [PATCH 03/55] Add Windows SDK install instructions --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 380e76db6..936464a4a 100644 --- a/README.md +++ b/README.md @@ -67,6 +67,8 @@ npm run tsc npm install --global --production windows-build-tools ``` +The Windows SDK is also needed which can be [downloaded here](https://developer.microsoft.com/en-us/windows/downloads/windows-10-sdk). Only the "Desktop C++ Apps" components are needed to be installed. + ## Debugging On Windows, you can show the winpty agent console window by adding the environment variable `WINPTY_SHOW_CONSOLE=1` to the pty's environment. See https://github.com/rprichard/winpty#debugging-winpty for more information. From 1b6ab0b52e05cf87528a444c59bb31381d7f9430 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 22 Aug 2018 14:24:10 -0700 Subject: [PATCH 04/55] Fix npm test (temporary) --- src/unixTerminal.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/unixTerminal.ts b/src/unixTerminal.ts index 99c308df4..251381ad0 100644 --- a/src/unixTerminal.ts +++ b/src/unixTerminal.ts @@ -17,7 +17,7 @@ declare interface INativePty { pty: string; } -const pty = require(path.join('..', 'build', 'Release', 'pty.node')); +const pty = require(path.join('..', 'build', 'Debug', 'pty.node')); const DEFAULT_FILE = 'sh'; const DEFAULT_NAME = 'xterm'; From 7e6c4a73ccaab6b34cb3bf231929be0d0434a50f Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 22 Aug 2018 14:24:36 -0700 Subject: [PATCH 05/55] Close conpty on PtyKill --- src/win/pty.cc | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/win/pty.cc b/src/win/pty.cc index f96995a2b..554a5e0cb 100644 --- a/src/win/pty.cc +++ b/src/win/pty.cc @@ -442,6 +442,18 @@ static NAN_METHOD(PtyKill) { // TODO: If the pty is backed by conpty, call ClosePseudoConsole // (using LoadLibrary/GetProcAddress to find ClosePseudoConsole if it exists) + // TODO: Share hLibrary between functions + HANDLE hLibrary = LoadLibraryExW(L"kernel32.dll", 0, 0); + bool fLoadedDll = hLibrary != nullptr; + if (fLoadedDll) + { + PFNCLOSEPSEUDOCONSOLE const pfnClosePseudoConsole = (PFNCLOSEPSEUDOCONSOLE)GetProcAddress((HMODULE)hLibrary, "CreatePseudoConsole"); + if (pfnClosePseudoConsole) + { + pfnClosePseudoConsole(hpc); + } + } + Nan::HandleScope scope; if (info.Length() != 2 || From 46ea91fd4384ccbb54d173de0f5675b4348f9f1e Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 22 Aug 2018 14:32:49 -0700 Subject: [PATCH 06/55] Properly clean up process in fork example --- examples/fork/index.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/examples/fork/index.js b/examples/fork/index.js index fa1450200..35d6c4297 100644 --- a/examples/fork/index.js +++ b/examples/fork/index.js @@ -24,6 +24,10 @@ setTimeout(() => { ptyProcess.write('ls\r'); }, 2000); +process.on('exit', () => { + ptyProcess.kill(); +}); + setTimeout(() => { - ptyProcess.kill() + process.exit(); }, 5000); From d6bf31f91c01d8cd0aa97660f7332316e16b4aef Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 22 Aug 2018 14:52:10 -0700 Subject: [PATCH 07/55] Handle resize for conpty --- src/win/pty.cc | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/win/pty.cc b/src/win/pty.cc index 554a5e0cb..a90d2d768 100644 --- a/src/win/pty.cc +++ b/src/win/pty.cc @@ -422,6 +422,20 @@ static NAN_METHOD(PtyResize) { int cols = info[1]->Int32Value(); int rows = info[2]->Int32Value(); + + // TODO: Share hLibrary between functions + HANDLE hLibrary = LoadLibraryExW(L"kernel32.dll", 0, 0); + bool fLoadedDll = hLibrary != nullptr; + if (fLoadedDll) + { + PFNRESIZEPSEUDOCONSOLE const pfnResizePseudoConsole = (PFNRESIZEPSEUDOCONSOLE)GetProcAddress((HMODULE)hLibrary, "ResizePseudoConsole"); + if (pfnResizePseudoConsole) + { + COORD size = {cols, rows}; + pfnResizePseudoConsole(hpc, size); + } + } + winpty_t *pc = get_pipe_handle(handle); if (pc == nullptr) { From 43fffd4cc376858e6eaa8d37af83249094a73420 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Mon, 8 Oct 2018 07:25:21 -0700 Subject: [PATCH 08/55] Comment out most winpty code --- examples/fork/index.js | 14 +-- src/win/pty.cc | 221 +++++++++++++++++++++-------------------- src/windowsPtyAgent.ts | 6 +- 3 files changed, 122 insertions(+), 119 deletions(-) diff --git a/examples/fork/index.js b/examples/fork/index.js index 35d6c4297..9ed0e2c81 100644 --- a/examples/fork/index.js +++ b/examples/fork/index.js @@ -1,12 +1,12 @@ var os = require('os'); var pty = require('../..'); -var shell = os.platform() === 'win32' ? 'powershell.exe' : 'bash'; +var shell = os.platform() === 'win32' ? 'cmd.exe' : 'bash'; var ptyProcess = pty.spawn(shell, [], { - name: 'xterm-color', + name: 'xterm-256color', cols: 80, - rows: 30, + rows: 19, cwd: process.env.HOME, env: process.env }); @@ -16,12 +16,12 @@ ptyProcess.on('data', function(data) { process.stdout.write(data); }); -ptyProcess.write('ls\r'); -// ptyProcess.resize(100, 40); +ptyProcess.write('dir\r'); // ptyProcess.write('ls\r'); setTimeout(() => { - ptyProcess.write('ls\r'); + ptyProcess.resize(30, 19); + ptyProcess.write('dir\r'); }, 2000); process.on('exit', () => { @@ -30,4 +30,4 @@ process.on('exit', () => { setTimeout(() => { process.exit(); -}, 5000); +}, 4000); diff --git a/src/win/pty.cc b/src/win/pty.cc index a90d2d768..7b15f126a 100644 --- a/src/win/pty.cc +++ b/src/win/pty.cc @@ -15,7 +15,6 @@ #include #include #include -#include #include #include #include "path_util.h" @@ -26,7 +25,7 @@ */ extern "C" void init(v8::Handle); -#define WINPTY_DBG_VARIABLE TEXT("WINPTYDBG") +// #define WINPTY_DBG_VARIABLE TEXT("WINPTYDBG") #ifndef PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE // Taken from the RS5 Windows SDK, but redefined here in case we're targeting <= 17134 @@ -43,45 +42,45 @@ typedef void (*PFNCLOSEPSEUDOCONSOLE)(HPCON hpc); /** * winpty */ -static std::vector ptyHandles; +// static std::vector ptyHandles; static volatile LONG ptyCounter; /** * Helpers */ -static winpty_t *get_pipe_handle(int handle) { - for (size_t i = 0; i < ptyHandles.size(); ++i) { - winpty_t *ptyHandle = ptyHandles[i]; - int current = (int)winpty_agent_process(ptyHandle); - if (current == handle) { - return ptyHandle; - } - } - return nullptr; -} - -static bool remove_pipe_handle(int handle) { - for (size_t i = 0; i < ptyHandles.size(); ++i) { - winpty_t *ptyHandle = ptyHandles[i]; - if ((int)winpty_agent_process(ptyHandle) == handle) { - winpty_free(ptyHandle); - ptyHandles.erase(ptyHandles.begin() + i); - ptyHandle = nullptr; - return true; - } - } - return false; -} - -void throw_winpty_error(const char *generalMsg, winpty_error_ptr_t error_ptr) { - std::stringstream why; - std::wstring msg(winpty_error_msg(error_ptr)); - std::string msg_(msg.begin(), msg.end()); - why << generalMsg << ": " << msg_; - Nan::ThrowError(why.str().c_str()); - winpty_error_free(error_ptr); -} +// static winpty_t *get_pipe_handle(int handle) { +// for (size_t i = 0; i < ptyHandles.size(); ++i) { +// winpty_t *ptyHandle = ptyHandles[i]; +// int current = (int)winpty_agent_process(ptyHandle); +// if (current == handle) { +// return ptyHandle; +// } +// } +// return nullptr; +// } + +// static bool remove_pipe_handle(int handle) { +// for (size_t i = 0; i < ptyHandles.size(); ++i) { +// winpty_t *ptyHandle = ptyHandles[i]; +// if ((int)winpty_agent_process(ptyHandle) == handle) { +// winpty_free(ptyHandle); +// ptyHandles.erase(ptyHandles.begin() + i); +// ptyHandle = nullptr; +// return true; +// } +// } +// return false; +// } + +// void throw_winpty_error(const char *generalMsg, winpty_error_ptr_t error_ptr) { +// std::stringstream why; +// std::wstring msg(winpty_error_msg(error_ptr)); +// std::string msg_(msg.begin(), msg.end()); +// why << generalMsg << ": " << msg_; +// Nan::ThrowError(why.str().c_str()); +// winpty_error_free(error_ptr); +// } static NAN_METHOD(PtyGetExitCode) { Nan::HandleScope scope; @@ -109,20 +108,20 @@ static NAN_METHOD(PtyGetProcessList) { int pid = info[0]->Int32Value(); - winpty_t *pc = get_pipe_handle(pid); - if (pc == nullptr) { + // winpty_t *pc = get_pipe_handle(pid); + // if (pc == nullptr) { info.GetReturnValue().Set(Nan::New(0)); return; - } - int processList[64]; - const int processCount = 64; - int actualCount = winpty_get_console_process_list(pc, processList, processCount, nullptr); - - v8::Local result = Nan::New(actualCount); - for (uint32_t i = 0; i < actualCount; i++) { - Nan::Set(result, i, Nan::New(processList[i])); - } - info.GetReturnValue().Set(result); + // } + // int processList[64]; + // const int processCount = 64; + // int actualCount = winpty_get_console_process_list(pc, processList, processCount, nullptr); + + // v8::Local result = Nan::New(actualCount); + // for (uint32_t i = 0; i < actualCount; i++) { + // Nan::Set(result, i, Nan::New(processList[i])); + // } + // info.GetReturnValue().Set(result); } // TODO these should probably not be globals @@ -279,55 +278,57 @@ static NAN_METHOD(PtyStartProcess) { } // Enable/disable debugging - SetEnvironmentVariable(WINPTY_DBG_VARIABLE, debug ? "1" : NULL); // NULL = deletes variable + // SetEnvironmentVariable(WINPTY_DBG_VARIABLE, debug ? "1" : NULL); // NULL = deletes variable // Create winpty config - winpty_error_ptr_t error_ptr = nullptr; - winpty_config_t* winpty_config = winpty_config_new(0, &error_ptr); - if (winpty_config == nullptr) { - throw_winpty_error("Error creating WinPTY config", error_ptr); - goto cleanup; - } - winpty_error_free(error_ptr); - - // Set pty size on config - winpty_config_set_initial_size(winpty_config, cols, rows); - - // Start the pty agent - winpty_t *pc = winpty_open(winpty_config, &error_ptr); - winpty_config_free(winpty_config); - if (pc == nullptr) { - throw_winpty_error("Error launching WinPTY agent", error_ptr); - goto cleanup; - } - winpty_error_free(error_ptr); + // winpty_error_ptr_t error_ptr = nullptr; + // winpty_config_t* winpty_config = winpty_config_new(0, &error_ptr); + // if (winpty_config == nullptr) { + // throw_winpty_error("Error creating WinPTY config", error_ptr); + // goto cleanup; + // } + // winpty_error_free(error_ptr); + + // // Set pty size on config + // winpty_config_set_initial_size(winpty_config, cols, rows); + + // // Start the pty agent + // winpty_t *pc = winpty_open(winpty_config, &error_ptr); + // winpty_config_free(winpty_config); + // if (pc == nullptr) { + // throw_winpty_error("Error launching WinPTY agent", error_ptr); + // goto cleanup; + // } + // winpty_error_free(error_ptr); // Save pty struct for later use - ptyHandles.insert(ptyHandles.end(), pc); + // ptyHandles.insert(ptyHandles.end(), pc); // Create winpty spawn config - winpty_spawn_config_t* config = winpty_spawn_config_new(WINPTY_SPAWN_FLAG_AUTO_SHUTDOWN, shellpath.c_str(), cmdline, cwd, env.c_str(), &error_ptr); - if (config == nullptr) { - throw_winpty_error("Error creating WinPTY spawn config", error_ptr); - goto cleanup; - } - winpty_error_free(error_ptr); + // winpty_spawn_config_t* config = winpty_spawn_config_new(WINPTY_SPAWN_FLAG_AUTO_SHUTDOWN, shellpath.c_str(), cmdline, cwd, env.c_str(), &error_ptr); + // if (config == nullptr) { + // throw_winpty_error("Error creating WinPTY spawn config", error_ptr); + // goto cleanup; + // } + // winpty_error_free(error_ptr); // Spawn the new process - HANDLE handle = nullptr; - BOOL spawnSuccess = winpty_spawn(pc, config, &handle, nullptr, nullptr, &error_ptr); - winpty_spawn_config_free(config); - if (!spawnSuccess) { - throw_winpty_error("Unable to start terminal process", error_ptr); - goto cleanup; - } - winpty_error_free(error_ptr); + // HANDLE handle = nullptr; + // BOOL spawnSuccess = winpty_spawn(pc, config, &handle, nullptr, nullptr, &error_ptr); + // winpty_spawn_config_free(config); + // if (!spawnSuccess) { + // throw_winpty_error("Unable to start terminal process", error_ptr); + // goto cleanup; + // } + // winpty_error_free(error_ptr); // Set return values v8::Local marshal = Nan::New(); - marshal->Set(Nan::New("innerPid").ToLocalChecked(), Nan::New((int)GetProcessId(handle))); - marshal->Set(Nan::New("innerPidHandle").ToLocalChecked(), Nan::New((int)handle)); - marshal->Set(Nan::New("pid").ToLocalChecked(), Nan::New((int)winpty_agent_process(pc))); + // TODO: Pull in innerPid, innerPidHandle(?) + // marshal->Set(Nan::New("innerPid").ToLocalChecked(), Nan::New((int)GetProcessId(handle))); + // marshal->Set(Nan::New("innerPidHandle").ToLocalChecked(), Nan::New((int)handle)); + // TODO: Pull in pid + // marshal->Set(Nan::New("pid").ToLocalChecked(), Nan::New((int)winpty_agent_process(pc))); marshal->Set(Nan::New("pty").ToLocalChecked(), Nan::New(InterlockedIncrement(&ptyCounter))); marshal->Set(Nan::New("fd").ToLocalChecked(), Nan::New(-1)); { @@ -436,17 +437,17 @@ static NAN_METHOD(PtyResize) { } } - winpty_t *pc = get_pipe_handle(handle); + // winpty_t *pc = get_pipe_handle(handle); - if (pc == nullptr) { - Nan::ThrowError("The pty doesn't appear to exist"); - return; - } - BOOL success = winpty_set_size(pc, cols, rows, nullptr); - if (!success) { - Nan::ThrowError("The pty could not be resized"); - return; - } + // if (pc == nullptr) { + // Nan::ThrowError("The pty doesn't appear to exist"); + // return; + // } + // BOOL success = winpty_set_size(pc, cols, rows, nullptr); + // if (!success) { + // Nan::ThrowError("The pty could not be resized"); + // return; + // } return info.GetReturnValue().SetUndefined(); } @@ -470,24 +471,24 @@ static NAN_METHOD(PtyKill) { Nan::HandleScope scope; - if (info.Length() != 2 || - !info[0]->IsNumber() || - !info[1]->IsNumber()) { - Nan::ThrowError("Usage: pty.kill(pid, innerPidHandle)"); - return; - } + // if (info.Length() != 2 || + // !info[0]->IsNumber() || + // !info[1]->IsNumber()) { + // Nan::ThrowError("Usage: pty.kill(pid, innerPidHandle)"); + // return; + // } - int handle = info[0]->Int32Value(); - HANDLE innerPidHandle = (HANDLE)info[1]->Int32Value(); + // int handle = info[0]->Int32Value(); + // HANDLE innerPidHandle = (HANDLE)info[1]->Int32Value(); - winpty_t *pc = get_pipe_handle(handle); - if (pc == nullptr) { - Nan::ThrowError("Pty seems to have been killed already"); - return; - } + // winpty_t *pc = get_pipe_handle(handle); + // if (pc == nullptr) { + // Nan::ThrowError("Pty seems to have been killed already"); + // return; + // } - assert(remove_pipe_handle(handle)); - CloseHandle(innerPidHandle); + // assert(remove_pipe_handle(handle)); + // CloseHandle(innerPidHandle); return info.GetReturnValue().SetUndefined(); } diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts index 4d97f6df6..be0857e08 100644 --- a/src/windowsPtyAgent.ts +++ b/src/windowsPtyAgent.ts @@ -85,7 +85,8 @@ export class WindowsPtyAgent { } public resize(cols: number, rows: number): void { - pty.resize(this._pid, cols, rows); + // TODO: Don't pass 0 in after pid is getting filled correctly + pty.resize(this._pid || 0, cols, rows); } public kill(): void { @@ -93,7 +94,8 @@ export class WindowsPtyAgent { this._inSocket.writable = false; this._outSocket.readable = false; this._outSocket.writable = false; - const processList: number[] = pty.getProcessList(this._pid); + // TODO: Don't pass in 0! + const processList: number[] = pty.getProcessList(this._pid || 0); // Tell the agent to kill the pty, this releases handles to the process pty.kill(this._pid, this._innerPidHandle); // Since pty.kill will kill most processes by itself and process IDs can be From 714f84d67b4c734d5b86f41ba3409d57c7c86c16 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 11 Oct 2018 16:59:49 -0700 Subject: [PATCH 09/55] Refactors, move code to winpty.cc and conpty.cc --- binding.gyp | 130 ++++++++------ src/win/{pty.cc => conpty.cc} | 133 +-------------- src/win/winpty.cc | 311 ++++++++++++++++++++++++++++++++++ 3 files changed, 393 insertions(+), 181 deletions(-) rename src/win/{pty.cc => conpty.cc} (77%) create mode 100644 src/win/winpty.cc diff --git a/binding.gyp b/binding.gyp index 9755fc0d1..681a26d82 100644 --- a/binding.gyp +++ b/binding.gyp @@ -1,57 +1,83 @@ { - 'targets': [{ - 'target_name': 'pty', - 'include_dirs' : [ - '); -// #define WINPTY_DBG_VARIABLE TEXT("WINPTYDBG") -#ifndef PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE +extern "C" void init(v8::Handle); + // Taken from the RS5 Windows SDK, but redefined here in case we're targeting <= 17134 +#ifndef PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE #define PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE \ ProcThreadAttributeValue(22, FALSE, TRUE, FALSE) @@ -39,49 +36,8 @@ typedef void (*PFNCLOSEPSEUDOCONSOLE)(HPCON hpc); #endif -/** -* winpty -*/ -// static std::vector ptyHandles; static volatile LONG ptyCounter; -/** -* Helpers -*/ - -// static winpty_t *get_pipe_handle(int handle) { -// for (size_t i = 0; i < ptyHandles.size(); ++i) { -// winpty_t *ptyHandle = ptyHandles[i]; -// int current = (int)winpty_agent_process(ptyHandle); -// if (current == handle) { -// return ptyHandle; -// } -// } -// return nullptr; -// } - -// static bool remove_pipe_handle(int handle) { -// for (size_t i = 0; i < ptyHandles.size(); ++i) { -// winpty_t *ptyHandle = ptyHandles[i]; -// if ((int)winpty_agent_process(ptyHandle) == handle) { -// winpty_free(ptyHandle); -// ptyHandles.erase(ptyHandles.begin() + i); -// ptyHandle = nullptr; -// return true; -// } -// } -// return false; -// } - -// void throw_winpty_error(const char *generalMsg, winpty_error_ptr_t error_ptr) { -// std::stringstream why; -// std::wstring msg(winpty_error_msg(error_ptr)); -// std::string msg_(msg.begin(), msg.end()); -// why << generalMsg << ": " << msg_; -// Nan::ThrowError(why.str().c_str()); -// winpty_error_free(error_ptr); -// } - static NAN_METHOD(PtyGetExitCode) { Nan::HandleScope scope; @@ -277,51 +233,6 @@ static NAN_METHOD(PtyStartProcess) { // TODO } - // Enable/disable debugging - // SetEnvironmentVariable(WINPTY_DBG_VARIABLE, debug ? "1" : NULL); // NULL = deletes variable - - // Create winpty config - // winpty_error_ptr_t error_ptr = nullptr; - // winpty_config_t* winpty_config = winpty_config_new(0, &error_ptr); - // if (winpty_config == nullptr) { - // throw_winpty_error("Error creating WinPTY config", error_ptr); - // goto cleanup; - // } - // winpty_error_free(error_ptr); - - // // Set pty size on config - // winpty_config_set_initial_size(winpty_config, cols, rows); - - // // Start the pty agent - // winpty_t *pc = winpty_open(winpty_config, &error_ptr); - // winpty_config_free(winpty_config); - // if (pc == nullptr) { - // throw_winpty_error("Error launching WinPTY agent", error_ptr); - // goto cleanup; - // } - // winpty_error_free(error_ptr); - - // Save pty struct for later use - // ptyHandles.insert(ptyHandles.end(), pc); - - // Create winpty spawn config - // winpty_spawn_config_t* config = winpty_spawn_config_new(WINPTY_SPAWN_FLAG_AUTO_SHUTDOWN, shellpath.c_str(), cmdline, cwd, env.c_str(), &error_ptr); - // if (config == nullptr) { - // throw_winpty_error("Error creating WinPTY spawn config", error_ptr); - // goto cleanup; - // } - // winpty_error_free(error_ptr); - - // Spawn the new process - // HANDLE handle = nullptr; - // BOOL spawnSuccess = winpty_spawn(pc, config, &handle, nullptr, nullptr, &error_ptr); - // winpty_spawn_config_free(config); - // if (!spawnSuccess) { - // throw_winpty_error("Unable to start terminal process", error_ptr); - // goto cleanup; - // } - // winpty_error_free(error_ptr); - // Set return values v8::Local marshal = Nan::New(); // TODO: Pull in innerPid, innerPidHandle(?) @@ -405,10 +316,6 @@ static NAN_METHOD(PtyConnect) { } static NAN_METHOD(PtyResize) { - - // TODO: If the pty is backed by conpty, call ResizePseudoConsole - // (using LoadLibrary/GetProcAddress to find ResizePseudoConsole if it exists) - Nan::HandleScope scope; if (info.Length() != 3 || @@ -437,22 +344,11 @@ static NAN_METHOD(PtyResize) { } } - // winpty_t *pc = get_pipe_handle(handle); - - // if (pc == nullptr) { - // Nan::ThrowError("The pty doesn't appear to exist"); - // return; - // } - // BOOL success = winpty_set_size(pc, cols, rows, nullptr); - // if (!success) { - // Nan::ThrowError("The pty could not be resized"); - // return; - // } - return info.GetReturnValue().SetUndefined(); } static NAN_METHOD(PtyKill) { + Nan::HandleScope scope; // TODO: If the pty is backed by conpty, call ClosePseudoConsole // (using LoadLibrary/GetProcAddress to find ClosePseudoConsole if it exists) @@ -469,27 +365,6 @@ static NAN_METHOD(PtyKill) { } } - Nan::HandleScope scope; - - // if (info.Length() != 2 || - // !info[0]->IsNumber() || - // !info[1]->IsNumber()) { - // Nan::ThrowError("Usage: pty.kill(pid, innerPidHandle)"); - // return; - // } - - // int handle = info[0]->Int32Value(); - // HANDLE innerPidHandle = (HANDLE)info[1]->Int32Value(); - - // winpty_t *pc = get_pipe_handle(handle); - // if (pc == nullptr) { - // Nan::ThrowError("Pty seems to have been killed already"); - // return; - // } - - // assert(remove_pipe_handle(handle)); - // CloseHandle(innerPidHandle); - return info.GetReturnValue().SetUndefined(); } diff --git a/src/win/winpty.cc b/src/win/winpty.cc new file mode 100644 index 000000000..77af9e423 --- /dev/null +++ b/src/win/winpty.cc @@ -0,0 +1,311 @@ +/** + * Copyright (c) 2013-2015, Christopher Jeffrey, Peter Sunde (MIT License) + * Copyright (c) 2016, Daniel Imms (MIT License). + * + * pty.cc: + * This file is responsible for starting processes + * with pseudo-terminal file descriptors. + */ + +#include +#include +#include // PathCombine, PathIsRelative +#include +#include +#include +#include +#include +#include + +#include "path_util.h" + +/** +* Misc +*/ +extern "C" void init(v8::Handle); + +#define WINPTY_DBG_VARIABLE TEXT("WINPTYDBG") + +/** +* winpty +*/ +static std::vector ptyHandles; +static volatile LONG ptyCounter; + +/** +* Helpers +*/ + +static winpty_t *get_pipe_handle(int handle) { + for (size_t i = 0; i < ptyHandles.size(); ++i) { + winpty_t *ptyHandle = ptyHandles[i]; + int current = (int)winpty_agent_process(ptyHandle); + if (current == handle) { + return ptyHandle; + } + } + return nullptr; +} + +static bool remove_pipe_handle(int handle) { + for (size_t i = 0; i < ptyHandles.size(); ++i) { + winpty_t *ptyHandle = ptyHandles[i]; + if ((int)winpty_agent_process(ptyHandle) == handle) { + winpty_free(ptyHandle); + ptyHandles.erase(ptyHandles.begin() + i); + ptyHandle = nullptr; + return true; + } + } + return false; +} + +void throw_winpty_error(const char *generalMsg, winpty_error_ptr_t error_ptr) { + std::stringstream why; + std::wstring msg(winpty_error_msg(error_ptr)); + std::string msg_(msg.begin(), msg.end()); + why << generalMsg << ": " << msg_; + Nan::ThrowError(why.str().c_str()); + winpty_error_free(error_ptr); +} + +static NAN_METHOD(PtyGetExitCode) { + Nan::HandleScope scope; + + if (info.Length() != 1 || + !info[0]->IsNumber()) { + Nan::ThrowError("Usage: pty.getExitCode(pidHandle)"); + return; + } + + DWORD exitCode = 0; + GetExitCodeProcess((HANDLE)info[0]->IntegerValue(), &exitCode); + + info.GetReturnValue().Set(Nan::New(exitCode)); +} + +static NAN_METHOD(PtyGetProcessList) { + Nan::HandleScope scope; + + if (info.Length() != 1 || + !info[0]->IsNumber()) { + Nan::ThrowError("Usage: pty.getProcessList(pid)"); + return; + } + + int pid = info[0]->Int32Value(); + + winpty_t *pc = get_pipe_handle(pid); + if (pc == nullptr) { + info.GetReturnValue().Set(Nan::New(0)); + return; + } + int processList[64]; + const int processCount = 64; + int actualCount = winpty_get_console_process_list(pc, processList, processCount, nullptr); + + v8::Local result = Nan::New(actualCount); + for (uint32_t i = 0; i < actualCount; i++) { + Nan::Set(result, i, Nan::New(processList[i])); + } + info.GetReturnValue().Set(result); +} + +static NAN_METHOD(PtyStartProcess) { + Nan::HandleScope scope; + + if (info.Length() != 7 || + !info[0]->IsString() || + !info[1]->IsString() || + !info[2]->IsArray() || + !info[3]->IsString() || + !info[4]->IsNumber() || + !info[5]->IsNumber() || + !info[6]->IsBoolean()) { + Nan::ThrowError("Usage: pty.startProcess(file, cmdline, env, cwd, cols, rows, debug)"); + return; + } + + std::stringstream why; + + const wchar_t *filename = path_util::to_wstring(v8::String::Utf8Value(info[0]->ToString())); + const wchar_t *cmdline = path_util::to_wstring(v8::String::Utf8Value(info[1]->ToString())); + const wchar_t *cwd = path_util::to_wstring(v8::String::Utf8Value(info[3]->ToString())); + + // create environment block + std::wstring env; + const v8::Handle envValues = v8::Handle::Cast(info[2]); + if (!envValues.IsEmpty()) { + + std::wstringstream envBlock; + + for(uint32_t i = 0; i < envValues->Length(); i++) { + std::wstring envValue(path_util::to_wstring(v8::String::Utf8Value(envValues->Get(i)->ToString()))); + envBlock << envValue << L'\0'; + } + + env = envBlock.str(); + } + + // use environment 'Path' variable to determine location of + // the relative path that we have recieved (e.g cmd.exe) + std::wstring shellpath; + if (::PathIsRelativeW(filename)) { + shellpath = path_util::get_shell_path(filename); + } else { + shellpath = filename; + } + + std::string shellpath_(shellpath.begin(), shellpath.end()); + + if (shellpath.empty() || !path_util::file_exists(shellpath)) { + why << "File not found: " << shellpath_; + Nan::ThrowError(why.str().c_str()); + goto cleanup; + } + + int cols = info[4]->Int32Value(); + int rows = info[5]->Int32Value(); + bool debug = info[6]->ToBoolean()->IsTrue(); + + // Enable/disable debugging + SetEnvironmentVariable(WINPTY_DBG_VARIABLE, debug ? "1" : NULL); // NULL = deletes variable + + // Create winpty config + winpty_error_ptr_t error_ptr = nullptr; + winpty_config_t* winpty_config = winpty_config_new(0, &error_ptr); + if (winpty_config == nullptr) { + throw_winpty_error("Error creating WinPTY config", error_ptr); + goto cleanup; + } + winpty_error_free(error_ptr); + + // Set pty size on config + winpty_config_set_initial_size(winpty_config, cols, rows); + + // Start the pty agent + winpty_t *pc = winpty_open(winpty_config, &error_ptr); + winpty_config_free(winpty_config); + if (pc == nullptr) { + throw_winpty_error("Error launching WinPTY agent", error_ptr); + goto cleanup; + } + winpty_error_free(error_ptr); + + // Save pty struct for later use + ptyHandles.insert(ptyHandles.end(), pc); + + // Create winpty spawn config + winpty_spawn_config_t* config = winpty_spawn_config_new(WINPTY_SPAWN_FLAG_AUTO_SHUTDOWN, shellpath.c_str(), cmdline, cwd, env.c_str(), &error_ptr); + if (config == nullptr) { + throw_winpty_error("Error creating WinPTY spawn config", error_ptr); + goto cleanup; + } + winpty_error_free(error_ptr); + + // Spawn the new process + HANDLE handle = nullptr; + BOOL spawnSuccess = winpty_spawn(pc, config, &handle, nullptr, nullptr, &error_ptr); + winpty_spawn_config_free(config); + if (!spawnSuccess) { + throw_winpty_error("Unable to start terminal process", error_ptr); + goto cleanup; + } + winpty_error_free(error_ptr); + + // Set return values + v8::Local marshal = Nan::New(); + marshal->Set(Nan::New("innerPid").ToLocalChecked(), Nan::New((int)GetProcessId(handle))); + marshal->Set(Nan::New("innerPidHandle").ToLocalChecked(), Nan::New((int)handle)); + marshal->Set(Nan::New("pid").ToLocalChecked(), Nan::New((int)winpty_agent_process(pc))); + marshal->Set(Nan::New("pty").ToLocalChecked(), Nan::New(InterlockedIncrement(&ptyCounter))); + marshal->Set(Nan::New("fd").ToLocalChecked(), Nan::New(-1)); + { + LPCWSTR coninPipeName = winpty_conin_name(pc); + std::wstring coninPipeNameWStr(coninPipeName); + std::string coninPipeNameStr(coninPipeNameWStr.begin(), coninPipeNameWStr.end()); + marshal->Set(Nan::New("conin").ToLocalChecked(), Nan::New(coninPipeNameStr).ToLocalChecked()); + LPCWSTR conoutPipeName = winpty_conout_name(pc); + std::wstring conoutPipeNameWStr(conoutPipeName); + std::string conoutPipeNameStr(conoutPipeNameWStr.begin(), conoutPipeNameWStr.end()); + marshal->Set(Nan::New("conout").ToLocalChecked(), Nan::New(conoutPipeNameStr).ToLocalChecked()); + } + info.GetReturnValue().Set(marshal); + + goto cleanup; + +cleanup: + delete filename; + delete cmdline; + delete cwd; +} + +static NAN_METHOD(PtyResize) { + Nan::HandleScope scope; + + if (info.Length() != 3 || + !info[0]->IsNumber() || + !info[1]->IsNumber() || + !info[2]->IsNumber()) { + Nan::ThrowError("Usage: pty.resize(pid, cols, rows)"); + return; + } + + int handle = info[0]->Int32Value(); + int cols = info[1]->Int32Value(); + int rows = info[2]->Int32Value(); + + winpty_t *pc = get_pipe_handle(handle); + + if (pc == nullptr) { + Nan::ThrowError("The pty doesn't appear to exist"); + return; + } + BOOL success = winpty_set_size(pc, cols, rows, nullptr); + if (!success) { + Nan::ThrowError("The pty could not be resized"); + return; + } + + return info.GetReturnValue().SetUndefined(); +} + +static NAN_METHOD(PtyKill) { + Nan::HandleScope scope; + + if (info.Length() != 2 || + !info[0]->IsNumber() || + !info[1]->IsNumber()) { + Nan::ThrowError("Usage: pty.kill(pid, innerPidHandle)"); + return; + } + + int handle = info[0]->Int32Value(); + HANDLE innerPidHandle = (HANDLE)info[1]->Int32Value(); + + winpty_t *pc = get_pipe_handle(handle); + if (pc == nullptr) { + Nan::ThrowError("Pty seems to have been killed already"); + return; + } + + assert(remove_pipe_handle(handle)); + CloseHandle(innerPidHandle); + + return info.GetReturnValue().SetUndefined(); +} + +/** +* Init +*/ + +extern "C" void init(v8::Handle target) { + Nan::HandleScope scope; + Nan::SetMethod(target, "startProcess", PtyStartProcess); + Nan::SetMethod(target, "resize", PtyResize); + Nan::SetMethod(target, "kill", PtyKill); + Nan::SetMethod(target, "getExitCode", PtyGetExitCode); + Nan::SetMethod(target, "getProcessList", PtyGetProcessList); +}; + +NODE_MODULE(pty, init); From 1abbe94cf6f6df85f09afa13bbcc4c71e611b66d Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 11 Oct 2018 17:14:42 -0700 Subject: [PATCH 10/55] Add useConpty option --- binding.gyp | 9 +-------- src/interfaces.ts | 1 + src/win/conpty.cc | 5 ++--- src/win/path_util.cc | 1 - src/windowsPtyAgent.ts | 22 ++++++++++++++++++++-- src/windowsTerminal.ts | 2 +- typings/node-pty.d.ts | 5 +++++ 7 files changed, 30 insertions(+), 15 deletions(-) diff --git a/binding.gyp b/binding.gyp index 681a26d82..b10989124 100644 --- a/binding.gyp +++ b/binding.gyp @@ -60,14 +60,7 @@ { 'target_name': 'conpty', 'include_dirs' : [ - ' #include #include "path_util.h" -#include "..\..\deps\winpty\src\shared\GenRandom.h" -#include "..\..\deps\winpty\src\shared\StringBuilder.h" extern "C" void init(v8::Handle); @@ -159,6 +157,7 @@ HRESULT CreateNamedPipesAndPseudoConsole(COORD size, static NAN_METHOD(PtyStartProcess) { Nan::HandleScope scope; + v8::Local marshal; std::wstring inName, outName, str_cmdline; BOOL fSuccess = FALSE; std::unique_ptr mutableCommandline; @@ -234,7 +233,7 @@ static NAN_METHOD(PtyStartProcess) { } // Set return values - v8::Local marshal = Nan::New(); + marshal = Nan::New(); // TODO: Pull in innerPid, innerPidHandle(?) // marshal->Set(Nan::New("innerPid").ToLocalChecked(), Nan::New((int)GetProcessId(handle))); // marshal->Set(Nan::New("innerPidHandle").ToLocalChecked(), Nan::New((int)handle)); diff --git a/src/win/path_util.cc b/src/win/path_util.cc index f38755609..684fd19d6 100644 --- a/src/win/path_util.cc +++ b/src/win/path_util.cc @@ -5,7 +5,6 @@ #include #include // PathCombine -#include #include "path_util.h" diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts index be0857e08..6021ca70f 100644 --- a/src/windowsPtyAgent.ts +++ b/src/windowsPtyAgent.ts @@ -3,11 +3,12 @@ * Copyright (c) 2016, Daniel Imms (MIT License). */ +import * as os from 'os'; import * as path from 'path'; import { Socket } from 'net'; import { ArgvOrCommandLine } from './types'; -const pty = require(path.join('..', 'build', 'Debug', 'pty.node')); +let pty: any; /** * Agent. Internal class. @@ -40,8 +41,16 @@ export class WindowsPtyAgent { cwd: string, cols: number, rows: number, - debug: boolean + debug: boolean, + useConpty: boolean | undefined ) { + if (!pty) { + if (useConpty === undefined) { + useConpty = this._getWindowsBuildNumber() >= 17692; + } + console.log('useConpty?', useConpty); + pty = require(path.join('..', 'build', 'Debug', `${useConpty ? 'conpty' : 'winpty'}.node`)); + } // Sanitize input variable. cwd = path.resolve(cwd); @@ -115,6 +124,15 @@ export class WindowsPtyAgent { public getExitCode(): number { return pty.getExitCode(this._innerPidHandle); } + + private _getWindowsBuildNumber(): number { + const osVersion = (/(\d+)\.(\d+)\.(\d+)/g).exec(os.release()); + let buildNumber: number = 0; + if (osVersion && osVersion.length === 4) { + buildNumber = parseInt(osVersion[3]); + } + return buildNumber; + } } // Convert argc/argv into a Win32 command-line following the escaping convention diff --git a/src/windowsTerminal.ts b/src/windowsTerminal.ts index d31bfcccf..2b25ab3be 100644 --- a/src/windowsTerminal.ts +++ b/src/windowsTerminal.ts @@ -47,7 +47,7 @@ export class WindowsTerminal extends Terminal { this._deferreds = []; // Create new termal. - this._agent = new WindowsPtyAgent(file, args, parsedEnv, cwd, cols, rows, false); + this._agent = new WindowsPtyAgent(file, args, parsedEnv, cwd, cols, rows, false, opt.experimentalUseConpty); this._socket = this._agent.outSocket; // Not available until `ready` event emitted. diff --git a/typings/node-pty.d.ts b/typings/node-pty.d.ts index 93048837e..822a07444 100644 --- a/typings/node-pty.d.ts +++ b/typings/node-pty.d.ts @@ -25,6 +25,11 @@ declare module 'node-pty' { uid?: number; gid?: number; encoding?: string; + /** + * Whether to use the experimental ConPTY system on Windows. This setting will be ignored on + * non-Windows. + */ + experimentalUseConpty?: boolean; } /** From 705a26ef73d52e9109adef612c9b00c2ec59d148 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 11 Oct 2018 17:22:55 -0700 Subject: [PATCH 11/55] Set pid in conpty --- src/win/conpty.cc | 7 ++++++- src/windowsPtyAgent.ts | 11 ++++++++--- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/win/conpty.cc b/src/win/conpty.cc index 0e0738901..4720e414c 100644 --- a/src/win/conpty.cc +++ b/src/win/conpty.cc @@ -263,6 +263,8 @@ static NAN_METHOD(PtyStartProcess) { } static NAN_METHOD(PtyConnect) { + Nan::HandleScope scope; + // If we're working with conpty's we need to call ConnectNamedPipe here AFTER // the Socket has attempted to connect to the other end, then actually // spawn the process here. @@ -271,6 +273,7 @@ static NAN_METHOD(PtyConnect) { BOOL fSuccess = FALSE; std::unique_ptr mutableCommandline; PROCESS_INFORMATION _piClient{}; + v8::Local marshal; BOOL success = ConnectNamedPipe(hIn, nullptr); success = ConnectNamedPipe(hOut, nullptr); @@ -311,7 +314,9 @@ static NAN_METHOD(PtyConnect) { ); // TODO: return the information about the client application out to the caller? - + marshal = Nan::New(); + marshal->Set(Nan::New("pid").ToLocalChecked(), Nan::New(_piClient.dwProcessId)); + info.GetReturnValue().Set(marshal); } static NAN_METHOD(PtyResize) { diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts index 6021ca70f..b65716151 100644 --- a/src/windowsPtyAgent.ts +++ b/src/windowsPtyAgent.ts @@ -51,6 +51,7 @@ export class WindowsPtyAgent { console.log('useConpty?', useConpty); pty = require(path.join('..', 'build', 'Debug', `${useConpty ? 'conpty' : 'winpty'}.node`)); } + // Sanitize input variable. cwd = path.resolve(cwd); @@ -61,7 +62,9 @@ export class WindowsPtyAgent { const term = pty.startProcess(file, commandLine, env, cwd, cols, rows, debug); // Terminal pid. - this._pid = term.pid; + if (!useConpty) { + this._pid = term.pid; + } this._innerPid = term.innerPid; this._innerPidHandle = term.innerPidHandle; @@ -89,8 +92,10 @@ export class WindowsPtyAgent { // TODO: Do we *need* to timeout here or wait for the sockets to connect? // or can we do this synchronously like this? - pty.connect(); - + if (useConpty) { + const connect = pty.connect(); + this._pid = connect.pid; + } } public resize(cols: number, rows: number): void { From 25218d62fdf1e17333456af8aa0b388c0edf82a1 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 11 Oct 2018 17:34:10 -0700 Subject: [PATCH 12/55] Set innerPid, not pid --- src/windowsPtyAgent.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts index b65716151..ccc5fabc5 100644 --- a/src/windowsPtyAgent.ts +++ b/src/windowsPtyAgent.ts @@ -94,7 +94,7 @@ export class WindowsPtyAgent { // or can we do this synchronously like this? if (useConpty) { const connect = pty.connect(); - this._pid = connect.pid; + this._innerPid = connect.pid; } } From 38e17cbc422f2bad0e5dfba0cbf2a98619536a05 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Mon, 15 Oct 2018 14:24:51 -0700 Subject: [PATCH 13/55] Generate random pipe names --- src/win/conpty.cc | 25 +++++++++++++++---------- src/windowsPtyAgent.ts | 22 +++++++++++++--------- 2 files changed, 28 insertions(+), 19 deletions(-) diff --git a/src/win/conpty.cc b/src/win/conpty.cc index 4720e414c..b8a59e294 100644 --- a/src/win/conpty.cc +++ b/src/win/conpty.cc @@ -83,12 +83,12 @@ HANDLE hIn, hOut; HPCON hpc; // Returns a new server named pipe. It has not yet been connected. -bool createDataServerPipe(bool write, std::wstring kind, HANDLE* hServer, std::wstring &name) +bool createDataServerPipe(bool write, std::wstring kind, HANDLE* hServer, std::wstring &name, std::wstring &pipeName) { *hServer = INVALID_HANDLE_VALUE; // TODO generate unique names for each pipe - name = L"\\\\.\\pipe\\conpty2-" + kind; + name = L"\\\\.\\pipe\\" + pipeName + L"-" + kind; const DWORD winOpenMode = PIPE_ACCESS_INBOUND | PIPE_ACCESS_OUTBOUND | FILE_FLAG_FIRST_PIPE_INSTANCE/* | FILE_FLAG_OVERLAPPED */; @@ -114,7 +114,8 @@ HRESULT CreateNamedPipesAndPseudoConsole(COORD size, HANDLE *phOutput, HPCON* phPC, std::wstring& inName, - std::wstring& outName) + std::wstring& outName, + std::wstring& pipeName) { HANDLE hLibrary = LoadLibraryExW(L"kernel32.dll", 0, 0); bool fLoadedDll = hLibrary != nullptr; @@ -128,12 +129,12 @@ HRESULT CreateNamedPipesAndPseudoConsole(COORD size, return E_INVALIDARG; } - bool success = createDataServerPipe(true, L"in", phInput, inName); + bool success = createDataServerPipe(true, L"in", phInput, inName, pipeName); if (!success) { return HRESULT_FROM_WIN32(GetLastError()); } - success = createDataServerPipe(false, L"out", phOutput, outName); + success = createDataServerPipe(false, L"out", phOutput, outName, pipeName); if (!success) { return HRESULT_FROM_WIN32(GetLastError()); @@ -158,22 +159,23 @@ static NAN_METHOD(PtyStartProcess) { Nan::HandleScope scope; v8::Local marshal; - std::wstring inName, outName, str_cmdline; + std::wstring inName, outName, str_cmdline, str_pipeName; BOOL fSuccess = FALSE; std::unique_ptr mutableCommandline; PROCESS_INFORMATION _piClient{}; DWORD dwExit = 0; - if (info.Length() != 7 || + if (info.Length() != 8 || !info[0]->IsString() || !info[1]->IsString() || !info[2]->IsArray() || !info[3]->IsString() || !info[4]->IsNumber() || !info[5]->IsNumber() || - !info[6]->IsBoolean()) { - Nan::ThrowError("Usage: pty.startProcess(file, cmdline, env, cwd, cols, rows, debug)"); + !info[6]->IsBoolean() || + !info[7]->IsString()) { + Nan::ThrowError("Usage: pty.startProcess(file, cmdline, env, cwd, cols, rows, debug, pipeName)"); return; } @@ -182,6 +184,7 @@ static NAN_METHOD(PtyStartProcess) { const wchar_t *filename = path_util::to_wstring(v8::String::Utf8Value(info[0]->ToString())); const wchar_t *cmdline = path_util::to_wstring(v8::String::Utf8Value(info[1]->ToString())); const wchar_t *cwd = path_util::to_wstring(v8::String::Utf8Value(info[3]->ToString())); + const wchar_t *pipeName = path_util::to_wstring(v8::String::Utf8Value(info[7]->ToString())); // create environment block std::wstring env; @@ -219,7 +222,9 @@ static NAN_METHOD(PtyStartProcess) { int rows = info[5]->Int32Value(); bool debug = info[6]->ToBoolean()->IsTrue(); - HRESULT hr = CreateNamedPipesAndPseudoConsole({(SHORT)cols, (SHORT)rows}, 0, &hIn, &hOut, &hpc, inName, outName); + str_pipeName = pipeName; + + HRESULT hr = CreateNamedPipesAndPseudoConsole({(SHORT)cols, (SHORT)rows}, 0, &hIn, &hOut, &hpc, inName, outName, str_pipeName); if (SUCCEEDED(hr)) { diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts index ccc5fabc5..b03bbc370 100644 --- a/src/windowsPtyAgent.ts +++ b/src/windowsPtyAgent.ts @@ -59,7 +59,7 @@ export class WindowsPtyAgent { const commandLine = argsToCommandLine(file, args); // Open pty session. - const term = pty.startProcess(file, commandLine, env, cwd, cols, rows, debug); + const term = pty.startProcess(file, commandLine, env, cwd, cols, rows, debug, this._generatePipeName()); // Terminal pid. if (!useConpty) { @@ -130,14 +130,18 @@ export class WindowsPtyAgent { return pty.getExitCode(this._innerPidHandle); } - private _getWindowsBuildNumber(): number { - const osVersion = (/(\d+)\.(\d+)\.(\d+)/g).exec(os.release()); - let buildNumber: number = 0; - if (osVersion && osVersion.length === 4) { - buildNumber = parseInt(osVersion[3]); - } - return buildNumber; - } + private _getWindowsBuildNumber(): number { + const osVersion = (/(\d+)\.(\d+)\.(\d+)/g).exec(os.release()); + let buildNumber: number = 0; + if (osVersion && osVersion.length === 4) { + buildNumber = parseInt(osVersion[3]); + } + return buildNumber; + } + + private _generatePipeName(): string { + return `conpty-${Math.random() * 10000000}`; + } } // Convert argc/argv into a Win32 command-line following the escaping convention From fda8529574c8e9a8e2d493e5be42f19496437272 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Mon, 15 Oct 2018 14:46:47 -0700 Subject: [PATCH 14/55] Fix type narrowing warning --- src/win/conpty.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/win/conpty.cc b/src/win/conpty.cc index b8a59e294..13d3e5e28 100644 --- a/src/win/conpty.cc +++ b/src/win/conpty.cc @@ -336,8 +336,8 @@ static NAN_METHOD(PtyResize) { } int handle = info[0]->Int32Value(); - int cols = info[1]->Int32Value(); - int rows = info[2]->Int32Value(); + SHORT cols = info[1]->Uint32Value(); + SHORT rows = info[2]->Uint32Value(); // TODO: Share hLibrary between functions From e7f9fc3cab8bb4684bf74478613cb6f572756422 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Mon, 15 Oct 2018 14:55:59 -0700 Subject: [PATCH 15/55] Shuffle args around --- src/win/conpty.cc | 64 ++++++++++++++++++++++-------------------- src/windowsPtyAgent.ts | 13 +++++---- 2 files changed, 41 insertions(+), 36 deletions(-) diff --git a/src/win/conpty.cc b/src/win/conpty.cc index 13d3e5e28..f0d9e8171 100644 --- a/src/win/conpty.cc +++ b/src/win/conpty.cc @@ -163,43 +163,29 @@ static NAN_METHOD(PtyStartProcess) { BOOL fSuccess = FALSE; std::unique_ptr mutableCommandline; PROCESS_INFORMATION _piClient{}; + std::stringstream why; DWORD dwExit = 0; - if (info.Length() != 8 || + if (info.Length() != 7 || !info[0]->IsString() || !info[1]->IsString() || - !info[2]->IsArray() || - !info[3]->IsString() || + !info[2]->IsString() || + !info[3]->IsNumber() || !info[4]->IsNumber() || - !info[5]->IsNumber() || - !info[6]->IsBoolean() || - !info[7]->IsString()) { - Nan::ThrowError("Usage: pty.startProcess(file, cmdline, env, cwd, cols, rows, debug, pipeName)"); + !info[5]->IsBoolean() || + !info[6]->IsString()) { + Nan::ThrowError("Usage: pty.startProcess(file, cmdline, cwd, cols, rows, debug, pipeName)"); return; } - std::stringstream why; - const wchar_t *filename = path_util::to_wstring(v8::String::Utf8Value(info[0]->ToString())); const wchar_t *cmdline = path_util::to_wstring(v8::String::Utf8Value(info[1]->ToString())); - const wchar_t *cwd = path_util::to_wstring(v8::String::Utf8Value(info[3]->ToString())); - const wchar_t *pipeName = path_util::to_wstring(v8::String::Utf8Value(info[7]->ToString())); - - // create environment block - std::wstring env; - const v8::Handle envValues = v8::Handle::Cast(info[2]); - if (!envValues.IsEmpty()) { - - std::wstringstream envBlock; - - for(uint32_t i = 0; i < envValues->Length(); i++) { - std::wstring envValue(path_util::to_wstring(v8::String::Utf8Value(envValues->Get(i)->ToString()))); - envBlock << envValue << L'\0'; - } - - env = envBlock.str(); - } + const wchar_t *cwd = path_util::to_wstring(v8::String::Utf8Value(info[2]->ToString())); + const SHORT cols = info[3]->Uint32Value(); + const SHORT rows = info[4]->Uint32Value(); + const bool debug = info[5]->ToBoolean()->IsTrue(); + const wchar_t *pipeName = path_util::to_wstring(v8::String::Utf8Value(info[6]->ToString())); // use environment 'Path' variable to determine location of // the relative path that we have recieved (e.g cmd.exe) @@ -218,13 +204,9 @@ static NAN_METHOD(PtyStartProcess) { goto cleanup; } - int cols = info[4]->Int32Value(); - int rows = info[5]->Int32Value(); - bool debug = info[6]->ToBoolean()->IsTrue(); - str_pipeName = pipeName; - HRESULT hr = CreateNamedPipesAndPseudoConsole({(SHORT)cols, (SHORT)rows}, 0, &hIn, &hOut, &hpc, inName, outName, str_pipeName); + HRESULT hr = CreateNamedPipesAndPseudoConsole({cols, rows}, 0, &hIn, &hOut, &hpc, inName, outName, str_pipeName); if (SUCCEEDED(hr)) { @@ -279,6 +261,26 @@ static NAN_METHOD(PtyConnect) { std::unique_ptr mutableCommandline; PROCESS_INFORMATION _piClient{}; v8::Local marshal; + + if (info.Length() != 1 || + !info[0]->IsArray()) { + Nan::ThrowError("Usage: pty.connect(env)"); + return; + } + + // Create environment block + std::wstring env; + const v8::Handle envValues = v8::Handle::Cast(info[2]); + if (!envValues.IsEmpty()) { + std::wstringstream envBlock; + for(uint32_t i = 0; i < envValues->Length(); i++) { + std::wstring envValue(path_util::to_wstring(v8::String::Utf8Value(envValues->Get(i)->ToString()))); + envBlock << envValue << L'\0'; + } + env = envBlock.str(); + } + // TODO: Use env + BOOL success = ConnectNamedPipe(hIn, nullptr); success = ConnectNamedPipe(hOut, nullptr); diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts index b03bbc370..03edb2d77 100644 --- a/src/windowsPtyAgent.ts +++ b/src/windowsPtyAgent.ts @@ -59,12 +59,15 @@ export class WindowsPtyAgent { const commandLine = argsToCommandLine(file, args); // Open pty session. - const term = pty.startProcess(file, commandLine, env, cwd, cols, rows, debug, this._generatePipeName()); - - // Terminal pid. - if (!useConpty) { + let term; + if (useConpty) { + term = pty.startProcess(file, commandLine, cwd, cols, rows, debug, this._generatePipeName()); + } else { + term = pty.startProcess(file, commandLine, env, cwd, cols, rows, debug); this._pid = term.pid; } + + // Terminal pid. this._innerPid = term.innerPid; this._innerPidHandle = term.innerPidHandle; @@ -93,7 +96,7 @@ export class WindowsPtyAgent { // TODO: Do we *need* to timeout here or wait for the sockets to connect? // or can we do this synchronously like this? if (useConpty) { - const connect = pty.connect(); + const connect = pty.connect(env); this._innerPid = connect.pid; } } From 942ac3ffbb30057cf70c81e7fadc3c1e1a0aaf81 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Mon, 15 Oct 2018 15:10:48 -0700 Subject: [PATCH 16/55] Pull in environment to conpty --- src/win/conpty.cc | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/win/conpty.cc b/src/win/conpty.cc index f0d9e8171..48751f37a 100644 --- a/src/win/conpty.cc +++ b/src/win/conpty.cc @@ -279,7 +279,8 @@ static NAN_METHOD(PtyConnect) { } env = envBlock.str(); } - // TODO: Use env + auto envV = std::vector(env.begin(), env.end()); + LPWSTR envArg = envV.empty() ? nullptr : envV.data(); BOOL success = ConnectNamedPipe(hIn, nullptr); success = ConnectNamedPipe(hOut, nullptr); @@ -303,7 +304,8 @@ static NAN_METHOD(PtyConnect) { NULL); // You need to pass a MUTABLE commandline to CreateProcess, so convert our const wchar_t* here. - str_cmdline = L"powershell.exe"; + // TODO: Pull in cmdline + str_cmdline = L"cmd.exe"; mutableCommandline = std::make_unique(str_cmdline.length() + 1); HRESULT hr = StringCchCopyW(mutableCommandline.get(), str_cmdline.length() + 1, str_cmdline.c_str()); @@ -314,7 +316,7 @@ static NAN_METHOD(PtyConnect) { nullptr, // lpThreadAttributes false, // bInheritHandles VERY IMPORTANT that this is false EXTENDED_STARTUPINFO_PRESENT, // dwCreationFlags - nullptr, // lpEnvironment + envArg, // lpEnvironment nullptr, // lpCurrentDirectory &siEx.StartupInfo, // lpStartupInfo &_piClient // lpProcessInformation From df76478fbb7bc5fc706f3e1e45c3cd029936fb5b Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Mon, 15 Oct 2018 15:14:25 -0700 Subject: [PATCH 17/55] Add env to example --- examples/fork/index.js | 4 ++-- src/win/conpty.cc | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/examples/fork/index.js b/examples/fork/index.js index 9ed0e2c81..1fa2aaca9 100644 --- a/examples/fork/index.js +++ b/examples/fork/index.js @@ -8,7 +8,7 @@ var ptyProcess = pty.spawn(shell, [], { cols: 80, rows: 19, cwd: process.env.HOME, - env: process.env + env: Object.assign(process.env, { TEST: "abc" }) }); ptyProcess.on('data', function(data) { @@ -21,7 +21,7 @@ ptyProcess.write('dir\r'); setTimeout(() => { ptyProcess.resize(30, 19); - ptyProcess.write('dir\r'); + ptyProcess.write('echo %TEST%\r'); }, 2000); process.on('exit', () => { diff --git a/src/win/conpty.cc b/src/win/conpty.cc index 48751f37a..903d12cab 100644 --- a/src/win/conpty.cc +++ b/src/win/conpty.cc @@ -316,7 +316,7 @@ static NAN_METHOD(PtyConnect) { nullptr, // lpThreadAttributes false, // bInheritHandles VERY IMPORTANT that this is false EXTENDED_STARTUPINFO_PRESENT, // dwCreationFlags - envArg, // lpEnvironment + envArg, // lpEnvironment nullptr, // lpCurrentDirectory &siEx.StartupInfo, // lpStartupInfo &_piClient // lpProcessInformation From 4079cfe0f5e869c2a1ba224274d2565e4d37c271 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Mon, 15 Oct 2018 15:18:59 -0700 Subject: [PATCH 18/55] Add env test --- src/windowsTerminal.test.ts | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/windowsTerminal.test.ts b/src/windowsTerminal.test.ts index 3d234e47e..996e2899f 100644 --- a/src/windowsTerminal.test.ts +++ b/src/windowsTerminal.test.ts @@ -2,7 +2,7 @@ * Copyright (c) 2017, Daniel Imms (MIT License). */ - import * as fs from 'fs'; +import * as fs from 'fs'; import * as assert from 'assert'; import { WindowsTerminal } from './windowsTerminal'; @@ -53,6 +53,20 @@ if (process.platform === 'win32') { }); }); + describe('env', () => { + it('should set environment variables of the shell', (done) => { + const term = new WindowsTerminal('cmd.exe', '/C echo %FOO%', { env: { FOO: 'BAR' }}); + let result = ''; + term.on('data', (data) => { + result += data; + }); + term.on('exit', () => { + assert.ok(result.indexOf('BAR') >= 0); + done(); + }); + }); + }); + describe('On close', () => { it('should return process zero exit codes', (done) => { const term = new WindowsTerminal('cmd.exe', '/C exit'); From 290f92aeed5cd4d7abdd9b000b7ec9cf48f73561 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 17 Oct 2018 13:33:22 -0700 Subject: [PATCH 19/55] Fix environment --- src/win/conpty.cc | 57 ++++++++++++++++++++++++++++++++++-------- src/windowsPtyAgent.ts | 4 ++- src/windowsTerminal.ts | 1 + 3 files changed, 51 insertions(+), 11 deletions(-) diff --git a/src/win/conpty.cc b/src/win/conpty.cc index 903d12cab..e2425d0ca 100644 --- a/src/win/conpty.cc +++ b/src/win/conpty.cc @@ -34,8 +34,27 @@ typedef void (*PFNCLOSEPSEUDOCONSOLE)(HPCON hpc); #endif +// TODO: Pull pty handle stuff into its own class +// struct pty_handle { +// int id; +// HANDLE hIn; +// HANDLE hOut; +// pty_handle(int _id, HANDLE _hIn, HANDLE _hOut) : id(_id), hIn(_hIn), hOut(_hOut) {}; +// }; + +// static std::vector ptyHandles; static volatile LONG ptyCounter; +// static pty_handle* get_pty_handle(int id) { +// for (size_t i = 0; i < ptyHandles.size(); ++i) { +// pty_handle* ptyHandle = ptyHandles[i]; +// if (ptyHandle->id == id) { +// return ptyHandle; +// } +// } +// return nullptr; +// } + static NAN_METHOD(PtyGetExitCode) { Nan::HandleScope scope; @@ -208,10 +227,16 @@ static NAN_METHOD(PtyStartProcess) { HRESULT hr = CreateNamedPipesAndPseudoConsole({cols, rows}, 0, &hIn, &hOut, &hpc, inName, outName, str_pipeName); + // Set return values + marshal = Nan::New(); + if (SUCCEEDED(hr)) { // We were able to instantiate a conpty, yay! - // TODO + const int ptyId = InterlockedIncrement(&ptyCounter); + // TODO: Name this pty "id" + marshal->Set(Nan::New("pty").ToLocalChecked(), Nan::New(ptyId)); + // ptyHandles.insert(ptyHandles.end(), new pty_handle(ptyId, hIn, hOut)); } else { @@ -219,14 +244,11 @@ static NAN_METHOD(PtyStartProcess) { // TODO } - // Set return values - marshal = Nan::New(); // TODO: Pull in innerPid, innerPidHandle(?) // marshal->Set(Nan::New("innerPid").ToLocalChecked(), Nan::New((int)GetProcessId(handle))); // marshal->Set(Nan::New("innerPidHandle").ToLocalChecked(), Nan::New((int)handle)); // TODO: Pull in pid // marshal->Set(Nan::New("pid").ToLocalChecked(), Nan::New((int)winpty_agent_process(pc))); - marshal->Set(Nan::New("pty").ToLocalChecked(), Nan::New(InterlockedIncrement(&ptyCounter))); marshal->Set(Nan::New("fd").ToLocalChecked(), Nan::New(-1)); { // LPCWSTR coninPipeName = winpty_conin_name(pc); @@ -249,6 +271,12 @@ static NAN_METHOD(PtyStartProcess) { delete cwd; } +template +std::vector vectorFromString(const std::basic_string &str) { + return std::vector(str.begin(), str.end()); +} + + static NAN_METHOD(PtyConnect) { Nan::HandleScope scope; @@ -262,24 +290,27 @@ static NAN_METHOD(PtyConnect) { PROCESS_INFORMATION _piClient{}; v8::Local marshal; - if (info.Length() != 1 || - !info[0]->IsArray()) { - Nan::ThrowError("Usage: pty.connect(env)"); + if (info.Length() != 2 || + !info[0]->IsNumber() || + !info[1]->IsArray()) { + Nan::ThrowError("Usage: pty.connect(id, env)"); return; } + const v8::Handle envValues = v8::Handle::Cast(info[1]); + // Create environment block std::wstring env; - const v8::Handle envValues = v8::Handle::Cast(info[2]); if (!envValues.IsEmpty()) { std::wstringstream envBlock; for(uint32_t i = 0; i < envValues->Length(); i++) { std::wstring envValue(path_util::to_wstring(v8::String::Utf8Value(envValues->Get(i)->ToString()))); envBlock << envValue << L'\0'; } + envBlock << L'\0'; env = envBlock.str(); } - auto envV = std::vector(env.begin(), env.end()); + auto envV = vectorFromString(env); LPWSTR envArg = envV.empty() ? nullptr : envV.data(); BOOL success = ConnectNamedPipe(hIn, nullptr); @@ -315,7 +346,7 @@ static NAN_METHOD(PtyConnect) { nullptr, // lpProcessAttributes nullptr, // lpThreadAttributes false, // bInheritHandles VERY IMPORTANT that this is false - EXTENDED_STARTUPINFO_PRESENT, // dwCreationFlags + EXTENDED_STARTUPINFO_PRESENT | CREATE_UNICODE_ENVIRONMENT, // dwCreationFlags envArg, // lpEnvironment nullptr, // lpCurrentDirectory &siEx.StartupInfo, // lpStartupInfo @@ -323,7 +354,13 @@ static NAN_METHOD(PtyConnect) { ); // TODO: return the information about the client application out to the caller? + DWORD error = GetLastError(); marshal = Nan::New(); + + if (!fSuccess) { + marshal->Set(Nan::New("error").ToLocalChecked(), Nan::New(error)); + } + marshal->Set(Nan::New("pid").ToLocalChecked(), Nan::New(_piClient.dwProcessId)); info.GetReturnValue().Set(marshal); } diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts index 03edb2d77..743696c12 100644 --- a/src/windowsPtyAgent.ts +++ b/src/windowsPtyAgent.ts @@ -96,7 +96,9 @@ export class WindowsPtyAgent { // TODO: Do we *need* to timeout here or wait for the sockets to connect? // or can we do this synchronously like this? if (useConpty) { - const connect = pty.connect(env); + console.log('this._pty = ' + this._pty); + const connect = pty.connect(this._pty, env); + console.log('connect.error' + connect.error); this._innerPid = connect.pid; } } diff --git a/src/windowsTerminal.ts b/src/windowsTerminal.ts index 2b25ab3be..08e9e7c62 100644 --- a/src/windowsTerminal.ts +++ b/src/windowsTerminal.ts @@ -40,6 +40,7 @@ export class WindowsTerminal extends Terminal { const name = opt.name || env.TERM || DEFAULT_NAME; const parsedEnv = this._parseEnv(env); + console.log('parsedEnv', parsedEnv); // If the terminal is ready this._isReady = false; From 61ed2cc431b77a8cf6bc483e972a4b03f6c17387 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 17 Oct 2018 13:38:10 -0700 Subject: [PATCH 20/55] Store handles in a vector --- src/win/conpty.cc | 46 +++++++++++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/src/win/conpty.cc b/src/win/conpty.cc index e2425d0ca..22ae78bc7 100644 --- a/src/win/conpty.cc +++ b/src/win/conpty.cc @@ -35,25 +35,25 @@ typedef void (*PFNCLOSEPSEUDOCONSOLE)(HPCON hpc); #endif // TODO: Pull pty handle stuff into its own class -// struct pty_handle { -// int id; -// HANDLE hIn; -// HANDLE hOut; -// pty_handle(int _id, HANDLE _hIn, HANDLE _hOut) : id(_id), hIn(_hIn), hOut(_hOut) {}; -// }; - -// static std::vector ptyHandles; +struct pty_handle { + int id; + HANDLE hIn; + HANDLE hOut; + pty_handle(int _id, HANDLE _hIn, HANDLE _hOut) : id(_id), hIn(_hIn), hOut(_hOut) {}; +}; + +static std::vector ptyHandles; static volatile LONG ptyCounter; -// static pty_handle* get_pty_handle(int id) { -// for (size_t i = 0; i < ptyHandles.size(); ++i) { -// pty_handle* ptyHandle = ptyHandles[i]; -// if (ptyHandle->id == id) { -// return ptyHandle; -// } -// } -// return nullptr; -// } +static pty_handle* get_pty_handle(int id) { + for (size_t i = 0; i < ptyHandles.size(); ++i) { + pty_handle* ptyHandle = ptyHandles[i]; + if (ptyHandle->id == id) { + return ptyHandle; + } + } + return nullptr; +} static NAN_METHOD(PtyGetExitCode) { Nan::HandleScope scope; @@ -98,7 +98,7 @@ static NAN_METHOD(PtyGetProcessList) { } // TODO these should probably not be globals -HANDLE hIn, hOut; +// HANDLE hIn, hOut; HPCON hpc; // Returns a new server named pipe. It has not yet been connected. @@ -225,6 +225,7 @@ static NAN_METHOD(PtyStartProcess) { str_pipeName = pipeName; + HANDLE hIn, hOut; HRESULT hr = CreateNamedPipesAndPseudoConsole({cols, rows}, 0, &hIn, &hOut, &hpc, inName, outName, str_pipeName); // Set return values @@ -236,7 +237,7 @@ static NAN_METHOD(PtyStartProcess) { const int ptyId = InterlockedIncrement(&ptyCounter); // TODO: Name this pty "id" marshal->Set(Nan::New("pty").ToLocalChecked(), Nan::New(ptyId)); - // ptyHandles.insert(ptyHandles.end(), new pty_handle(ptyId, hIn, hOut)); + ptyHandles.insert(ptyHandles.end(), new pty_handle(ptyId, hIn, hOut)); } else { @@ -297,6 +298,7 @@ static NAN_METHOD(PtyConnect) { return; } + const int id = info[0]->Int32Value(); const v8::Handle envValues = v8::Handle::Cast(info[1]); // Create environment block @@ -313,8 +315,10 @@ static NAN_METHOD(PtyConnect) { auto envV = vectorFromString(env); LPWSTR envArg = envV.empty() ? nullptr : envV.data(); - BOOL success = ConnectNamedPipe(hIn, nullptr); - success = ConnectNamedPipe(hOut, nullptr); + const pty_handle* handle = get_pty_handle(id); + + BOOL success = ConnectNamedPipe(handle->hIn, nullptr); + success = ConnectNamedPipe(handle->hOut, nullptr); // Attach the pseudoconsole to the client application we're creating STARTUPINFOEXW siEx; From 0c9f57403ba122bc6808c0d2fd4cdfb286f24881 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 17 Oct 2018 13:51:27 -0700 Subject: [PATCH 21/55] Use vector of handles --- src/win/conpty.cc | 85 +++++++++++++++++++++++------------------- src/windowsPtyAgent.ts | 51 +++++++++++++------------ 2 files changed, 74 insertions(+), 62 deletions(-) diff --git a/src/win/conpty.cc b/src/win/conpty.cc index 22ae78bc7..5a390b762 100644 --- a/src/win/conpty.cc +++ b/src/win/conpty.cc @@ -39,7 +39,8 @@ struct pty_handle { int id; HANDLE hIn; HANDLE hOut; - pty_handle(int _id, HANDLE _hIn, HANDLE _hOut) : id(_id), hIn(_hIn), hOut(_hOut) {}; + HPCON hpc; + pty_handle(int _id, HANDLE _hIn, HANDLE _hOut, HPCON _hpc) : id(_id), hIn(_hIn), hOut(_hOut), hpc(_hpc) {}; }; static std::vector ptyHandles; @@ -70,36 +71,32 @@ static NAN_METHOD(PtyGetExitCode) { info.GetReturnValue().Set(Nan::New(exitCode)); } -static NAN_METHOD(PtyGetProcessList) { - Nan::HandleScope scope; - - if (info.Length() != 1 || - !info[0]->IsNumber()) { - Nan::ThrowError("Usage: pty.getProcessList(pid)"); - return; - } - - int pid = info[0]->Int32Value(); - - // winpty_t *pc = get_pipe_handle(pid); - // if (pc == nullptr) { - info.GetReturnValue().Set(Nan::New(0)); - return; - // } - // int processList[64]; - // const int processCount = 64; - // int actualCount = winpty_get_console_process_list(pc, processList, processCount, nullptr); - - // v8::Local result = Nan::New(actualCount); - // for (uint32_t i = 0; i < actualCount; i++) { - // Nan::Set(result, i, Nan::New(processList[i])); - // } - // info.GetReturnValue().Set(result); -} - -// TODO these should probably not be globals -// HANDLE hIn, hOut; -HPCON hpc; +// static NAN_METHOD(PtyGetProcessList) { +// Nan::HandleScope scope; + +// if (info.Length() != 1 || +// !info[0]->IsNumber()) { +// Nan::ThrowError("Usage: pty.getProcessList(pid)"); +// return; +// } + +// int pid = info[0]->Int32Value(); + +// // winpty_t *pc = get_pipe_handle(pid); +// // if (pc == nullptr) { +// info.GetReturnValue().Set(Nan::New(0)); +// return; +// // } +// // int processList[64]; +// // const int processCount = 64; +// // int actualCount = winpty_get_console_process_list(pc, processList, processCount, nullptr); + +// // v8::Local result = Nan::New(actualCount); +// // for (uint32_t i = 0; i < actualCount; i++) { +// // Nan::Set(result, i, Nan::New(processList[i])); +// // } +// // info.GetReturnValue().Set(result); +// } // Returns a new server named pipe. It has not yet been connected. bool createDataServerPipe(bool write, std::wstring kind, HANDLE* hServer, std::wstring &name, std::wstring &pipeName) @@ -226,6 +223,7 @@ static NAN_METHOD(PtyStartProcess) { str_pipeName = pipeName; HANDLE hIn, hOut; + HPCON hpc; HRESULT hr = CreateNamedPipesAndPseudoConsole({cols, rows}, 0, &hIn, &hOut, &hpc, inName, outName, str_pipeName); // Set return values @@ -237,7 +235,7 @@ static NAN_METHOD(PtyStartProcess) { const int ptyId = InterlockedIncrement(&ptyCounter); // TODO: Name this pty "id" marshal->Set(Nan::New("pty").ToLocalChecked(), Nan::New(ptyId)); - ptyHandles.insert(ptyHandles.end(), new pty_handle(ptyId, hIn, hOut)); + ptyHandles.insert(ptyHandles.end(), new pty_handle(ptyId, hIn, hOut, hpc)); } else { @@ -333,7 +331,7 @@ static NAN_METHOD(PtyConnect) { fSuccess = UpdateProcThreadAttribute(siEx.lpAttributeList, 0, PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE, - hpc, + handle->hpc, sizeof(HPCON), NULL, NULL); @@ -376,14 +374,15 @@ static NAN_METHOD(PtyResize) { !info[0]->IsNumber() || !info[1]->IsNumber() || !info[2]->IsNumber()) { - Nan::ThrowError("Usage: pty.resize(pid, cols, rows)"); + Nan::ThrowError("Usage: pty.resize(id, cols, rows)"); return; } - int handle = info[0]->Int32Value(); + int id = info[0]->Int32Value(); SHORT cols = info[1]->Uint32Value(); SHORT rows = info[2]->Uint32Value(); + const pty_handle* handle = get_pty_handle(id); // TODO: Share hLibrary between functions HANDLE hLibrary = LoadLibraryExW(L"kernel32.dll", 0, 0); @@ -394,7 +393,7 @@ static NAN_METHOD(PtyResize) { if (pfnResizePseudoConsole) { COORD size = {cols, rows}; - pfnResizePseudoConsole(hpc, size); + pfnResizePseudoConsole(handle->hpc, size); } } @@ -404,9 +403,19 @@ static NAN_METHOD(PtyResize) { static NAN_METHOD(PtyKill) { Nan::HandleScope scope; + if (info.Length() != 1 || + !info[0]->IsNumber()) { + Nan::ThrowError("Usage: pty.kill(id)"); + return; + } + // TODO: If the pty is backed by conpty, call ClosePseudoConsole // (using LoadLibrary/GetProcAddress to find ClosePseudoConsole if it exists) + int id = info[0]->Int32Value(); + + const pty_handle* handle = get_pty_handle(id); + // TODO: Share hLibrary between functions HANDLE hLibrary = LoadLibraryExW(L"kernel32.dll", 0, 0); bool fLoadedDll = hLibrary != nullptr; @@ -415,7 +424,7 @@ static NAN_METHOD(PtyKill) { PFNCLOSEPSEUDOCONSOLE const pfnClosePseudoConsole = (PFNCLOSEPSEUDOCONSOLE)GetProcAddress((HMODULE)hLibrary, "CreatePseudoConsole"); if (pfnClosePseudoConsole) { - pfnClosePseudoConsole(hpc); + pfnClosePseudoConsole(handle->hpc); } } @@ -433,7 +442,7 @@ extern "C" void init(v8::Handle target) { Nan::SetMethod(target, "resize", PtyResize); Nan::SetMethod(target, "kill", PtyKill); Nan::SetMethod(target, "getExitCode", PtyGetExitCode); - Nan::SetMethod(target, "getProcessList", PtyGetProcessList); + // Nan::SetMethod(target, "getProcessList", PtyGetProcessList); }; NODE_MODULE(pty, init); diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts index 743696c12..1d96e5fab 100644 --- a/src/windowsPtyAgent.ts +++ b/src/windowsPtyAgent.ts @@ -42,14 +42,14 @@ export class WindowsPtyAgent { cols: number, rows: number, debug: boolean, - useConpty: boolean | undefined + private _useConpty: boolean | undefined ) { if (!pty) { - if (useConpty === undefined) { - useConpty = this._getWindowsBuildNumber() >= 17692; + if (this._useConpty === undefined) { + this._useConpty = this._getWindowsBuildNumber() >= 17692; } - console.log('useConpty?', useConpty); - pty = require(path.join('..', 'build', 'Debug', `${useConpty ? 'conpty' : 'winpty'}.node`)); + console.log('useConpty?', this._useConpty); + pty = require(path.join('..', 'build', 'Debug', `${this._useConpty ? 'conpty' : 'winpty'}.node`)); } // Sanitize input variable. @@ -60,7 +60,7 @@ export class WindowsPtyAgent { // Open pty session. let term; - if (useConpty) { + if (this._useConpty) { term = pty.startProcess(file, commandLine, cwd, cols, rows, debug, this._generatePipeName()); } else { term = pty.startProcess(file, commandLine, env, cwd, cols, rows, debug); @@ -95,7 +95,7 @@ export class WindowsPtyAgent { // TODO: Do we *need* to timeout here or wait for the sockets to connect? // or can we do this synchronously like this? - if (useConpty) { + if (this._useConpty) { console.log('this._pty = ' + this._pty); const connect = pty.connect(this._pty, env); console.log('connect.error' + connect.error); @@ -104,8 +104,8 @@ export class WindowsPtyAgent { } public resize(cols: number, rows: number): void { - // TODO: Don't pass 0 in after pid is getting filled correctly - pty.resize(this._pid || 0, cols, rows); + // TODO: Guard against invalid pty values + pty.resize(this._useConpty ? this._pty : this._pid, cols, rows); } public kill(): void { @@ -113,22 +113,25 @@ export class WindowsPtyAgent { this._inSocket.writable = false; this._outSocket.readable = false; this._outSocket.writable = false; - // TODO: Don't pass in 0! - const processList: number[] = pty.getProcessList(this._pid || 0); // Tell the agent to kill the pty, this releases handles to the process - pty.kill(this._pid, this._innerPidHandle); - // Since pty.kill will kill most processes by itself and process IDs can be - // reused as soon as all handles to them are dropped, we want to immediately - // kill the entire console process list. If we do not force kill all - // processes here, node servers in particular seem to become detached and - // remain running (see Microsoft/vscode#26807). - processList.forEach(pid => { - try { - process.kill(pid); - } catch (e) { - // Ignore if process cannot be found (kill ESRCH error) - } - }); + if (this._useConpty) { + pty.kill(this._pty); + } else { + const processList: number[] = pty.getProcessList(this._pid); + pty.kill(this._pid, this._innerPidHandle); + // Since pty.kill will kill most processes by itself and process IDs can be + // reused as soon as all handles to them are dropped, we want to immediately + // kill the entire console process list. If we do not force kill all + // processes here, node servers in particular seem to become detached and + // remain running (see Microsoft/vscode#26807). + processList.forEach(pid => { + try { + process.kill(pid); + } catch (e) { + // Ignore if process cannot be found (kill ESRCH error) + } + }); + } } public getExitCode(): number { From 1be58b702c629430f831abd3b6384c2f56edf9cb Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 17 Oct 2018 14:40:18 -0700 Subject: [PATCH 22/55] Support cmdline, improve error handling --- src/win/conpty.cc | 72 ++++++++++++++++++++++-------------------- src/windowsPtyAgent.ts | 4 +-- 2 files changed, 39 insertions(+), 37 deletions(-) diff --git a/src/win/conpty.cc b/src/win/conpty.cc index 5a390b762..97b9a475a 100644 --- a/src/win/conpty.cc +++ b/src/win/conpty.cc @@ -56,6 +56,13 @@ static pty_handle* get_pty_handle(int id) { return nullptr; } +void throwNanError(const Nan::FunctionCallbackInfo* info, const char* text) { + std::stringstream errorText; + errorText << "Cannot create process, error code: " << GetLastError(); + Nan::ThrowError(errorText.str().c_str()); + (*info).GetReturnValue().SetUndefined(); +} + static NAN_METHOD(PtyGetExitCode) { Nan::HandleScope scope; @@ -183,25 +190,23 @@ static NAN_METHOD(PtyStartProcess) { DWORD dwExit = 0; - if (info.Length() != 7 || + if (info.Length() != 6 || !info[0]->IsString() || !info[1]->IsString() || - !info[2]->IsString() || + !info[2]->IsNumber() || !info[3]->IsNumber() || - !info[4]->IsNumber() || - !info[5]->IsBoolean() || - !info[6]->IsString()) { - Nan::ThrowError("Usage: pty.startProcess(file, cmdline, cwd, cols, rows, debug, pipeName)"); + !info[4]->IsBoolean() || + !info[5]->IsString()) { + Nan::ThrowError("Usage: pty.startProcess(file, cwd, cols, rows, debug, pipeName)"); return; } const wchar_t *filename = path_util::to_wstring(v8::String::Utf8Value(info[0]->ToString())); - const wchar_t *cmdline = path_util::to_wstring(v8::String::Utf8Value(info[1]->ToString())); - const wchar_t *cwd = path_util::to_wstring(v8::String::Utf8Value(info[2]->ToString())); - const SHORT cols = info[3]->Uint32Value(); - const SHORT rows = info[4]->Uint32Value(); - const bool debug = info[5]->ToBoolean()->IsTrue(); - const wchar_t *pipeName = path_util::to_wstring(v8::String::Utf8Value(info[6]->ToString())); + const wchar_t *cwd = path_util::to_wstring(v8::String::Utf8Value(info[1]->ToString())); + const SHORT cols = info[2]->Uint32Value(); + const SHORT rows = info[3]->Uint32Value(); + const bool debug = info[4]->ToBoolean()->IsTrue(); + const wchar_t *pipeName = path_util::to_wstring(v8::String::Utf8Value(info[5]->ToString())); // use environment 'Path' variable to determine location of // the relative path that we have recieved (e.g cmd.exe) @@ -266,7 +271,6 @@ static NAN_METHOD(PtyStartProcess) { cleanup: delete filename; - delete cmdline; delete cwd; } @@ -275,7 +279,6 @@ std::vector vectorFromString(const std::basic_string &str) { return std::vector(str.begin(), str.end()); } - static NAN_METHOD(PtyConnect) { Nan::HandleScope scope; @@ -283,21 +286,20 @@ static NAN_METHOD(PtyConnect) { // the Socket has attempted to connect to the other end, then actually // spawn the process here. - std::wstring str_cmdline; + std::stringstream errorText; BOOL fSuccess = FALSE; - std::unique_ptr mutableCommandline; - PROCESS_INFORMATION _piClient{}; - v8::Local marshal; - if (info.Length() != 2 || + if (info.Length() != 3 || !info[0]->IsNumber() || - !info[1]->IsArray()) { - Nan::ThrowError("Usage: pty.connect(id, env)"); + !info[1]->IsString() || + !info[2]->IsArray()) { + Nan::ThrowError("Usage: pty.connect(id, cmdline, env)"); return; } const int id = info[0]->Int32Value(); - const v8::Handle envValues = v8::Handle::Cast(info[1]); + const std::wstring cmdline(path_util::to_wstring(v8::String::Utf8Value(info[1]->ToString()))); + const v8::Handle envValues = v8::Handle::Cast(info[2]); // Create environment block std::wstring env; @@ -319,8 +321,7 @@ static NAN_METHOD(PtyConnect) { success = ConnectNamedPipe(handle->hOut, nullptr); // Attach the pseudoconsole to the client application we're creating - STARTUPINFOEXW siEx; - siEx = {0}; + STARTUPINFOEXW siEx{0}; siEx.StartupInfo.cb = sizeof(STARTUPINFOEXW); size_t size; InitializeProcThreadAttributeList(NULL, 1, 0, &size); @@ -328,6 +329,9 @@ static NAN_METHOD(PtyConnect) { siEx.lpAttributeList = reinterpret_cast(attrList); fSuccess = InitializeProcThreadAttributeList(siEx.lpAttributeList, 1, 0, (PSIZE_T)&size); + if (!fSuccess) { + return throwNanError(&info, "InitializeProcThreadAttributeList failed, error code: "); + } fSuccess = UpdateProcThreadAttribute(siEx.lpAttributeList, 0, PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE, @@ -335,13 +339,15 @@ static NAN_METHOD(PtyConnect) { sizeof(HPCON), NULL, NULL); + if (!fSuccess) { + return throwNanError(&info, "UpdateProcThreadAttribute failed, error code: "); + } - // You need to pass a MUTABLE commandline to CreateProcess, so convert our const wchar_t* here. - // TODO: Pull in cmdline - str_cmdline = L"cmd.exe"; - mutableCommandline = std::make_unique(str_cmdline.length() + 1); - HRESULT hr = StringCchCopyW(mutableCommandline.get(), str_cmdline.length() + 1, str_cmdline.c_str()); + // A mutable commandline needs to be passed to CreateProcessW + std::unique_ptr mutableCommandline = std::make_unique(cmdline.length() + 1); + HRESULT hr = StringCchCopyW(mutableCommandline.get(), cmdline.length() + 1, cmdline.c_str()); + PROCESS_INFORMATION _piClient{}; fSuccess = !!CreateProcessW( nullptr, mutableCommandline.get(), @@ -354,15 +360,11 @@ static NAN_METHOD(PtyConnect) { &siEx.StartupInfo, // lpStartupInfo &_piClient // lpProcessInformation ); - - // TODO: return the information about the client application out to the caller? - DWORD error = GetLastError(); - marshal = Nan::New(); - if (!fSuccess) { - marshal->Set(Nan::New("error").ToLocalChecked(), Nan::New(error)); + return throwNanError(&info, "Cannot create process, error code: "); } + v8::Local marshal = Nan::New(); marshal->Set(Nan::New("pid").ToLocalChecked(), Nan::New(_piClient.dwProcessId)); info.GetReturnValue().Set(marshal); } diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts index 1d96e5fab..895a9545e 100644 --- a/src/windowsPtyAgent.ts +++ b/src/windowsPtyAgent.ts @@ -61,7 +61,7 @@ export class WindowsPtyAgent { // Open pty session. let term; if (this._useConpty) { - term = pty.startProcess(file, commandLine, cwd, cols, rows, debug, this._generatePipeName()); + term = pty.startProcess(file, cwd, cols, rows, debug, this._generatePipeName()); } else { term = pty.startProcess(file, commandLine, env, cwd, cols, rows, debug); this._pid = term.pid; @@ -97,7 +97,7 @@ export class WindowsPtyAgent { // or can we do this synchronously like this? if (this._useConpty) { console.log('this._pty = ' + this._pty); - const connect = pty.connect(this._pty, env); + const connect = pty.connect(this._pty, commandLine, env); console.log('connect.error' + connect.error); this._innerPid = connect.pid; } From ba6d4669518048fce824d9383d61829850f1dae6 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 17 Oct 2018 14:59:51 -0700 Subject: [PATCH 23/55] Support cwd --- examples/fork/index.js | 8 +++---- src/win/conpty.cc | 49 +++++++++++++++++++++++------------------- src/windowsPtyAgent.ts | 4 ++-- 3 files changed, 33 insertions(+), 28 deletions(-) diff --git a/examples/fork/index.js b/examples/fork/index.js index 1fa2aaca9..d9ee720a4 100644 --- a/examples/fork/index.js +++ b/examples/fork/index.js @@ -1,14 +1,14 @@ var os = require('os'); var pty = require('../..'); -var shell = os.platform() === 'win32' ? 'cmd.exe' : 'bash'; +var shell = os.platform() === 'win32' ? 'powershell.exe' : 'bash'; var ptyProcess = pty.spawn(shell, [], { name: 'xterm-256color', cols: 80, - rows: 19, - cwd: process.env.HOME, - env: Object.assign(process.env, { TEST: "abc" }) + rows: 26, + cwd: os.platform() === 'win32' ? process.env.USERPROFILE : process.env.HOME, + env: Object.assign({ TEST: "abc" }, process.env) }); ptyProcess.on('data', function(data) { diff --git a/src/win/conpty.cc b/src/win/conpty.cc index 97b9a475a..e7286d11f 100644 --- a/src/win/conpty.cc +++ b/src/win/conpty.cc @@ -190,23 +190,22 @@ static NAN_METHOD(PtyStartProcess) { DWORD dwExit = 0; - if (info.Length() != 6 || + if (info.Length() != 5 || !info[0]->IsString() || - !info[1]->IsString() || + !info[1]->IsNumber() || !info[2]->IsNumber() || - !info[3]->IsNumber() || - !info[4]->IsBoolean() || - !info[5]->IsString()) { - Nan::ThrowError("Usage: pty.startProcess(file, cwd, cols, rows, debug, pipeName)"); + !info[3]->IsBoolean() || + !info[4]->IsString()) { + Nan::ThrowError("Usage: pty.startProcess(file, cols, rows, debug, pipeName)"); return; } const wchar_t *filename = path_util::to_wstring(v8::String::Utf8Value(info[0]->ToString())); - const wchar_t *cwd = path_util::to_wstring(v8::String::Utf8Value(info[1]->ToString())); - const SHORT cols = info[2]->Uint32Value(); - const SHORT rows = info[3]->Uint32Value(); - const bool debug = info[4]->ToBoolean()->IsTrue(); - const wchar_t *pipeName = path_util::to_wstring(v8::String::Utf8Value(info[5]->ToString())); + // const wchar_t *cwd = path_util::to_wstring(v8::String::Utf8Value(info[1]->ToString())); + const SHORT cols = info[1]->Uint32Value(); + const SHORT rows = info[2]->Uint32Value(); + const bool debug = info[3]->ToBoolean()->IsTrue(); + const wchar_t *pipeName = path_util::to_wstring(v8::String::Utf8Value(info[4]->ToString())); // use environment 'Path' variable to determine location of // the relative path that we have recieved (e.g cmd.exe) @@ -271,7 +270,6 @@ static NAN_METHOD(PtyStartProcess) { cleanup: delete filename; - delete cwd; } template @@ -289,19 +287,29 @@ static NAN_METHOD(PtyConnect) { std::stringstream errorText; BOOL fSuccess = FALSE; - if (info.Length() != 3 || + if (info.Length() != 4 || !info[0]->IsNumber() || !info[1]->IsString() || - !info[2]->IsArray()) { - Nan::ThrowError("Usage: pty.connect(id, cmdline, env)"); + !info[2]->IsString() || + !info[3]->IsArray()) { + Nan::ThrowError("Usage: pty.connect(id, cmdline, cwd, env)"); return; } const int id = info[0]->Int32Value(); const std::wstring cmdline(path_util::to_wstring(v8::String::Utf8Value(info[1]->ToString()))); - const v8::Handle envValues = v8::Handle::Cast(info[2]); + const std::wstring cwd(path_util::to_wstring(v8::String::Utf8Value(info[2]->ToString()))); + const v8::Handle envValues = v8::Handle::Cast(info[3]); + + // Prepare command line + std::unique_ptr mutableCommandline = std::make_unique(cmdline.length() + 1); + HRESULT hr = StringCchCopyW(mutableCommandline.get(), cmdline.length() + 1, cmdline.c_str()); + + // Prepare cwd + std::unique_ptr mutableCwd = std::make_unique(cwd.length() + 1); + hr = StringCchCopyW(mutableCwd.get(), cwd.length() + 1, cwd.c_str()); - // Create environment block + // Prepare environment std::wstring env; if (!envValues.IsEmpty()) { std::wstringstream envBlock; @@ -315,6 +323,7 @@ static NAN_METHOD(PtyConnect) { auto envV = vectorFromString(env); LPWSTR envArg = envV.empty() ? nullptr : envV.data(); + // Fetch pty handle from ID and start process const pty_handle* handle = get_pty_handle(id); BOOL success = ConnectNamedPipe(handle->hIn, nullptr); @@ -343,10 +352,6 @@ static NAN_METHOD(PtyConnect) { return throwNanError(&info, "UpdateProcThreadAttribute failed, error code: "); } - // A mutable commandline needs to be passed to CreateProcessW - std::unique_ptr mutableCommandline = std::make_unique(cmdline.length() + 1); - HRESULT hr = StringCchCopyW(mutableCommandline.get(), cmdline.length() + 1, cmdline.c_str()); - PROCESS_INFORMATION _piClient{}; fSuccess = !!CreateProcessW( nullptr, @@ -356,7 +361,7 @@ static NAN_METHOD(PtyConnect) { false, // bInheritHandles VERY IMPORTANT that this is false EXTENDED_STARTUPINFO_PRESENT | CREATE_UNICODE_ENVIRONMENT, // dwCreationFlags envArg, // lpEnvironment - nullptr, // lpCurrentDirectory + mutableCwd.get(), // lpCurrentDirectory &siEx.StartupInfo, // lpStartupInfo &_piClient // lpProcessInformation ); diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts index 895a9545e..e99242a24 100644 --- a/src/windowsPtyAgent.ts +++ b/src/windowsPtyAgent.ts @@ -61,7 +61,7 @@ export class WindowsPtyAgent { // Open pty session. let term; if (this._useConpty) { - term = pty.startProcess(file, cwd, cols, rows, debug, this._generatePipeName()); + term = pty.startProcess(file, cols, rows, debug, this._generatePipeName()); } else { term = pty.startProcess(file, commandLine, env, cwd, cols, rows, debug); this._pid = term.pid; @@ -97,7 +97,7 @@ export class WindowsPtyAgent { // or can we do this synchronously like this? if (this._useConpty) { console.log('this._pty = ' + this._pty); - const connect = pty.connect(this._pty, commandLine, env); + const connect = pty.connect(this._pty, commandLine, cwd, env); console.log('connect.error' + connect.error); this._innerPid = connect.pid; } From 79ea7076e09fccb24da254af6dbac385ede42d3c Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 17 Oct 2018 15:10:52 -0700 Subject: [PATCH 24/55] Fix build for winpty --- binding.gyp | 59 +++++++++++++++++++++++++++++++++++++++++- examples/fork/index.js | 3 ++- src/windowsPtyAgent.ts | 2 +- 3 files changed, 61 insertions(+), 3 deletions(-) diff --git a/binding.gyp b/binding.gyp index b10989124..ec768a6b7 100644 --- a/binding.gyp +++ b/binding.gyp @@ -69,8 +69,65 @@ 'libraries': [ 'shlwapi.lib' ] - } + }, + # { + # 'target_name': 'pty', + # 'include_dirs' : [ + # '= 17692; } console.log('useConpty?', this._useConpty); - pty = require(path.join('..', 'build', 'Debug', `${this._useConpty ? 'conpty' : 'winpty'}.node`)); + pty = require(path.join('..', 'build', 'Debug', `${this._useConpty ? 'conpty' : 'pty'}.node`)); } // Sanitize input variable. From ee7b2dbc0b5b972d8a941d5eef6a3c4088985692 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 17 Oct 2018 15:13:35 -0700 Subject: [PATCH 25/55] Build winpty as well --- binding.gyp | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/binding.gyp b/binding.gyp index ec768a6b7..092104f14 100644 --- a/binding.gyp +++ b/binding.gyp @@ -70,26 +70,26 @@ 'shlwapi.lib' ] }, - # { - # 'target_name': 'pty', - # 'include_dirs' : [ - # ' Date: Wed, 17 Oct 2018 15:23:39 -0700 Subject: [PATCH 26/55] Use release build --- scripts/install.js | 2 +- scripts/post-install.js | 2 ++ src/windowsPtyAgent.ts | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/scripts/install.js b/scripts/install.js index b6058cfc8..0b5b5ce0c 100644 --- a/scripts/install.js +++ b/scripts/install.js @@ -4,7 +4,7 @@ const os = require('os'); const path = require('path'); const spawn = require('child_process').spawn; -const p = spawn(os.platform() === 'win32' ? 'node-gyp.cmd' : 'node-gyp', ['rebuild', '--debug'], { +const p = spawn(os.platform() === 'win32' ? 'node-gyp.cmd' : 'node-gyp', ['rebuild'/*, '--debug'*/], { cwd: path.join(__dirname, '..'), stdio: 'inherit' }); diff --git a/scripts/post-install.js b/scripts/post-install.js index 57413d818..e09aeb333 100644 --- a/scripts/post-install.js +++ b/scripts/post-install.js @@ -3,6 +3,8 @@ var path = require('path'); var RELEASE_DIR = path.join(__dirname, '..', 'build', 'Release'); var BUILD_FILES = [ + path.join(RELEASE_DIR, 'conpty.node'), + path.join(RELEASE_DIR, 'conpty.pdb'), path.join(RELEASE_DIR, 'pty.node'), path.join(RELEASE_DIR, 'pty.pdb'), path.join(RELEASE_DIR, 'winpty-agent.exe'), diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts index 001089eb2..d22e696a4 100644 --- a/src/windowsPtyAgent.ts +++ b/src/windowsPtyAgent.ts @@ -49,7 +49,7 @@ export class WindowsPtyAgent { this._useConpty = this._getWindowsBuildNumber() >= 17692; } console.log('useConpty?', this._useConpty); - pty = require(path.join('..', 'build', 'Debug', `${this._useConpty ? 'conpty' : 'pty'}.node`)); + pty = require(path.join('..', 'build', /*'Debug'*/'Release', `${this._useConpty ? 'conpty' : 'pty'}.node`)); } // Sanitize input variable. From 8ecb80c5c423695cc969b0391000a0d07752733e Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 17 Oct 2018 16:04:47 -0700 Subject: [PATCH 27/55] Fix handling of size type --- src/win/conpty.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/win/conpty.cc b/src/win/conpty.cc index e7286d11f..98eb44a1d 100644 --- a/src/win/conpty.cc +++ b/src/win/conpty.cc @@ -332,12 +332,12 @@ static NAN_METHOD(PtyConnect) { // Attach the pseudoconsole to the client application we're creating STARTUPINFOEXW siEx{0}; siEx.StartupInfo.cb = sizeof(STARTUPINFOEXW); - size_t size; - InitializeProcThreadAttributeList(NULL, 1, 0, &size); - BYTE *attrList = new BYTE[size]; + PSIZE_T size; + InitializeProcThreadAttributeList(NULL, 1, 0, size); + BYTE *attrList = new BYTE[*size]; siEx.lpAttributeList = reinterpret_cast(attrList); - fSuccess = InitializeProcThreadAttributeList(siEx.lpAttributeList, 1, 0, (PSIZE_T)&size); + fSuccess = InitializeProcThreadAttributeList(siEx.lpAttributeList, 1, 0, size); if (!fSuccess) { return throwNanError(&info, "InitializeProcThreadAttributeList failed, error code: "); } From de6a767fefd2bd5a55d0270b5d74b9805da31b21 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 18 Oct 2018 15:12:01 -0700 Subject: [PATCH 28/55] Allow use conpty option to work beyond first terminal --- examples/fork/index.js | 4 ++-- src/windowsPtyAgent.ts | 39 +++++++++++++++++++++++++-------------- 2 files changed, 27 insertions(+), 16 deletions(-) diff --git a/examples/fork/index.js b/examples/fork/index.js index d1781dff6..ac669e0b7 100644 --- a/examples/fork/index.js +++ b/examples/fork/index.js @@ -9,7 +9,7 @@ var ptyProcess = pty.spawn(shell, [], { rows: 26, cwd: os.platform() === 'win32' ? process.env.USERPROFILE : process.env.HOME, env: Object.assign({ TEST: "abc" }, process.env), - experimentalUseConpty: false + experimentalUseConpty: true }); ptyProcess.on('data', function(data) { @@ -22,7 +22,7 @@ ptyProcess.write('dir\r'); setTimeout(() => { ptyProcess.resize(30, 19); - ptyProcess.write('echo %TEST%\r'); + ptyProcess.write(shell === 'powershell.exe' ? '$Env:TEST\r' : 'echo %TEST%\r'); }, 2000); process.on('exit', () => { diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts index d22e696a4..05f45dfe2 100644 --- a/src/windowsPtyAgent.ts +++ b/src/windowsPtyAgent.ts @@ -8,7 +8,9 @@ import * as path from 'path'; import { Socket } from 'net'; import { ArgvOrCommandLine } from './types'; -let pty: any; +// TODO: Pull conpty/winpty details into its own interface? +let conptyNative: any; +let winptyNative: any; /** * Agent. Internal class. @@ -27,6 +29,7 @@ export class WindowsPtyAgent { private _fd: any; private _pty: number; + private _ptyNative: any; public get inSocket(): Socket { return this._inSocket; } public get outSocket(): Socket { return this._outSocket; } @@ -44,13 +47,20 @@ export class WindowsPtyAgent { debug: boolean, private _useConpty: boolean | undefined ) { - if (!pty) { - if (this._useConpty === undefined) { - this._useConpty = this._getWindowsBuildNumber() >= 17692; + if (this._useConpty === undefined) { + this._useConpty = this._getWindowsBuildNumber() >= 17692; + } + console.log('useConpty?', this._useConpty); + if (this._useConpty) { + if (!conptyNative) { + conptyNative = require(path.join('..', 'build', /*'Debug'*/'Release', 'conpty.node')); + } + } else { + if (!winptyNative) { + winptyNative = require(path.join('..', 'build', /*'Debug'*/'Release', 'pty.node')); } - console.log('useConpty?', this._useConpty); - pty = require(path.join('..', 'build', /*'Debug'*/'Release', `${this._useConpty ? 'conpty' : 'pty'}.node`)); } + this._ptyNative = this._useConpty ? conptyNative : winptyNative; // Sanitize input variable. cwd = path.resolve(cwd); @@ -61,9 +71,9 @@ export class WindowsPtyAgent { // Open pty session. let term; if (this._useConpty) { - term = pty.startProcess(file, cols, rows, debug, this._generatePipeName()); + term = this._ptyNative.startProcess(file, cols, rows, debug, this._generatePipeName()); } else { - term = pty.startProcess(file, commandLine, env, cwd, cols, rows, debug); + term = this._ptyNative.startProcess(file, commandLine, env, cwd, cols, rows, debug); this._pid = term.pid; } @@ -97,7 +107,7 @@ export class WindowsPtyAgent { // or can we do this synchronously like this? if (this._useConpty) { console.log('this._pty = ' + this._pty); - const connect = pty.connect(this._pty, commandLine, cwd, env); + const connect = this._ptyNative.connect(this._pty, commandLine, cwd, env); console.log('connect.error' + connect.error); this._innerPid = connect.pid; } @@ -105,7 +115,7 @@ export class WindowsPtyAgent { public resize(cols: number, rows: number): void { // TODO: Guard against invalid pty values - pty.resize(this._useConpty ? this._pty : this._pid, cols, rows); + this._ptyNative.resize(this._useConpty ? this._pty : this._pid, cols, rows); } public kill(): void { @@ -115,10 +125,10 @@ export class WindowsPtyAgent { this._outSocket.writable = false; // Tell the agent to kill the pty, this releases handles to the process if (this._useConpty) { - pty.kill(this._pty); + this._ptyNative.kill(this._pty); } else { - const processList: number[] = pty.getProcessList(this._pid); - pty.kill(this._pid, this._innerPidHandle); + const processList: number[] = this._ptyNative.getProcessList(this._pid); + this._ptyNative.kill(this._pid, this._innerPidHandle); // Since pty.kill will kill most processes by itself and process IDs can be // reused as soon as all handles to them are dropped, we want to immediately // kill the entire console process list. If we do not force kill all @@ -135,7 +145,8 @@ export class WindowsPtyAgent { } public getExitCode(): number { - return pty.getExitCode(this._innerPidHandle); + // TODO: Fix for conpty + return this._ptyNative.getExitCode(this._innerPidHandle); } private _getWindowsBuildNumber(): number { From dc0922c6c4ac0d5ac86f41d75e28ab9ee47d4f4c Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 18 Oct 2018 15:33:46 -0700 Subject: [PATCH 29/55] Fix fault in conpty.kill --- src/win/conpty.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/win/conpty.cc b/src/win/conpty.cc index 98eb44a1d..d302262ae 100644 --- a/src/win/conpty.cc +++ b/src/win/conpty.cc @@ -428,7 +428,7 @@ static NAN_METHOD(PtyKill) { bool fLoadedDll = hLibrary != nullptr; if (fLoadedDll) { - PFNCLOSEPSEUDOCONSOLE const pfnClosePseudoConsole = (PFNCLOSEPSEUDOCONSOLE)GetProcAddress((HMODULE)hLibrary, "CreatePseudoConsole"); + PFNCLOSEPSEUDOCONSOLE const pfnClosePseudoConsole = (PFNCLOSEPSEUDOCONSOLE)GetProcAddress((HMODULE)hLibrary, "ClosePseudoConsole"); if (pfnClosePseudoConsole) { pfnClosePseudoConsole(handle->hpc); From 4d18161d0fd5d2cf49b3c5b5cb89facef6c1d46a Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 18 Oct 2018 16:45:11 -0700 Subject: [PATCH 30/55] Remove old gyp definition --- binding.gyp | 55 ----------------------------------------------------- 1 file changed, 55 deletions(-) diff --git a/binding.gyp b/binding.gyp index 092104f14..7fa4d0023 100644 --- a/binding.gyp +++ b/binding.gyp @@ -1,59 +1,4 @@ { - # 'targets': [{ - # 'target_name': 'pty', - # 'include_dirs' : [ - # ' Date: Fri, 19 Oct 2018 09:31:20 -0700 Subject: [PATCH 31/55] Load correct build type in unix terminal --- src/unixTerminal.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/unixTerminal.ts b/src/unixTerminal.ts index 251381ad0..8d114f26d 100644 --- a/src/unixTerminal.ts +++ b/src/unixTerminal.ts @@ -17,7 +17,7 @@ declare interface INativePty { pty: string; } -const pty = require(path.join('..', 'build', 'Debug', 'pty.node')); +const pty = require(path.join('..', 'build', /*'Debug'*/'Release', 'pty.node')); const DEFAULT_FILE = 'sh'; const DEFAULT_NAME = 'xterm'; From 3c977543ff1cb1c92e048b2f7f07fb4ea27ea551 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 19 Oct 2018 09:40:32 -0700 Subject: [PATCH 32/55] Use release, fallback to debug --- scripts/install.js | 6 +++++- src/unixTerminal.ts | 6 ++---- src/utils.ts | 10 ++++++++++ src/windowsPtyAgent.ts | 5 +++-- 4 files changed, 20 insertions(+), 7 deletions(-) diff --git a/scripts/install.js b/scripts/install.js index 0b5b5ce0c..4477e48b6 100644 --- a/scripts/install.js +++ b/scripts/install.js @@ -4,7 +4,11 @@ const os = require('os'); const path = require('path'); const spawn = require('child_process').spawn; -const p = spawn(os.platform() === 'win32' ? 'node-gyp.cmd' : 'node-gyp', ['rebuild'/*, '--debug'*/], { +const args = ['rebuild']; +if (process.env.NODE_PTY_DEBUG) { + args.push('--debug'); +} +const p = spawn(os.platform() === 'win32' ? 'node-gyp.cmd' : 'node-gyp', args, { cwd: path.join(__dirname, '..'), stdio: 'inherit' }); diff --git a/src/unixTerminal.ts b/src/unixTerminal.ts index 8d114f26d..8fba06b0a 100644 --- a/src/unixTerminal.ts +++ b/src/unixTerminal.ts @@ -4,12 +4,10 @@ */ import * as net from 'net'; -import * as path from 'path'; -import * as tty from 'tty'; import { Terminal, DEFAULT_COLS, DEFAULT_ROWS } from './terminal'; import { IProcessEnv, IPtyForkOptions, IPtyOpenOptions } from './interfaces'; import { ArgvOrCommandLine } from './types'; -import { assign } from './utils'; +import { assign, loadNative } from './utils'; declare interface INativePty { master: number; @@ -17,7 +15,7 @@ declare interface INativePty { pty: string; } -const pty = require(path.join('..', 'build', /*'Debug'*/'Release', 'pty.node')); +const pty = loadNative('pty'); const DEFAULT_FILE = 'sh'; const DEFAULT_NAME = 'xterm'; diff --git a/src/utils.ts b/src/utils.ts index ffd7d1dac..0d62c3c47 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -2,7 +2,17 @@ * Copyright (c) 2017, Daniel Imms (MIT License). */ +import * as path from 'path'; + export function assign(target: any, ...sources: any[]): any { sources.forEach(source => Object.keys(source).forEach(key => target[key] = source[key])); return target; } + +export function loadNative(moduleName: string): any { + try { + return require(path.join('..', 'build', 'Release', `${moduleName}.node`)); + } catch { + return require(path.join('..', 'build', 'Debug', `${moduleName}.node`)); + } +} diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts index 05f45dfe2..07bf9612c 100644 --- a/src/windowsPtyAgent.ts +++ b/src/windowsPtyAgent.ts @@ -7,6 +7,7 @@ import * as os from 'os'; import * as path from 'path'; import { Socket } from 'net'; import { ArgvOrCommandLine } from './types'; +import { loadNative } from './utils'; // TODO: Pull conpty/winpty details into its own interface? let conptyNative: any; @@ -53,11 +54,11 @@ export class WindowsPtyAgent { console.log('useConpty?', this._useConpty); if (this._useConpty) { if (!conptyNative) { - conptyNative = require(path.join('..', 'build', /*'Debug'*/'Release', 'conpty.node')); + conptyNative = loadNative('conpty'); } } else { if (!winptyNative) { - winptyNative = require(path.join('..', 'build', /*'Debug'*/'Release', 'pty.node')); + winptyNative = loadNative('pty'); } } this._ptyNative = this._useConpty ? conptyNative : winptyNative; From 21ab08d7a00808d5af7715ca01f792d8a2569cba Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 19 Oct 2018 10:35:51 -0700 Subject: [PATCH 33/55] Remove goto from conpty --- src/win/conpty.cc | 67 +++++++++++++---------------------------------- 1 file changed, 18 insertions(+), 49 deletions(-) diff --git a/src/win/conpty.cc b/src/win/conpty.cc index d302262ae..2a1d58682 100644 --- a/src/win/conpty.cc +++ b/src/win/conpty.cc @@ -56,6 +56,11 @@ static pty_handle* get_pty_handle(int id) { return nullptr; } +template +std::vector vectorFromString(const std::basic_string &str) { + return std::vector(str.begin(), str.end()); +} + void throwNanError(const Nan::FunctionCallbackInfo* info, const char* text) { std::stringstream errorText; errorText << "Cannot create process, error code: " << GetLastError(); @@ -78,35 +83,12 @@ static NAN_METHOD(PtyGetExitCode) { info.GetReturnValue().Set(Nan::New(exitCode)); } -// static NAN_METHOD(PtyGetProcessList) { -// Nan::HandleScope scope; - -// if (info.Length() != 1 || -// !info[0]->IsNumber()) { -// Nan::ThrowError("Usage: pty.getProcessList(pid)"); -// return; -// } - -// int pid = info[0]->Int32Value(); - -// // winpty_t *pc = get_pipe_handle(pid); -// // if (pc == nullptr) { -// info.GetReturnValue().Set(Nan::New(0)); -// return; -// // } -// // int processList[64]; -// // const int processCount = 64; -// // int actualCount = winpty_get_console_process_list(pc, processList, processCount, nullptr); - -// // v8::Local result = Nan::New(actualCount); -// // for (uint32_t i = 0; i < actualCount; i++) { -// // Nan::Set(result, i, Nan::New(processList[i])); -// // } -// // info.GetReturnValue().Set(result); -// } - // Returns a new server named pipe. It has not yet been connected. -bool createDataServerPipe(bool write, std::wstring kind, HANDLE* hServer, std::wstring &name, std::wstring &pipeName) +bool createDataServerPipe(bool write, + std::wstring kind, + HANDLE* hServer, + std::wstring &name, + const std::wstring &pipeName) { *hServer = INVALID_HANDLE_VALUE; @@ -138,7 +120,7 @@ HRESULT CreateNamedPipesAndPseudoConsole(COORD size, HPCON* phPC, std::wstring& inName, std::wstring& outName, - std::wstring& pipeName) + const std::wstring& pipeName) { HANDLE hLibrary = LoadLibraryExW(L"kernel32.dll", 0, 0); bool fLoadedDll = hLibrary != nullptr; @@ -182,7 +164,7 @@ static NAN_METHOD(PtyStartProcess) { Nan::HandleScope scope; v8::Local marshal; - std::wstring inName, outName, str_cmdline, str_pipeName; + std::wstring inName, outName; BOOL fSuccess = FALSE; std::unique_ptr mutableCommandline; PROCESS_INFORMATION _piClient{}; @@ -200,18 +182,17 @@ static NAN_METHOD(PtyStartProcess) { return; } - const wchar_t *filename = path_util::to_wstring(v8::String::Utf8Value(info[0]->ToString())); - // const wchar_t *cwd = path_util::to_wstring(v8::String::Utf8Value(info[1]->ToString())); + const std::wstring filename(path_util::to_wstring(v8::String::Utf8Value(info[0]->ToString()))); const SHORT cols = info[1]->Uint32Value(); const SHORT rows = info[2]->Uint32Value(); const bool debug = info[3]->ToBoolean()->IsTrue(); - const wchar_t *pipeName = path_util::to_wstring(v8::String::Utf8Value(info[4]->ToString())); + const std::wstring pipeName(path_util::to_wstring(v8::String::Utf8Value(info[4]->ToString()))); // use environment 'Path' variable to determine location of // the relative path that we have recieved (e.g cmd.exe) std::wstring shellpath; - if (::PathIsRelativeW(filename)) { - shellpath = path_util::get_shell_path(filename); + if (::PathIsRelativeW(filename.c_str())) { + shellpath = path_util::get_shell_path(filename.c_str()); } else { shellpath = filename; } @@ -221,14 +202,12 @@ static NAN_METHOD(PtyStartProcess) { if (shellpath.empty() || !path_util::file_exists(shellpath)) { why << "File not found: " << shellpath_; Nan::ThrowError(why.str().c_str()); - goto cleanup; + return; } - str_pipeName = pipeName; - HANDLE hIn, hOut; HPCON hpc; - HRESULT hr = CreateNamedPipesAndPseudoConsole({cols, rows}, 0, &hIn, &hOut, &hpc, inName, outName, str_pipeName); + HRESULT hr = CreateNamedPipesAndPseudoConsole({cols, rows}, 0, &hIn, &hOut, &hpc, inName, outName, pipeName); // Set return values marshal = Nan::New(); @@ -265,16 +244,6 @@ static NAN_METHOD(PtyStartProcess) { marshal->Set(Nan::New("conout").ToLocalChecked(), Nan::New(conoutPipeNameStr).ToLocalChecked()); } info.GetReturnValue().Set(marshal); - - goto cleanup; - -cleanup: - delete filename; -} - -template -std::vector vectorFromString(const std::basic_string &str) { - return std::vector(str.begin(), str.end()); } static NAN_METHOD(PtyConnect) { From a0af090e90b98e070826181f8102a974305aa9ae Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 19 Oct 2018 10:42:17 -0700 Subject: [PATCH 34/55] Fix throwing node exceptions --- src/win/conpty.cc | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/win/conpty.cc b/src/win/conpty.cc index 2a1d58682..764d83945 100644 --- a/src/win/conpty.cc +++ b/src/win/conpty.cc @@ -61,9 +61,12 @@ std::vector vectorFromString(const std::basic_string &str) { return std::vector(str.begin(), str.end()); } -void throwNanError(const Nan::FunctionCallbackInfo* info, const char* text) { +void throwNanError(const Nan::FunctionCallbackInfo* info, const char* text, const bool getLastError) { std::stringstream errorText; - errorText << "Cannot create process, error code: " << GetLastError(); + errorText << text; + if (getLastError) { + errorText << ", error code: " << GetLastError(); + } Nan::ThrowError(errorText.str().c_str()); (*info).GetReturnValue().SetUndefined(); } @@ -168,9 +171,6 @@ static NAN_METHOD(PtyStartProcess) { BOOL fSuccess = FALSE; std::unique_ptr mutableCommandline; PROCESS_INFORMATION _piClient{}; - std::stringstream why; - - DWORD dwExit = 0; if (info.Length() != 5 || !info[0]->IsString() || @@ -200,6 +200,7 @@ static NAN_METHOD(PtyStartProcess) { std::string shellpath_(shellpath.begin(), shellpath.end()); if (shellpath.empty() || !path_util::file_exists(shellpath)) { + std::stringstream why; why << "File not found: " << shellpath_; Nan::ThrowError(why.str().c_str()); return; @@ -308,7 +309,7 @@ static NAN_METHOD(PtyConnect) { fSuccess = InitializeProcThreadAttributeList(siEx.lpAttributeList, 1, 0, size); if (!fSuccess) { - return throwNanError(&info, "InitializeProcThreadAttributeList failed, error code: "); + return throwNanError(&info, "InitializeProcThreadAttributeList failed", true); } fSuccess = UpdateProcThreadAttribute(siEx.lpAttributeList, 0, @@ -318,7 +319,7 @@ static NAN_METHOD(PtyConnect) { NULL, NULL); if (!fSuccess) { - return throwNanError(&info, "UpdateProcThreadAttribute failed, error code: "); + return throwNanError(&info, "UpdateProcThreadAttribute failed", true); } PROCESS_INFORMATION _piClient{}; @@ -335,7 +336,7 @@ static NAN_METHOD(PtyConnect) { &_piClient // lpProcessInformation ); if (!fSuccess) { - return throwNanError(&info, "Cannot create process, error code: "); + return throwNanError(&info, "Cannot create process", true); } v8::Local marshal = Nan::New(); From d9d44ea1d2928c8d6a85b23d4c66ca3816faa63c Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 18 Dec 2018 16:38:06 -0800 Subject: [PATCH 35/55] Remove Terminal.redraw It wasn't part of the API, not used anywhere AFAIK --- src/terminal.ts | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/terminal.ts b/src/terminal.ts index 7de63e197..f68db7230 100644 --- a/src/terminal.ts +++ b/src/terminal.ts @@ -126,19 +126,6 @@ export abstract class Terminal implements ITerminal { public abstract get master(): Socket; public abstract get slave(): Socket; - // TODO: Should this be in the API? - public redraw(): void { - let cols = this._cols; - let rows = this._rows; - - // We could just send SIGWINCH, but most programs will ignore it if the - // size hasn't actually changed. - - this.resize(cols + 1, rows + 1); - - setTimeout(() => this.resize(cols, rows), 30); - } - protected _close(): void { this._socket.writable = false; this._socket.readable = false; From c81796efdaf91834b1e31e8079543a4fcbb5ddbe Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 18 Dec 2018 16:45:26 -0800 Subject: [PATCH 36/55] Resolve TODO --- src/win/conpty.cc | 3 +-- src/windowsPtyAgent.ts | 16 ++++++++-------- src/windowsTerminal.ts | 2 +- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/win/conpty.cc b/src/win/conpty.cc index 764d83945..03993667c 100644 --- a/src/win/conpty.cc +++ b/src/win/conpty.cc @@ -217,8 +217,7 @@ static NAN_METHOD(PtyStartProcess) { { // We were able to instantiate a conpty, yay! const int ptyId = InterlockedIncrement(&ptyCounter); - // TODO: Name this pty "id" - marshal->Set(Nan::New("pty").ToLocalChecked(), Nan::New(ptyId)); + marshal->Set(Nan::New("ptyId").ToLocalChecked(), Nan::New(ptyId)); ptyHandles.insert(ptyHandles.end(), new pty_handle(ptyId, hIn, hOut, hpc)); } else diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts index 6e2e159d0..5a542bd6a 100644 --- a/src/windowsPtyAgent.ts +++ b/src/windowsPtyAgent.ts @@ -29,14 +29,14 @@ export class WindowsPtyAgent { private _innerPidHandle: number; private _fd: any; - private _pty: number; + private _ptyId: number; private _ptyNative: any; public get inSocket(): Socket { return this._inSocket; } public get outSocket(): Socket { return this._outSocket; } public get fd(): any { return this._fd; } public get innerPid(): number { return this._innerPid; } - public get pty(): number { return this._pty; } + public get ptyId(): number { return this._ptyId; } constructor( file: string, @@ -85,9 +85,9 @@ export class WindowsPtyAgent { // Not available on windows. this._fd = term.fd; - // Generated incremental number that has no real purpose besides using it + // Generated incremental number that has no real purpose besides using it // as a terminal id. - this._pty = term.pty; + this._ptyId = term.ptyId; // Create terminal pipe IPC channel and forward to a local unix socket. this._outSocket = new Socket(); @@ -107,8 +107,8 @@ export class WindowsPtyAgent { // TODO: Do we *need* to timeout here or wait for the sockets to connect? // or can we do this synchronously like this? if (this._useConpty) { - console.log('this._pty = ' + this._pty); - const connect = this._ptyNative.connect(this._pty, commandLine, cwd, env); + console.log('this._ptyId = ' + this._ptyId); + const connect = this._ptyNative.connect(this._ptyId, commandLine, cwd, env); console.log('connect.error' + connect.error); this._innerPid = connect.pid; } @@ -116,7 +116,7 @@ export class WindowsPtyAgent { public resize(cols: number, rows: number): void { // TODO: Guard against invalid pty values - this._ptyNative.resize(this._useConpty ? this._pty : this._pid, cols, rows); + this._ptyNative.resize(this._useConpty ? this._ptyId : this._pid, cols, rows); } public kill(): void { @@ -126,7 +126,7 @@ export class WindowsPtyAgent { this._outSocket.writable = false; // Tell the agent to kill the pty, this releases handles to the process if (this._useConpty) { - this._ptyNative.kill(this._pty); + this._ptyNative.kill(this._ptyId); } else { const processList: number[] = this._ptyNative.getProcessList(this._pid); this._ptyNative.kill(this._pid, this._innerPidHandle); diff --git a/src/windowsTerminal.ts b/src/windowsTerminal.ts index 08e9e7c62..720c8648f 100644 --- a/src/windowsTerminal.ts +++ b/src/windowsTerminal.ts @@ -54,7 +54,7 @@ export class WindowsTerminal extends Terminal { // Not available until `ready` event emitted. this._pid = this._agent.innerPid; this._fd = this._agent.fd; - this._pty = this._agent.pty; + this._pty = this._agent.ptyId; // The forked windows terminal is not available until `ready` event is // emitted. From ee82095458b473fb45651aa0c8deb219e80d5bd3 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 18 Dec 2018 16:58:02 -0800 Subject: [PATCH 37/55] Revert "Resolve TODO" This reverts commit c81796efdaf91834b1e31e8079543a4fcbb5ddbe. --- src/win/conpty.cc | 3 ++- src/windowsPtyAgent.ts | 16 ++++++++-------- src/windowsTerminal.ts | 2 +- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/win/conpty.cc b/src/win/conpty.cc index 03993667c..764d83945 100644 --- a/src/win/conpty.cc +++ b/src/win/conpty.cc @@ -217,7 +217,8 @@ static NAN_METHOD(PtyStartProcess) { { // We were able to instantiate a conpty, yay! const int ptyId = InterlockedIncrement(&ptyCounter); - marshal->Set(Nan::New("ptyId").ToLocalChecked(), Nan::New(ptyId)); + // TODO: Name this pty "id" + marshal->Set(Nan::New("pty").ToLocalChecked(), Nan::New(ptyId)); ptyHandles.insert(ptyHandles.end(), new pty_handle(ptyId, hIn, hOut, hpc)); } else diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts index 5a542bd6a..6e2e159d0 100644 --- a/src/windowsPtyAgent.ts +++ b/src/windowsPtyAgent.ts @@ -29,14 +29,14 @@ export class WindowsPtyAgent { private _innerPidHandle: number; private _fd: any; - private _ptyId: number; + private _pty: number; private _ptyNative: any; public get inSocket(): Socket { return this._inSocket; } public get outSocket(): Socket { return this._outSocket; } public get fd(): any { return this._fd; } public get innerPid(): number { return this._innerPid; } - public get ptyId(): number { return this._ptyId; } + public get pty(): number { return this._pty; } constructor( file: string, @@ -85,9 +85,9 @@ export class WindowsPtyAgent { // Not available on windows. this._fd = term.fd; - // Generated incremental number that has no real purpose besides using it + // Generated incremental number that has no real purpose besides using it // as a terminal id. - this._ptyId = term.ptyId; + this._pty = term.pty; // Create terminal pipe IPC channel and forward to a local unix socket. this._outSocket = new Socket(); @@ -107,8 +107,8 @@ export class WindowsPtyAgent { // TODO: Do we *need* to timeout here or wait for the sockets to connect? // or can we do this synchronously like this? if (this._useConpty) { - console.log('this._ptyId = ' + this._ptyId); - const connect = this._ptyNative.connect(this._ptyId, commandLine, cwd, env); + console.log('this._pty = ' + this._pty); + const connect = this._ptyNative.connect(this._pty, commandLine, cwd, env); console.log('connect.error' + connect.error); this._innerPid = connect.pid; } @@ -116,7 +116,7 @@ export class WindowsPtyAgent { public resize(cols: number, rows: number): void { // TODO: Guard against invalid pty values - this._ptyNative.resize(this._useConpty ? this._ptyId : this._pid, cols, rows); + this._ptyNative.resize(this._useConpty ? this._pty : this._pid, cols, rows); } public kill(): void { @@ -126,7 +126,7 @@ export class WindowsPtyAgent { this._outSocket.writable = false; // Tell the agent to kill the pty, this releases handles to the process if (this._useConpty) { - this._ptyNative.kill(this._ptyId); + this._ptyNative.kill(this._pty); } else { const processList: number[] = this._ptyNative.getProcessList(this._pid); this._ptyNative.kill(this._pid, this._innerPidHandle); diff --git a/src/windowsTerminal.ts b/src/windowsTerminal.ts index 720c8648f..08e9e7c62 100644 --- a/src/windowsTerminal.ts +++ b/src/windowsTerminal.ts @@ -54,7 +54,7 @@ export class WindowsTerminal extends Terminal { // Not available until `ready` event emitted. this._pid = this._agent.innerPid; this._fd = this._agent.fd; - this._pty = this._agent.ptyId; + this._pty = this._agent.pty; // The forked windows terminal is not available until `ready` event is // emitted. From c2c4636c0b30af785bdbc345f3b44c0964495688 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 18 Dec 2018 17:00:35 -0800 Subject: [PATCH 38/55] Clean up --- src/win/conpty.cc | 6 ++---- src/windowsPtyAgent.ts | 3 --- src/windowsTerminal.ts | 1 - 3 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/win/conpty.cc b/src/win/conpty.cc index 764d83945..cca430e53 100644 --- a/src/win/conpty.cc +++ b/src/win/conpty.cc @@ -215,16 +215,14 @@ static NAN_METHOD(PtyStartProcess) { if (SUCCEEDED(hr)) { - // We were able to instantiate a conpty, yay! + // We were able to instantiate a conpty const int ptyId = InterlockedIncrement(&ptyCounter); - // TODO: Name this pty "id" marshal->Set(Nan::New("pty").ToLocalChecked(), Nan::New(ptyId)); ptyHandles.insert(ptyHandles.end(), new pty_handle(ptyId, hIn, hOut, hpc)); } else { - // We weren't able to start conpty. Fall back to winpty. - // TODO + // TODO: We weren't able to start conpty. Fall back to winpty. } // TODO: Pull in innerPid, innerPidHandle(?) diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts index 6e2e159d0..1a0899910 100644 --- a/src/windowsPtyAgent.ts +++ b/src/windowsPtyAgent.ts @@ -51,7 +51,6 @@ export class WindowsPtyAgent { if (this._useConpty === undefined) { this._useConpty = this._getWindowsBuildNumber() >= 17692; } - console.log('useConpty?', this._useConpty); if (this._useConpty) { if (!conptyNative) { conptyNative = loadNative('conpty'); @@ -104,8 +103,6 @@ export class WindowsPtyAgent { this._inSocket.connect(term.conin); // TODO: Wait for ready event? - // TODO: Do we *need* to timeout here or wait for the sockets to connect? - // or can we do this synchronously like this? if (this._useConpty) { console.log('this._pty = ' + this._pty); const connect = this._ptyNative.connect(this._pty, commandLine, cwd, env); diff --git a/src/windowsTerminal.ts b/src/windowsTerminal.ts index 08e9e7c62..2b25ab3be 100644 --- a/src/windowsTerminal.ts +++ b/src/windowsTerminal.ts @@ -40,7 +40,6 @@ export class WindowsTerminal extends Terminal { const name = opt.name || env.TERM || DEFAULT_NAME; const parsedEnv = this._parseEnv(env); - console.log('parsedEnv', parsedEnv); // If the terminal is ready this._isReady = false; From b0d8f5a98692f3226fb46025d0c4bc4d294a576c Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 19 Dec 2018 16:20:36 -0800 Subject: [PATCH 39/55] Temp working callback --- src/win/conpty.cc | 71 +++++++++++++++++++++++++++++++++++++----- src/windowsPtyAgent.ts | 11 ++++++- 2 files changed, 74 insertions(+), 8 deletions(-) diff --git a/src/win/conpty.cc b/src/win/conpty.cc index cca430e53..333476de3 100644 --- a/src/win/conpty.cc +++ b/src/win/conpty.cc @@ -245,6 +245,20 @@ static NAN_METHOD(PtyStartProcess) { info.GetReturnValue().Set(marshal); } +// VOID CALLBACK OnProcessExit( +// _In_ PVOID context, +// _In_ BOOLEAN TimerOrWaitFired) { +// v8::Handle contextObject = v8::Handle::Cast(*static_cast*>(context)); +// //*static_cast*>(context); +// v8::Handle funcValue = contextObject->Get(Nan::New("_$onProcessExit").ToLocalChecked()); +// MessageBox(0, "The process has exited.", "INFO", MB_OK); +// v8::Handle func = v8::Handle::Cast(funcValue); +// v8::Handle args[1]; +// func->Call(contextObject, 0, args); +// // TODO: UnregisterWait +// return; +// } + static NAN_METHOD(PtyConnect) { Nan::HandleScope scope; @@ -255,19 +269,21 @@ static NAN_METHOD(PtyConnect) { std::stringstream errorText; BOOL fSuccess = FALSE; - if (info.Length() != 4 || + if (info.Length() != 5 || !info[0]->IsNumber() || !info[1]->IsString() || !info[2]->IsString() || - !info[3]->IsArray()) { - Nan::ThrowError("Usage: pty.connect(id, cmdline, cwd, env)"); + !info[3]->IsArray() || + !info[5]->IsFunction()) { + Nan::ThrowError("Usage: pty.connect(id, cmdline, cwd, env, exitCallback)"); return; } const int id = info[0]->Int32Value(); const std::wstring cmdline(path_util::to_wstring(v8::String::Utf8Value(info[1]->ToString()))); const std::wstring cwd(path_util::to_wstring(v8::String::Utf8Value(info[2]->ToString()))); - const v8::Handle envValues = v8::Handle::Cast(info[3]); + const v8::Handle envValues = info[3].As(); + const v8::Handle exitCallback = info[4].As(); // Prepare command line std::unique_ptr mutableCommandline = std::make_unique(cmdline.length() + 1); @@ -320,7 +336,7 @@ static NAN_METHOD(PtyConnect) { return throwNanError(&info, "UpdateProcThreadAttribute failed", true); } - PROCESS_INFORMATION _piClient{}; + PROCESS_INFORMATION piClient{}; fSuccess = !!CreateProcessW( nullptr, mutableCommandline.get(), @@ -331,14 +347,55 @@ static NAN_METHOD(PtyConnect) { envArg, // lpEnvironment mutableCwd.get(), // lpCurrentDirectory &siEx.StartupInfo, // lpStartupInfo - &_piClient // lpProcessInformation + &piClient // lpProcessInformation ); if (!fSuccess) { return throwNanError(&info, "Cannot create process", true); } + // PHANDLE hNewHandle; + // RegisterWaitForSingleObject(hNewHandle, piClient.hProcess, OnProcessExit, NULL, INFINITE, WT_EXECUTEONLYONCE); + + // Nan::MakeCallback(info.This(), "_$onProcessExit", 0, NULL); + // v8::Handle context = info.This(); + // auto lambda = [context]() { + // v8::Handle value = context->Get(Nan::New("_$onProcessExit").ToLocalChecked()); + // // if (value->IsFunction()) { + // v8::Handle func = v8::Handle::Cast(value); + // v8::Handle args[1]; + // func->Call(context, 0, args); + // }; + // lambda(); + // MessageBox(0, "CALLED", "INFO", MB_OK); + // } + + + // auto lambda = [context]( + // _In_ PVOID lpParameter, + // _In_ BOOLEAN TimerOrWaitFired) { + // v8::Handle value = context->Get(Nan::New("_$onProcessExit").ToLocalChecked()); + // // if (value->IsFunction()) { + // v8::Handle func = v8::Handle::Cast(value); + // v8::Handle args[1]; + // args[0] = Nan::New("test"); + // func->Call(context, 1, args); + // }; + + // v8::Handle funcValue = context->Get(Nan::New("_$onProcessExit").ToLocalChecked()); + // MessageBox(0, "The process has exited.", "INFO", MB_OK); + // v8::Handle func = v8::Handle::Cast(funcValue); + Nan::Persistent cb; + cb.Reset(exitCallback); + v8::Handle args[1] = { Nan::New("test2").ToLocalChecked() }; + v8::Handle local = Nan::New(cb); + local->Call(Nan::GetCurrentContext()->Global(), 1, args); + + // baton->cb.Reset(v8::Local::Cast(info[9])); + // PHANDLE hNewHandle; + // RegisterWaitForSingleObject(hNewHandle, piClient.hProcess, OnProcessExit, (PVOID)&cb, INFINITE, WT_EXECUTEONLYONCE); + v8::Local marshal = Nan::New(); - marshal->Set(Nan::New("pid").ToLocalChecked(), Nan::New(_piClient.dwProcessId)); + marshal->Set(Nan::New("pid").ToLocalChecked(), Nan::New(piClient.dwProcessId)); info.GetReturnValue().Set(marshal); } diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts index 1a0899910..d0f8d6b2d 100644 --- a/src/windowsPtyAgent.ts +++ b/src/windowsPtyAgent.ts @@ -105,7 +105,7 @@ export class WindowsPtyAgent { if (this._useConpty) { console.log('this._pty = ' + this._pty); - const connect = this._ptyNative.connect(this._pty, commandLine, cwd, env); + const connect = this._ptyNative.connect(this._pty, commandLine, cwd, env, this._$onProcessExit.bind(this)); console.log('connect.error' + connect.error); this._innerPid = connect.pid; } @@ -159,6 +159,15 @@ export class WindowsPtyAgent { private _generatePipeName(): string { return `conpty-${Math.random() * 10000000}`; } + + /** + * Triggered from the native side when a contpy process exits. + */ + private _$onProcessExit(arg: string): void { + console.log('_$onProcessExit ' + arg); + console.log('this'); + console.log(this); + } } // Convert argc/argv into a Win32 command-line following the escaping convention From f0183e0c97a291004bb61982a279adbd0a11567f Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 19 Dec 2018 17:17:42 -0800 Subject: [PATCH 40/55] Why u no work? --- src/win/conpty.cc | 82 ++++++++++++++++++++++++++++-------------- src/windowsPtyAgent.ts | 1 + 2 files changed, 57 insertions(+), 26 deletions(-) diff --git a/src/win/conpty.cc b/src/win/conpty.cc index 333476de3..570ad0c39 100644 --- a/src/win/conpty.cc +++ b/src/win/conpty.cc @@ -19,6 +19,9 @@ #include #include "path_util.h" +struct pty_baton { + Nan::Persistent cb; +}; extern "C" void init(v8::Handle); @@ -245,19 +248,39 @@ static NAN_METHOD(PtyStartProcess) { info.GetReturnValue().Set(marshal); } -// VOID CALLBACK OnProcessExit( -// _In_ PVOID context, -// _In_ BOOLEAN TimerOrWaitFired) { -// v8::Handle contextObject = v8::Handle::Cast(*static_cast*>(context)); -// //*static_cast*>(context); -// v8::Handle funcValue = contextObject->Get(Nan::New("_$onProcessExit").ToLocalChecked()); -// MessageBox(0, "The process has exited.", "INFO", MB_OK); -// v8::Handle func = v8::Handle::Cast(funcValue); -// v8::Handle args[1]; -// func->Call(contextObject, 0, args); -// // TODO: UnregisterWait -// return; -// } +VOID CALLBACK OnProcessExit( + _In_ PVOID context, + _In_ BOOLEAN TimerOrWaitFired) { + // Nan::HandleScope scope; + pty_baton *baton = static_cast(context); + + + // v8::Handle args[1] = { Nan::New("test2").ToLocalChecked() }; + v8::Handle local = Nan::New(baton->cb); + local->Call(Nan::GetCurrentContext()->Global(), 0, 0); + + + + + // v8::Handle args[1] = { Nan::New("test2").ToLocalChecked() }; + // v8::Handle args[1]; + + // v8::Handle local = Nan::New(baton->cb); + // // TODO: Reset baton cb + // local->Call(Nan::GetCurrentContext()->Global(), 0, 0); + // MessageBox(0, "The process has exited.", "INFO", MB_OK); + + + + + // v8::Handle contextObject = v8::Handle::Cast(*static_cast*>(context)); + // //*static_cast*>(context); + // v8::Handle funcValue = contextObject->Get(Nan::New("_$onProcessExit").ToLocalChecked()); + // v8::Handle func = v8::Handle::Cast(funcValue); + // v8::Handle args[1]; + // func->Call(contextObject, 0, args); + // TODO: UnregisterWait +} static NAN_METHOD(PtyConnect) { Nan::HandleScope scope; @@ -274,7 +297,7 @@ static NAN_METHOD(PtyConnect) { !info[1]->IsString() || !info[2]->IsString() || !info[3]->IsArray() || - !info[5]->IsFunction()) { + !info[4]->IsFunction()) { Nan::ThrowError("Usage: pty.connect(id, cmdline, cwd, env, exitCallback)"); return; } @@ -283,7 +306,7 @@ static NAN_METHOD(PtyConnect) { const std::wstring cmdline(path_util::to_wstring(v8::String::Utf8Value(info[1]->ToString()))); const std::wstring cwd(path_util::to_wstring(v8::String::Utf8Value(info[2]->ToString()))); const v8::Handle envValues = info[3].As(); - const v8::Handle exitCallback = info[4].As(); + const v8::Local exitCallback = v8::Local::Cast(info[4]); // Prepare command line std::unique_ptr mutableCommandline = std::make_unique(cmdline.length() + 1); @@ -381,18 +404,25 @@ static NAN_METHOD(PtyConnect) { // func->Call(context, 1, args); // }; - // v8::Handle funcValue = context->Get(Nan::New("_$onProcessExit").ToLocalChecked()); - // MessageBox(0, "The process has exited.", "INFO", MB_OK); - // v8::Handle func = v8::Handle::Cast(funcValue); - Nan::Persistent cb; - cb.Reset(exitCallback); - v8::Handle args[1] = { Nan::New("test2").ToLocalChecked() }; - v8::Handle local = Nan::New(cb); - local->Call(Nan::GetCurrentContext()->Global(), 1, args); + pty_baton *baton = new pty_baton(); - // baton->cb.Reset(v8::Local::Cast(info[9])); - // PHANDLE hNewHandle; - // RegisterWaitForSingleObject(hNewHandle, piClient.hProcess, OnProcessExit, (PVOID)&cb, INFINITE, WT_EXECUTEONLYONCE); + baton->cb.Reset(exitCallback); + + // v8::Handle args[1] = { Nan::New("test2").ToLocalChecked() }; + // v8::Handle local = Nan::New(cb); + // local->Call(Nan::GetCurrentContext()->Global(), 1, args); + + + // pty_baton *b = static_cast((PVOID)baton); + // v8::Handle args[1] = { Nan::New("test2").ToLocalChecked() }; + // v8::Handle local = Nan::New(b->cb); + // local->Call(Nan::GetCurrentContext()->Global(), 0, 0); + + // OnProcessExit((PVOID)baton, 0); + + + PHANDLE hNewHandle; + RegisterWaitForSingleObject(hNewHandle, piClient.hProcess, OnProcessExit, (PVOID)baton, INFINITE, WT_EXECUTEONLYONCE); v8::Local marshal = Nan::New(); marshal->Set(Nan::New("pid").ToLocalChecked(), Nan::New(piClient.dwProcessId)); diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts index d0f8d6b2d..ac19a5aa2 100644 --- a/src/windowsPtyAgent.ts +++ b/src/windowsPtyAgent.ts @@ -167,6 +167,7 @@ export class WindowsPtyAgent { console.log('_$onProcessExit ' + arg); console.log('this'); console.log(this); + require('fs').writeFileSync('C:\\Users\\daimms.REDMOND\\Documents\\dev\\Tyriar\\node-pty\\out.log', arg); } } From 85b7ea60d98eb2e99a836569677ef02a7d35148a Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 20 Dec 2018 20:02:53 -0800 Subject: [PATCH 41/55] Disable DOM APIs in tsconfig.json --- tsconfig.json | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tsconfig.json b/tsconfig.json index 87c27a17d..a7a2c45cd 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -4,7 +4,10 @@ "target": "es5", "rootDir": "src", "outDir": "lib", - "sourceMap": true + "sourceMap": true, + "lib": [ + "es5" + ] }, "exclude": [ "node_modules", From e4c294a63ed029f0472664eb22423165e26c66e6 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 20 Dec 2018 20:40:17 -0800 Subject: [PATCH 42/55] Fixed all tests in conpty --- src/win/conpty.cc | 115 +++++++++++++----------------------- src/windowsPtyAgent.ts | 51 +++++++++++++--- src/windowsTerminal.test.ts | 7 ++- src/windowsTerminal.ts | 4 +- 4 files changed, 89 insertions(+), 88 deletions(-) diff --git a/src/win/conpty.cc b/src/win/conpty.cc index 570ad0c39..b17937e7c 100644 --- a/src/win/conpty.cc +++ b/src/win/conpty.cc @@ -19,10 +19,6 @@ #include #include "path_util.h" -struct pty_baton { - Nan::Persistent cb; -}; - extern "C" void init(v8::Handle); // Taken from the RS5 Windows SDK, but redefined here in case we're targeting <= 17134 @@ -37,12 +33,18 @@ typedef void (*PFNCLOSEPSEUDOCONSOLE)(HPCON hpc); #endif -// TODO: Pull pty handle stuff into its own class struct pty_handle { int id; HANDLE hIn; HANDLE hOut; HPCON hpc; + + HANDLE hShell; + PHANDLE hWait; + Nan::Persistent cb; + uv_async_t async; + uv_thread_t tid; + pty_handle(int _id, HANDLE _hIn, HANDLE _hOut, HPCON _hpc) : id(_id), hIn(_hIn), hOut(_hOut), hpc(_hpc) {}; }; @@ -248,38 +250,35 @@ static NAN_METHOD(PtyStartProcess) { info.GetReturnValue().Set(marshal); } -VOID CALLBACK OnProcessExit( +VOID CALLBACK OnProcessExitWinEvent( _In_ PVOID context, _In_ BOOLEAN TimerOrWaitFired) { - // Nan::HandleScope scope; - pty_baton *baton = static_cast(context); + pty_handle *baton = static_cast(context); + // Unregister wait handler + // UnregisterWait(baton->hWait); - // v8::Handle args[1] = { Nan::New("test2").ToLocalChecked() }; - v8::Handle local = Nan::New(baton->cb); - local->Call(Nan::GetCurrentContext()->Global(), 0, 0); - - - - - // v8::Handle args[1] = { Nan::New("test2").ToLocalChecked() }; - // v8::Handle args[1]; - - // v8::Handle local = Nan::New(baton->cb); - // // TODO: Reset baton cb - // local->Call(Nan::GetCurrentContext()->Global(), 0, 0); - // MessageBox(0, "The process has exited.", "INFO", MB_OK); + // Fire OnProcessExit + uv_async_send(&baton->async); +} +static void OnProcessExit(uv_async_t *async) { + Nan::HandleScope scope; + pty_handle *baton = static_cast(async->data); + // Get exit code + DWORD exitCode = 0; + GetExitCodeProcess(baton->hShell, &exitCode); + // Call function + v8::Handle args[1] = { + Nan::New(exitCode) + }; + v8::Handle local = Nan::New(baton->cb); + local->Call(Nan::GetCurrentContext()->Global(), 1, args); - // v8::Handle contextObject = v8::Handle::Cast(*static_cast*>(context)); - // //*static_cast*>(context); - // v8::Handle funcValue = contextObject->Get(Nan::New("_$onProcessExit").ToLocalChecked()); - // v8::Handle func = v8::Handle::Cast(funcValue); - // v8::Handle args[1]; - // func->Call(contextObject, 0, args); - // TODO: UnregisterWait + // Clean up + baton->cb.Reset(); } static NAN_METHOD(PtyConnect) { @@ -331,7 +330,7 @@ static NAN_METHOD(PtyConnect) { LPWSTR envArg = envV.empty() ? nullptr : envV.data(); // Fetch pty handle from ID and start process - const pty_handle* handle = get_pty_handle(id); + pty_handle* handle = get_pty_handle(id); BOOL success = ConnectNamedPipe(handle->hIn, nullptr); success = ConnectNamedPipe(handle->hOut, nullptr); @@ -376,54 +375,20 @@ static NAN_METHOD(PtyConnect) { return throwNanError(&info, "Cannot create process", true); } - // PHANDLE hNewHandle; - // RegisterWaitForSingleObject(hNewHandle, piClient.hProcess, OnProcessExit, NULL, INFINITE, WT_EXECUTEONLYONCE); - - // Nan::MakeCallback(info.This(), "_$onProcessExit", 0, NULL); - // v8::Handle context = info.This(); - // auto lambda = [context]() { - // v8::Handle value = context->Get(Nan::New("_$onProcessExit").ToLocalChecked()); - // // if (value->IsFunction()) { - // v8::Handle func = v8::Handle::Cast(value); - // v8::Handle args[1]; - // func->Call(context, 0, args); - // }; - // lambda(); - // MessageBox(0, "CALLED", "INFO", MB_OK); - // } - - - // auto lambda = [context]( - // _In_ PVOID lpParameter, - // _In_ BOOLEAN TimerOrWaitFired) { - // v8::Handle value = context->Get(Nan::New("_$onProcessExit").ToLocalChecked()); - // // if (value->IsFunction()) { - // v8::Handle func = v8::Handle::Cast(value); - // v8::Handle args[1]; - // args[0] = Nan::New("test"); - // func->Call(context, 1, args); - // }; - - pty_baton *baton = new pty_baton(); - - baton->cb.Reset(exitCallback); - - // v8::Handle args[1] = { Nan::New("test2").ToLocalChecked() }; - // v8::Handle local = Nan::New(cb); - // local->Call(Nan::GetCurrentContext()->Global(), 1, args); - - - // pty_baton *b = static_cast((PVOID)baton); - // v8::Handle args[1] = { Nan::New("test2").ToLocalChecked() }; - // v8::Handle local = Nan::New(b->cb); - // local->Call(Nan::GetCurrentContext()->Global(), 0, 0); - - // OnProcessExit((PVOID)baton, 0); + // Update handle + handle->hShell = piClient.hProcess; + handle->cb.Reset(exitCallback); + handle->async.data = handle; + // Setup OnProcessExit callback + uv_async_init(uv_default_loop(), &handle->async, OnProcessExit); - PHANDLE hNewHandle; - RegisterWaitForSingleObject(hNewHandle, piClient.hProcess, OnProcessExit, (PVOID)baton, INFINITE, WT_EXECUTEONLYONCE); + // Setup Windows wait for process exit event + PHANDLE hWait; + RegisterWaitForSingleObject(hWait, piClient.hProcess, OnProcessExitWinEvent, (PVOID)handle, INFINITE, WT_EXECUTEONLYONCE); + handle->hWait = hWait; + // Return v8::Local marshal = Nan::New(); marshal->Set(Nan::New("pid").ToLocalChecked(), Nan::New(piClient.dwProcessId)); info.GetReturnValue().Set(marshal); diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts index ac19a5aa2..d17064ac7 100644 --- a/src/windowsPtyAgent.ts +++ b/src/windowsPtyAgent.ts @@ -13,6 +13,13 @@ import { loadNative } from './utils'; let conptyNative: any; let winptyNative: any; +/** + * The amount of time to wait for additional data after the conpty shell process has exited before + * shutting down the socket. The timer will be reset if a new data event comes in after the timer + * has started. + */ +const FLUSH_DATA_INTERVAL = 20; + /** * Agent. Internal class. * @@ -27,6 +34,8 @@ export class WindowsPtyAgent { private _pid: number; private _innerPid: number; private _innerPidHandle: number; + private _closeTimeout: NodeJS.Timer; + private _exitCode: number | undefined; private _fd: any; private _pty: number; @@ -38,6 +47,7 @@ export class WindowsPtyAgent { public get innerPid(): number { return this._innerPid; } public get pty(): number { return this._pty; } + constructor( file: string, args: ArgvOrCommandLine, @@ -112,8 +122,15 @@ export class WindowsPtyAgent { } public resize(cols: number, rows: number): void { + if (this._useConpty) { + if (this._exitCode !== undefined) { + throw new Error('Cannot resize a pty that has already exited'); + } + this._ptyNative.resize(this._pty, cols, rows); + return; + } // TODO: Guard against invalid pty values - this._ptyNative.resize(this._useConpty ? this._pty : this._pid, cols, rows); + this._ptyNative.resize(this._pid, cols, rows); } public kill(): void { @@ -142,8 +159,10 @@ export class WindowsPtyAgent { } } - public getExitCode(): number { - // TODO: Fix for conpty + public get exitCode(): number { + if (this._useConpty) { + return this._exitCode; + } return this._ptyNative.getExitCode(this._innerPidHandle); } @@ -163,11 +182,27 @@ export class WindowsPtyAgent { /** * Triggered from the native side when a contpy process exits. */ - private _$onProcessExit(arg: string): void { - console.log('_$onProcessExit ' + arg); - console.log('this'); - console.log(this); - require('fs').writeFileSync('C:\\Users\\daimms.REDMOND\\Documents\\dev\\Tyriar\\node-pty\\out.log', arg); + private _$onProcessExit(exitCode: number): void { + this._exitCode = exitCode; + console.log('_$onProcessExit, ' + exitCode); + this._flushDataAndCleanUp(); + this._outSocket.on('data', () => this._flushDataAndCleanUp()); + } + + private _flushDataAndCleanUp(): void { + if (this._closeTimeout) { + clearTimeout(this._closeTimeout); + } + this._closeTimeout = setTimeout(() => this._cleanUpProcess(), FLUSH_DATA_INTERVAL); + } + + private _cleanUpProcess(): void { + this._inSocket.readable = false; + this._inSocket.writable = false; + this._outSocket.readable = false; + this._outSocket.writable = false; + // this._outSocket.emit('exit', exitCode); + this._outSocket.destroy(); } } diff --git a/src/windowsTerminal.test.ts b/src/windowsTerminal.test.ts index 3b72c08f0..dc9495e5a 100644 --- a/src/windowsTerminal.test.ts +++ b/src/windowsTerminal.test.ts @@ -28,9 +28,11 @@ if (process.platform === 'win32') { it('should throw an non-native exception when resizing a killed terminal', (done) => { const term = new WindowsTerminal('cmd.exe', [], {}); (term)._defer(() => { + term.on('exit', () => { + assert.throws(() => term.resize(1, 1)); + done(); + }); term.destroy(); - assert.throws(() => term.resize(1, 1)); - done(); }); }); }); @@ -56,6 +58,7 @@ if (process.platform === 'win32') { result += data; }); term.on('exit', () => { + console.log('result: ' + result); assert.ok(result.indexOf('hello world') >= 1); done(); }); diff --git a/src/windowsTerminal.ts b/src/windowsTerminal.ts index 2b25ab3be..4084925cd 100644 --- a/src/windowsTerminal.ts +++ b/src/windowsTerminal.ts @@ -3,9 +3,7 @@ * Copyright (c) 2016, Daniel Imms (MIT License). */ -import * as path from 'path'; import { Socket } from 'net'; -import { inherits } from 'util'; import { Terminal, DEFAULT_COLS, DEFAULT_ROWS } from './terminal'; import { WindowsPtyAgent } from './windowsPtyAgent'; import { IPtyForkOptions, IPtyOpenOptions } from './interfaces'; @@ -107,7 +105,7 @@ export class WindowsTerminal extends Terminal { // Cleanup after the socket is closed. this._socket.on('close', () => { - this.emit('exit', this._agent.getExitCode()); + this.emit('exit', this._agent.exitCode); this._close(); }); From 772de9cd59a55d187c3101ed323b7c8fba312b4f Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 20 Dec 2018 22:06:27 -0800 Subject: [PATCH 43/55] Types wip --- src/native.d.ts | 18 ++++++++++++++++++ src/windowsPtyAgent.ts | 23 ++++++++--------------- 2 files changed, 26 insertions(+), 15 deletions(-) create mode 100644 src/native.d.ts diff --git a/src/native.d.ts b/src/native.d.ts new file mode 100644 index 000000000..eec532b66 --- /dev/null +++ b/src/native.d.ts @@ -0,0 +1,18 @@ +/** + * Copyright (c) 2018, Microsoft (MIT License). + */ + +interface IConptyNative { + startProcess(file: string, cols: number, rows: number, debug: boolean, pipeName: string): IConptyPty; +} + +interface IWinptyNative { + startProcess(file, commandLine, env, cwd, cols, rows, debug): IConptyPty; +} + +interface IConptyPty { + pty: number; + fd: number; + conin: string; + conout: string; +} diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts index d17064ac7..f136e9dfe 100644 --- a/src/windowsPtyAgent.ts +++ b/src/windowsPtyAgent.ts @@ -1,6 +1,7 @@ /** * Copyright (c) 2012-2015, Christopher Jeffrey, Peter Sunde (MIT License) * Copyright (c) 2016, Daniel Imms (MIT License). + * Copyright (c) 2018, Microsoft (MIT License). */ import * as os from 'os'; @@ -10,8 +11,8 @@ import { ArgvOrCommandLine } from './types'; import { loadNative } from './utils'; // TODO: Pull conpty/winpty details into its own interface? -let conptyNative: any; -let winptyNative: any; +let conptyNative: IConptyNative; +let winptyNative: IWinptyNative; /** * The amount of time to wait for additional data after the conpty shell process has exited before @@ -21,13 +22,9 @@ let winptyNative: any; const FLUSH_DATA_INTERVAL = 20; /** - * Agent. Internal class. - * - * Everytime a new pseudo terminal is created it is contained - * within agent.exe. When this process is started there are two - * available named pipes (control and data socket). + * This agent sits between the WindowsTerminal class and provides a common interface for both conpty + * and winpty. */ - export class WindowsPtyAgent { private _inSocket: Socket; private _outSocket: Socket; @@ -39,7 +36,7 @@ export class WindowsPtyAgent { private _fd: any; private _pty: number; - private _ptyNative: any; + private _ptyNative: IConptyNative | IWinptyNative; public get inSocket(): Socket { return this._inSocket; } public get outSocket(): Socket { return this._outSocket; } @@ -47,7 +44,6 @@ export class WindowsPtyAgent { public get innerPid(): number { return this._innerPid; } public get pty(): number { return this._pty; } - constructor( file: string, args: ArgvOrCommandLine, @@ -81,9 +77,9 @@ export class WindowsPtyAgent { // Open pty session. let term; if (this._useConpty) { - term = this._ptyNative.startProcess(file, cols, rows, debug, this._generatePipeName()); + term = (this._ptyNative as IConptyNative).startProcess(file, cols, rows, debug, this._generatePipeName()); } else { - term = this._ptyNative.startProcess(file, commandLine, env, cwd, cols, rows, debug); + term = (this._ptyNative as IWinptyNative).startProcess(file, commandLine, env, cwd, cols, rows, debug); this._pid = term.pid; } @@ -114,9 +110,7 @@ export class WindowsPtyAgent { // TODO: Wait for ready event? if (this._useConpty) { - console.log('this._pty = ' + this._pty); const connect = this._ptyNative.connect(this._pty, commandLine, cwd, env, this._$onProcessExit.bind(this)); - console.log('connect.error' + connect.error); this._innerPid = connect.pid; } } @@ -129,7 +123,6 @@ export class WindowsPtyAgent { this._ptyNative.resize(this._pty, cols, rows); return; } - // TODO: Guard against invalid pty values this._ptyNative.resize(this._pid, cols, rows); } From 1a3ce77d0466572d87077babf650ff39b338d8ff Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 21 Dec 2018 03:53:48 -0800 Subject: [PATCH 44/55] Type Windows native components --- src/native.d.ts | 23 ++++++++++++++++++++--- src/win/conpty.cc | 17 ----------------- src/windowsPtyAgent.ts | 22 ++++++++++------------ src/windowsTerminal.test.ts | 1 - 4 files changed, 30 insertions(+), 33 deletions(-) diff --git a/src/native.d.ts b/src/native.d.ts index eec532b66..ba9183682 100644 --- a/src/native.d.ts +++ b/src/native.d.ts @@ -3,16 +3,33 @@ */ interface IConptyNative { - startProcess(file: string, cols: number, rows: number, debug: boolean, pipeName: string): IConptyPty; + startProcess(file: string, cols: number, rows: number, debug: boolean, pipeName: string): IConptyProcess; + connect(ptyId: number, commandLine: string, cwd: string, env: string[], onProcessExitCallback: (exitCode: number) => void); + resize(ptyId: number, cols: number, rows: number): void; + kill(ptyId: number): void; } interface IWinptyNative { - startProcess(file, commandLine, env, cwd, cols, rows, debug): IConptyPty; + startProcess(file, commandLine, env, cwd, cols, rows, debug): IWinptyProcess; + resize(processHandle: number, cols: number, rows: number): void; + kill(pid: number, innerPidHandle: number): void; + getProcessList(pid: number): number[]; + getExitCode(innerPidHandle: number): number; } -interface IConptyPty { +interface IConptyProcess { pty: number; fd: number; conin: string; conout: string; } + +interface IWinptyProcess { + pty: number; + fd: number; + conin: string; + conout: string; + pid: number; + innerPid: number; + innerPidHandle: number; +} diff --git a/src/win/conpty.cc b/src/win/conpty.cc index b17937e7c..8c5141921 100644 --- a/src/win/conpty.cc +++ b/src/win/conpty.cc @@ -76,21 +76,6 @@ void throwNanError(const Nan::FunctionCallbackInfo* info, const char* (*info).GetReturnValue().SetUndefined(); } -static NAN_METHOD(PtyGetExitCode) { - Nan::HandleScope scope; - - if (info.Length() != 1 || - !info[0]->IsNumber()) { - Nan::ThrowError("Usage: pty.getExitCode(pidHandle)"); - return; - } - - DWORD exitCode = 0; - GetExitCodeProcess((HANDLE)info[0]->IntegerValue(), &exitCode); - - info.GetReturnValue().Set(Nan::New(exitCode)); -} - // Returns a new server named pipe. It has not yet been connected. bool createDataServerPipe(bool write, std::wstring kind, @@ -468,8 +453,6 @@ extern "C" void init(v8::Handle target) { Nan::SetMethod(target, "connect", PtyConnect); Nan::SetMethod(target, "resize", PtyResize); Nan::SetMethod(target, "kill", PtyKill); - Nan::SetMethod(target, "getExitCode", PtyGetExitCode); - // Nan::SetMethod(target, "getProcessList", PtyGetProcessList); }; NODE_MODULE(pty, init); diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts index f136e9dfe..26b673518 100644 --- a/src/windowsPtyAgent.ts +++ b/src/windowsPtyAgent.ts @@ -75,18 +75,17 @@ export class WindowsPtyAgent { const commandLine = argsToCommandLine(file, args); // Open pty session. - let term; + let term: IConptyProcess | IWinptyProcess; if (this._useConpty) { term = (this._ptyNative as IConptyNative).startProcess(file, cols, rows, debug, this._generatePipeName()); } else { term = (this._ptyNative as IWinptyNative).startProcess(file, commandLine, env, cwd, cols, rows, debug); - this._pid = term.pid; + this._pid = (term as IWinptyProcess).pid; + // Terminal pid. + this._innerPid = (term as IWinptyProcess).innerPid; + this._innerPidHandle = (term as IWinptyProcess).innerPidHandle; } - // Terminal pid. - this._innerPid = term.innerPid; - this._innerPidHandle = term.innerPidHandle; - // Not available on windows. this._fd = term.fd; @@ -110,7 +109,7 @@ export class WindowsPtyAgent { // TODO: Wait for ready event? if (this._useConpty) { - const connect = this._ptyNative.connect(this._pty, commandLine, cwd, env, this._$onProcessExit.bind(this)); + const connect = (this._ptyNative as IConptyNative).connect(this._pty, commandLine, cwd, env, this._$onProcessExit.bind(this)); this._innerPid = connect.pid; } } @@ -133,10 +132,10 @@ export class WindowsPtyAgent { this._outSocket.writable = false; // Tell the agent to kill the pty, this releases handles to the process if (this._useConpty) { - this._ptyNative.kill(this._pty); + (this._ptyNative as IConptyNative).kill(this._pty); } else { - const processList: number[] = this._ptyNative.getProcessList(this._pid); - this._ptyNative.kill(this._pid, this._innerPidHandle); + const processList: number[] = (this._ptyNative as IWinptyNative).getProcessList(this._pid); + (this._ptyNative as IWinptyNative).kill(this._pid, this._innerPidHandle); // Since pty.kill will kill most processes by itself and process IDs can be // reused as soon as all handles to them are dropped, we want to immediately // kill the entire console process list. If we do not force kill all @@ -156,7 +155,7 @@ export class WindowsPtyAgent { if (this._useConpty) { return this._exitCode; } - return this._ptyNative.getExitCode(this._innerPidHandle); + return (this._ptyNative as IWinptyNative).getExitCode(this._innerPidHandle); } private _getWindowsBuildNumber(): number { @@ -177,7 +176,6 @@ export class WindowsPtyAgent { */ private _$onProcessExit(exitCode: number): void { this._exitCode = exitCode; - console.log('_$onProcessExit, ' + exitCode); this._flushDataAndCleanUp(); this._outSocket.on('data', () => this._flushDataAndCleanUp()); } diff --git a/src/windowsTerminal.test.ts b/src/windowsTerminal.test.ts index dc9495e5a..7e259f315 100644 --- a/src/windowsTerminal.test.ts +++ b/src/windowsTerminal.test.ts @@ -58,7 +58,6 @@ if (process.platform === 'win32') { result += data; }); term.on('exit', () => { - console.log('result: ' + result); assert.ok(result.indexOf('hello world') >= 1); done(); }); From 70df397110bd93fe2d82e4e167d23b971a55f8b0 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 21 Dec 2018 04:14:44 -0800 Subject: [PATCH 45/55] Clean up --- src/win/conpty.cc | 54 ++++++++++++++---------------------------- src/windowsPtyAgent.ts | 1 - 2 files changed, 18 insertions(+), 37 deletions(-) diff --git a/src/win/conpty.cc b/src/win/conpty.cc index 8c5141921..4580cd6ce 100644 --- a/src/win/conpty.cc +++ b/src/win/conpty.cc @@ -11,8 +11,6 @@ #include #include // PathCombine, PathIsRelative #include -#include -#include #include #include #include @@ -33,7 +31,7 @@ typedef void (*PFNCLOSEPSEUDOCONSOLE)(HPCON hpc); #endif -struct pty_handle { +struct pty_baton { int id; HANDLE hIn; HANDLE hOut; @@ -45,15 +43,15 @@ struct pty_handle { uv_async_t async; uv_thread_t tid; - pty_handle(int _id, HANDLE _hIn, HANDLE _hOut, HPCON _hpc) : id(_id), hIn(_hIn), hOut(_hOut), hpc(_hpc) {}; + pty_baton(int _id, HANDLE _hIn, HANDLE _hOut, HPCON _hpc) : id(_id), hIn(_hIn), hOut(_hOut), hpc(_hpc) {}; }; -static std::vector ptyHandles; +static std::vector ptyHandles; static volatile LONG ptyCounter; -static pty_handle* get_pty_handle(int id) { +static pty_baton* get_pty_baton(int id) { for (size_t i = 0; i < ptyHandles.size(); ++i) { - pty_handle* ptyHandle = ptyHandles[i]; + pty_baton* ptyHandle = ptyHandles[i]; if (ptyHandle->id == id) { return ptyHandle; } @@ -85,7 +83,6 @@ bool createDataServerPipe(bool write, { *hServer = INVALID_HANDLE_VALUE; - // TODO generate unique names for each pipe name = L"\\\\.\\pipe\\" + pipeName + L"-" + kind; const DWORD winOpenMode = PIPE_ACCESS_INBOUND | PIPE_ACCESS_OUTBOUND | FILE_FLAG_FIRST_PIPE_INSTANCE/* | FILE_FLAG_OVERLAPPED */; @@ -203,32 +200,21 @@ static NAN_METHOD(PtyStartProcess) { // Set return values marshal = Nan::New(); - if (SUCCEEDED(hr)) - { + if (SUCCEEDED(hr)) { // We were able to instantiate a conpty const int ptyId = InterlockedIncrement(&ptyCounter); marshal->Set(Nan::New("pty").ToLocalChecked(), Nan::New(ptyId)); - ptyHandles.insert(ptyHandles.end(), new pty_handle(ptyId, hIn, hOut, hpc)); - } - else - { - // TODO: We weren't able to start conpty. Fall back to winpty. + ptyHandles.insert(ptyHandles.end(), new pty_baton(ptyId, hIn, hOut, hpc)); + } else { + Nan::ThrowError("Cannot launch conpty"); + return; } - // TODO: Pull in innerPid, innerPidHandle(?) - // marshal->Set(Nan::New("innerPid").ToLocalChecked(), Nan::New((int)GetProcessId(handle))); - // marshal->Set(Nan::New("innerPidHandle").ToLocalChecked(), Nan::New((int)handle)); - // TODO: Pull in pid - // marshal->Set(Nan::New("pid").ToLocalChecked(), Nan::New((int)winpty_agent_process(pc))); marshal->Set(Nan::New("fd").ToLocalChecked(), Nan::New(-1)); { - // LPCWSTR coninPipeName = winpty_conin_name(pc); - // std::wstring coninPipeNameWStr(coninPipeName); std::string coninPipeNameStr(inName.begin(), inName.end()); marshal->Set(Nan::New("conin").ToLocalChecked(), Nan::New(coninPipeNameStr).ToLocalChecked()); - // LPCWSTR conoutPipeName = winpty_conout_name(pc); - // std::wstring conoutPipeNameWStr(conoutPipeName); std::string conoutPipeNameStr(outName.begin(), outName.end()); marshal->Set(Nan::New("conout").ToLocalChecked(), Nan::New(conoutPipeNameStr).ToLocalChecked()); } @@ -238,10 +224,7 @@ static NAN_METHOD(PtyStartProcess) { VOID CALLBACK OnProcessExitWinEvent( _In_ PVOID context, _In_ BOOLEAN TimerOrWaitFired) { - pty_handle *baton = static_cast(context); - - // Unregister wait handler - // UnregisterWait(baton->hWait); + pty_baton *baton = static_cast(context); // Fire OnProcessExit uv_async_send(&baton->async); @@ -249,7 +232,10 @@ VOID CALLBACK OnProcessExitWinEvent( static void OnProcessExit(uv_async_t *async) { Nan::HandleScope scope; - pty_handle *baton = static_cast(async->data); + pty_baton *baton = static_cast(async->data); + + // TODO: Unregister wait handler + // UnregisterWait(baton->hWait); // Get exit code DWORD exitCode = 0; @@ -315,7 +301,7 @@ static NAN_METHOD(PtyConnect) { LPWSTR envArg = envV.empty() ? nullptr : envV.data(); // Fetch pty handle from ID and start process - pty_handle* handle = get_pty_handle(id); + pty_baton* handle = get_pty_baton(id); BOOL success = ConnectNamedPipe(handle->hIn, nullptr); success = ConnectNamedPipe(handle->hOut, nullptr); @@ -394,7 +380,7 @@ static NAN_METHOD(PtyResize) { SHORT cols = info[1]->Uint32Value(); SHORT rows = info[2]->Uint32Value(); - const pty_handle* handle = get_pty_handle(id); + const pty_baton* handle = get_pty_baton(id); // TODO: Share hLibrary between functions HANDLE hLibrary = LoadLibraryExW(L"kernel32.dll", 0, 0); @@ -421,14 +407,10 @@ static NAN_METHOD(PtyKill) { return; } - // TODO: If the pty is backed by conpty, call ClosePseudoConsole - // (using LoadLibrary/GetProcAddress to find ClosePseudoConsole if it exists) - int id = info[0]->Int32Value(); - const pty_handle* handle = get_pty_handle(id); + const pty_baton* handle = get_pty_baton(id); - // TODO: Share hLibrary between functions HANDLE hLibrary = LoadLibraryExW(L"kernel32.dll", 0, 0); bool fLoadedDll = hLibrary != nullptr; if (fLoadedDll) diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts index 26b673518..79a2eb4d5 100644 --- a/src/windowsPtyAgent.ts +++ b/src/windowsPtyAgent.ts @@ -81,7 +81,6 @@ export class WindowsPtyAgent { } else { term = (this._ptyNative as IWinptyNative).startProcess(file, commandLine, env, cwd, cols, rows, debug); this._pid = (term as IWinptyProcess).pid; - // Terminal pid. this._innerPid = (term as IWinptyProcess).innerPid; this._innerPidHandle = (term as IWinptyProcess).innerPidHandle; } From 0284d4287255dcf712f563dec799d9ede98579c1 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 21 Dec 2018 10:29:03 -0800 Subject: [PATCH 46/55] Call UnregisterWait --- src/win/conpty.cc | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/win/conpty.cc b/src/win/conpty.cc index 4580cd6ce..7e6b5db57 100644 --- a/src/win/conpty.cc +++ b/src/win/conpty.cc @@ -38,7 +38,7 @@ struct pty_baton { HPCON hpc; HANDLE hShell; - PHANDLE hWait; + HANDLE hWait; Nan::Persistent cb; uv_async_t async; uv_thread_t tid; @@ -235,7 +235,7 @@ static void OnProcessExit(uv_async_t *async) { pty_baton *baton = static_cast(async->data); // TODO: Unregister wait handler - // UnregisterWait(baton->hWait); + UnregisterWait(baton->hWait); // Get exit code DWORD exitCode = 0; @@ -355,9 +355,7 @@ static NAN_METHOD(PtyConnect) { uv_async_init(uv_default_loop(), &handle->async, OnProcessExit); // Setup Windows wait for process exit event - PHANDLE hWait; - RegisterWaitForSingleObject(hWait, piClient.hProcess, OnProcessExitWinEvent, (PVOID)handle, INFINITE, WT_EXECUTEONLYONCE); - handle->hWait = hWait; + RegisterWaitForSingleObject(&handle->hWait, piClient.hProcess, OnProcessExitWinEvent, (PVOID)handle, INFINITE, WT_EXECUTEONLYONCE); // Return v8::Local marshal = Nan::New(); From 57dbb1c1e2580c879f5c1f2dc9d4dcaf57529db2 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 21 Dec 2018 10:29:39 -0800 Subject: [PATCH 47/55] Remove TODO --- src/win/conpty.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/win/conpty.cc b/src/win/conpty.cc index 7e6b5db57..144608573 100644 --- a/src/win/conpty.cc +++ b/src/win/conpty.cc @@ -234,7 +234,6 @@ static void OnProcessExit(uv_async_t *async) { Nan::HandleScope scope; pty_baton *baton = static_cast(async->data); - // TODO: Unregister wait handler UnregisterWait(baton->hWait); // Get exit code From dfb319c2fe01830131bbe52a2fa304cef86391e3 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 21 Dec 2018 10:48:24 -0800 Subject: [PATCH 48/55] Fix lint --- src/index.ts | 1 + src/interfaces.ts | 1 + src/native.d.ts | 6 +++--- src/terminal.test.ts | 1 + src/terminal.ts | 1 + src/types.ts | 1 + src/unixTerminal.test.ts | 1 + src/unixTerminal.ts | 1 + src/utils.ts | 1 + src/win/conpty.cc | 1 + src/win/path_util.cc | 1 + src/win/path_util.h | 1 + src/win/winpty.cc | 1 + src/windowsPtyAgent.test.ts | 1 + src/windowsPtyAgent.ts | 4 +--- src/windowsTerminal.test.ts | 1 + src/windowsTerminal.ts | 1 + 17 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/index.ts b/src/index.ts index 77c3172d3..ea9ddda77 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,6 +1,7 @@ /** * Copyright (c) 2012-2015, Christopher Jeffrey, Peter Sunde (MIT License) * Copyright (c) 2016, Daniel Imms (MIT License). + * Copyright (c) 2018, Microsoft Corporation (MIT License). */ import * as path from 'path'; diff --git a/src/interfaces.ts b/src/interfaces.ts index cbbb744ae..d2ffb8844 100644 --- a/src/interfaces.ts +++ b/src/interfaces.ts @@ -1,5 +1,6 @@ /** * Copyright (c) 2016, Daniel Imms (MIT License). + * Copyright (c) 2018, Microsoft Corporation (MIT License). */ import * as net from 'net'; diff --git a/src/native.d.ts b/src/native.d.ts index ba9183682..ed83620b0 100644 --- a/src/native.d.ts +++ b/src/native.d.ts @@ -1,16 +1,16 @@ /** - * Copyright (c) 2018, Microsoft (MIT License). + * Copyright (c) 2018, Microsoft Corporation (MIT License). */ interface IConptyNative { startProcess(file: string, cols: number, rows: number, debug: boolean, pipeName: string): IConptyProcess; - connect(ptyId: number, commandLine: string, cwd: string, env: string[], onProcessExitCallback: (exitCode: number) => void); + connect(ptyId: number, commandLine: string, cwd: string, env: string[], onProcessExitCallback: (exitCode: number) => void): { pid: number }; resize(ptyId: number, cols: number, rows: number): void; kill(ptyId: number): void; } interface IWinptyNative { - startProcess(file, commandLine, env, cwd, cols, rows, debug): IWinptyProcess; + startProcess(file: string, commandLine: string, env: string[], cwd: string, cols: number, rows: number, debug: boolean): IWinptyProcess; resize(processHandle: number, cols: number, rows: number): void; kill(pid: number, innerPidHandle: number): void; getProcessList(pid: number): number[]; diff --git a/src/terminal.test.ts b/src/terminal.test.ts index e3aa46da4..1827c4d2c 100644 --- a/src/terminal.test.ts +++ b/src/terminal.test.ts @@ -1,5 +1,6 @@ /** * Copyright (c) 2017, Daniel Imms (MIT License). + * Copyright (c) 2018, Microsoft Corporation (MIT License). */ import * as assert from 'assert'; diff --git a/src/terminal.ts b/src/terminal.ts index f68db7230..ae3349145 100644 --- a/src/terminal.ts +++ b/src/terminal.ts @@ -1,6 +1,7 @@ /** * Copyright (c) 2012-2015, Christopher Jeffrey (MIT License) * Copyright (c) 2016, Daniel Imms (MIT License). + * Copyright (c) 2018, Microsoft Corporation (MIT License). */ import * as path from 'path'; diff --git a/src/types.ts b/src/types.ts index cb20756b9..e249a05d0 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1,5 +1,6 @@ /** * Copyright (c) 2017, Daniel Imms (MIT License). + * Copyright (c) 2018, Microsoft Corporation (MIT License). */ export type ArgvOrCommandLine = string[] | string; diff --git a/src/unixTerminal.test.ts b/src/unixTerminal.test.ts index d0c9b6d57..2b4110f33 100644 --- a/src/unixTerminal.test.ts +++ b/src/unixTerminal.test.ts @@ -1,5 +1,6 @@ /** * Copyright (c) 2017, Daniel Imms (MIT License). + * Copyright (c) 2018, Microsoft Corporation (MIT License). */ import { UnixTerminal } from './unixTerminal'; diff --git a/src/unixTerminal.ts b/src/unixTerminal.ts index 8fba06b0a..5446b61f3 100644 --- a/src/unixTerminal.ts +++ b/src/unixTerminal.ts @@ -1,6 +1,7 @@ /** * Copyright (c) 2012-2015, Christopher Jeffrey (MIT License) * Copyright (c) 2016, Daniel Imms (MIT License). + * Copyright (c) 2018, Microsoft Corporation (MIT License). */ import * as net from 'net'; diff --git a/src/utils.ts b/src/utils.ts index 0d62c3c47..4d28e47bb 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1,5 +1,6 @@ /** * Copyright (c) 2017, Daniel Imms (MIT License). + * Copyright (c) 2018, Microsoft Corporation (MIT License). */ import * as path from 'path'; diff --git a/src/win/conpty.cc b/src/win/conpty.cc index 144608573..de9538b4a 100644 --- a/src/win/conpty.cc +++ b/src/win/conpty.cc @@ -1,6 +1,7 @@ /** * Copyright (c) 2013-2015, Christopher Jeffrey, Peter Sunde (MIT License) * Copyright (c) 2016, Daniel Imms (MIT License). + * Copyright (c) 2018, Microsoft Corporation (MIT License). * * pty.cc: * This file is responsible for starting processes diff --git a/src/win/path_util.cc b/src/win/path_util.cc index 684fd19d6..4387d2550 100644 --- a/src/win/path_util.cc +++ b/src/win/path_util.cc @@ -1,6 +1,7 @@ /** * Copyright (c) 2013-2015, Christopher Jeffrey, Peter Sunde (MIT License) * Copyright (c) 2016, Daniel Imms (MIT License). + * Copyright (c) 2018, Microsoft Corporation (MIT License). */ #include diff --git a/src/win/path_util.h b/src/win/path_util.h index ed196445b..2bfcabf19 100644 --- a/src/win/path_util.h +++ b/src/win/path_util.h @@ -1,6 +1,7 @@ /** * Copyright (c) 2013-2015, Christopher Jeffrey, Peter Sunde (MIT License) * Copyright (c) 2016, Daniel Imms (MIT License). + * Copyright (c) 2018, Microsoft Corporation (MIT License). */ #ifndef NODE_PTY_PATH_UTIL_H_ diff --git a/src/win/winpty.cc b/src/win/winpty.cc index 77af9e423..1b5e6b350 100644 --- a/src/win/winpty.cc +++ b/src/win/winpty.cc @@ -1,6 +1,7 @@ /** * Copyright (c) 2013-2015, Christopher Jeffrey, Peter Sunde (MIT License) * Copyright (c) 2016, Daniel Imms (MIT License). + * Copyright (c) 2018, Microsoft Corporation (MIT License). * * pty.cc: * This file is responsible for starting processes diff --git a/src/windowsPtyAgent.test.ts b/src/windowsPtyAgent.test.ts index a4a065768..af2893336 100644 --- a/src/windowsPtyAgent.test.ts +++ b/src/windowsPtyAgent.test.ts @@ -1,5 +1,6 @@ /** * Copyright (c) 2017, Daniel Imms (MIT License). + * Copyright (c) 2018, Microsoft Corporation (MIT License). */ import * as assert from 'assert'; diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts index 79a2eb4d5..b6f33a488 100644 --- a/src/windowsPtyAgent.ts +++ b/src/windowsPtyAgent.ts @@ -1,7 +1,7 @@ /** * Copyright (c) 2012-2015, Christopher Jeffrey, Peter Sunde (MIT License) * Copyright (c) 2016, Daniel Imms (MIT License). - * Copyright (c) 2018, Microsoft (MIT License). + * Copyright (c) 2018, Microsoft Corporation (MIT License). */ import * as os from 'os'; @@ -10,7 +10,6 @@ import { Socket } from 'net'; import { ArgvOrCommandLine } from './types'; import { loadNative } from './utils'; -// TODO: Pull conpty/winpty details into its own interface? let conptyNative: IConptyNative; let winptyNative: IWinptyNative; @@ -191,7 +190,6 @@ export class WindowsPtyAgent { this._inSocket.writable = false; this._outSocket.readable = false; this._outSocket.writable = false; - // this._outSocket.emit('exit', exitCode); this._outSocket.destroy(); } } diff --git a/src/windowsTerminal.test.ts b/src/windowsTerminal.test.ts index 7e259f315..49f1da918 100644 --- a/src/windowsTerminal.test.ts +++ b/src/windowsTerminal.test.ts @@ -1,5 +1,6 @@ /** * Copyright (c) 2017, Daniel Imms (MIT License). + * Copyright (c) 2018, Microsoft Corporation (MIT License). */ import * as fs from 'fs'; diff --git a/src/windowsTerminal.ts b/src/windowsTerminal.ts index 4084925cd..218b3bc7d 100644 --- a/src/windowsTerminal.ts +++ b/src/windowsTerminal.ts @@ -1,6 +1,7 @@ /** * Copyright (c) 2012-2015, Christopher Jeffrey, Peter Sunde (MIT License) * Copyright (c) 2016, Daniel Imms (MIT License). + * Copyright (c) 2018, Microsoft Corporation (MIT License). */ import { Socket } from 'net'; From 4281739161698c9083c72a16d8678fe1b1776c2d Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 21 Dec 2018 11:03:02 -0800 Subject: [PATCH 49/55] Mention conpty in readme --- README.md | 2 +- src/win/conpty.cc | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/README.md b/README.md index 04e60c954..73db1afe9 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ This is useful for: - Writing a terminal emulator (eg. via [xterm.js](https://github.com/sourcelair/xterm.js)). - Getting certain programs to *think* you're a terminal, such as when you need a program to send you control sequences. -`node-pty` supports Linux, macOS and Windows. Windows support is possible by utilizing the [winpty](https://github.com/rprichard/winpty) library. +`node-pty` supports Linux, macOS and Windows. Windows support is possible by utilizing the [Windows conpty API](https://blogs.msdn.microsoft.com/commandline/2018/08/02/windows-command-line-introducing-the-windows-pseudo-console-conpty/) on Windows 1809+ and the [winpty](https://github.com/rprichard/winpty) library in older version. ## Real-world Uses diff --git a/src/win/conpty.cc b/src/win/conpty.cc index de9538b4a..2702bec6b 100644 --- a/src/win/conpty.cc +++ b/src/win/conpty.cc @@ -380,7 +380,6 @@ static NAN_METHOD(PtyResize) { const pty_baton* handle = get_pty_baton(id); - // TODO: Share hLibrary between functions HANDLE hLibrary = LoadLibraryExW(L"kernel32.dll", 0, 0); bool fLoadedDll = hLibrary != nullptr; if (fLoadedDll) From d4c69c388809d2b5d32faec843b8f8c4d637c8a4 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 21 Dec 2018 11:19:17 -0800 Subject: [PATCH 50/55] Add unix native typings --- src/native.d.ts | 21 ++++++++++++++++++++- src/unixTerminal.ts | 10 ++-------- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/src/native.d.ts b/src/native.d.ts index ed83620b0..22cde78c2 100644 --- a/src/native.d.ts +++ b/src/native.d.ts @@ -4,7 +4,7 @@ interface IConptyNative { startProcess(file: string, cols: number, rows: number, debug: boolean, pipeName: string): IConptyProcess; - connect(ptyId: number, commandLine: string, cwd: string, env: string[], onProcessExitCallback: (exitCode: number) => void): { pid: number }; + connect(ptyId: number, commandLine: string, cwd: string, env: string[], onExitCallback: (exitCode: number) => void): { pid: number }; resize(ptyId: number, cols: number, rows: number): void; kill(ptyId: number): void; } @@ -17,6 +17,13 @@ interface IWinptyNative { getExitCode(innerPidHandle: number): number; } +interface IUnixNative { + fork(file: string, args: string[], parsedEnv: string[], cwd: string, cols: number, rows: number, uid: number, gid: number, useUtf8: boolean, onExitCallback: (code: number, signal: number) => void): IUnixProcess; + open(cols: number, rows: number): IUnixOpenProcess; + process(fd: number, pty: string): string; + resize(fd: number, cols: number, rows: number): void; +} + interface IConptyProcess { pty: number; fd: number; @@ -33,3 +40,15 @@ interface IWinptyProcess { innerPid: number; innerPidHandle: number; } + +interface IUnixProcess { + fd: number; + pid: number; + pty: string; +} + +interface IUnixOpenProcess { + master: number; + slave: number; + pty: string; +} diff --git a/src/unixTerminal.ts b/src/unixTerminal.ts index 5446b61f3..ae791ad05 100644 --- a/src/unixTerminal.ts +++ b/src/unixTerminal.ts @@ -10,13 +10,7 @@ import { IProcessEnv, IPtyForkOptions, IPtyOpenOptions } from './interfaces'; import { ArgvOrCommandLine } from './types'; import { assign, loadNative } from './utils'; -declare interface INativePty { - master: number; - slave: number; - pty: string; -} - -const pty = loadNative('pty'); +const pty: IUnixNative = loadNative('pty'); const DEFAULT_FILE = 'sh'; const DEFAULT_NAME = 'xterm'; @@ -178,7 +172,7 @@ export class UnixTerminal extends Terminal { const encoding = opt.encoding ? 'utf8' : opt.encoding; // open - const term: INativePty = pty.open(cols, rows); + const term: IUnixOpenProcess = pty.open(cols, rows); self._master = new PipeSocket(term.master); self._master.setEncoding(encoding); From 2d3c8de77dfa6cebfd5adad144b6227423226b68 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 21 Dec 2018 11:22:42 -0800 Subject: [PATCH 51/55] Fix license --- LICENSE | 28 +++++++++++++++++++++++++++- README.md | 3 ++- package.json | 2 +- 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/LICENSE b/LICENSE index 37131058c..22f780daf 100644 --- a/LICENSE +++ b/LICENSE @@ -40,4 +40,30 @@ FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -SOFTWARE. \ No newline at end of file +SOFTWARE. + + + +MIT License + +Copyright (c) 2018 - present Microsoft Corporation + +All rights reserved. + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/README.md b/README.md index 73db1afe9..d98693b85 100644 --- a/README.md +++ b/README.md @@ -96,4 +96,5 @@ This project is forked from [chjj/pty.js](https://github.com/chjj/pty.js) with t ## License Copyright (c) 2012-2015, Christopher Jeffrey (MIT License).
-Copyright (c) 2016, Daniel Imms (MIT License). +Copyright (c) 2016, Daniel Imms (MIT License).
+Copyright (c) 2018, Microsoft Corporation (MIT License). diff --git a/package.json b/package.json index 4a95d429a..e6b898fe5 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "name": "node-pty", "description": "Fork pseudoterminals in Node.JS", "author": { - "name": "Daniel Imms" + "name": "Microsoft Corporation" }, "version": "0.7.8", "license": "MIT", From 7125d380d66825a47dc32e617a28ba5fcf9800d5 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 21 Dec 2018 11:29:55 -0800 Subject: [PATCH 52/55] Elaborate on experimentalUseConpty setting --- src/interfaces.ts | 2 +- typings/node-pty.d.ts | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/interfaces.ts b/src/interfaces.ts index d2ffb8844..cd5774837 100644 --- a/src/interfaces.ts +++ b/src/interfaces.ts @@ -116,7 +116,7 @@ export interface IPtyForkOptions { uid?: number; gid?: number; encoding?: string; - experimentalUseConpty?: boolean; + experimentalUseConpty?: boolean | undefined; } export interface IPtyOpenOptions { diff --git a/typings/node-pty.d.ts b/typings/node-pty.d.ts index 822a07444..30aa14b70 100644 --- a/typings/node-pty.d.ts +++ b/typings/node-pty.d.ts @@ -1,5 +1,6 @@ /** * Copyright (c) 2017, Daniel Imms (MIT License). + * Copyright (c) 2018, Microsoft Corporation (MIT License). */ declare module 'node-pty' { @@ -26,8 +27,10 @@ declare module 'node-pty' { gid?: number; encoding?: string; /** - * Whether to use the experimental ConPTY system on Windows. This setting will be ignored on - * non-Windows. + * Whether to use the experimental ConPTY system on Windows. When this is not set, ConPTY will + * be used when the Windows build number is >= 17692. + * + * This setting does nothing on non-Windows. */ experimentalUseConpty?: boolean; } From 6dcdf5eb4584db25a0c41192cf93570a01dec949 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 21 Dec 2018 11:33:22 -0800 Subject: [PATCH 53/55] Ignore vscode workspace settings --- .gitignore | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 76fab2175..bf051c8f2 100644 --- a/.gitignore +++ b/.gitignore @@ -9,4 +9,5 @@ builderror.log lib/ npm-debug.log package-lock.json -fixtures/space folder/ \ No newline at end of file +fixtures/space folder/ +.vscode/settings.json From 6849180e470d23ee8db2b1ef14ecc3e0bc179aa7 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 21 Dec 2018 11:38:18 -0800 Subject: [PATCH 54/55] Use stricter tsc options --- src/windowsTerminal.ts | 2 +- tsconfig.json | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/windowsTerminal.ts b/src/windowsTerminal.ts index 218b3bc7d..57c4b5327 100644 --- a/src/windowsTerminal.ts +++ b/src/windowsTerminal.ts @@ -60,7 +60,7 @@ export class WindowsTerminal extends Terminal { // These events needs to be forwarded. ['connect', 'data', 'end', 'timeout', 'drain'].forEach(event => { - this._socket.on(event, data => { + this._socket.on(event, () => { // Wait until the first data event is fired then we can run deferreds. if (!this._isReady && event === 'data') { diff --git a/tsconfig.json b/tsconfig.json index a7a2c45cd..172c415e2 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -7,7 +7,10 @@ "sourceMap": true, "lib": [ "es5" - ] + ], + "alwaysStrict": true, + "noImplicitAny": true, + "noImplicitThis": true }, "exclude": [ "node_modules", From 44640eb9206eceffecde1f3e14ffe054974155a1 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 21 Dec 2018 12:03:02 -0800 Subject: [PATCH 55/55] Only allow conpty to be used on 17692+, even when set to true --- src/windowsPtyAgent.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts index b6f33a488..bc748823a 100644 --- a/src/windowsPtyAgent.ts +++ b/src/windowsPtyAgent.ts @@ -53,7 +53,7 @@ export class WindowsPtyAgent { debug: boolean, private _useConpty: boolean | undefined ) { - if (this._useConpty === undefined) { + if (this._useConpty === undefined || this._useConpty === true) { this._useConpty = this._getWindowsBuildNumber() >= 17692; } if (this._useConpty) {