From 880c00c69eeca73643ddb576f02c5badbec81f56 Mon Sep 17 00:00:00 2001 From: Ryan Prichard Date: Wed, 25 May 2016 19:49:45 -0500 Subject: [PATCH] Replace the libwinpty API. The new API improves upon the old API in a number of ways: * The old API provided a single data pipe for input and output, and it provided the pipe in the form of a HANDLE opened with FILE_FLAG_OVERLAPPED. Using a bare HANDLE is difficult in higher-level languages, and overlapped/asynchronous I/O is hard to get right. winpty_close closed the data pipe, which also didn't help things, though I think the handle could have been duplicated. Using a single handle for input and output complicates shutdown. When the child process exits, the agent scrapes the final output, then closes the data pipe once its written. It's possible that a winpty client will first detect the closed pipe when it's writing *input* rather than reading output, which seems wrong. (On the other hand, the agent doesn't implement "backpressure" for either input or output (yet), and if it did, it's possible that post-exit shutdown should interrupt a blocked write into the console input queue. I need to think about it more.) The new API splits the data pipe into CONIN and CONOUT pipes, which are accessed by filename. After `winpty_open` returns, the client queries pipe names using `winpty_conin_name` and `winpty_conout_name`, then opens them using any mechanism, low-level or high-level, blocking or overlapped. * The old libwinpty handled errors by killing the process. The new libwinpty uses C++ exceptions internally and translates exceptions at API boundaries using: - a boolean error result (e.g. BOOL, NULL-vs-non-NULL) - a potentially heap-allocated winpty_error_t object returned via an optional winpty_error_ptr_t parameter. That parameter can be NULL. If it isn't, then an error code and message can be queried from the error object. The winpty_error_t object must be freed with winpty_error_free. * winpty_start_process is renamed to winpty_spawn. winpty_open and winpty_spawn accept a "config" object which holds parameters. New configuration options can be added without disturbing the source or binary API. * The winpty_get_exit_code and winpty_get_process_id APIs are removed. The winpty_spawn function has an out parameter providing the child's process and thread HANDLEs (duplicated from the agent via DuplicateHandle). The winpty client can call GetExitCodeProcess and GetProcessId (as well as the WaitForXXX APIs to wait for child exit). --- Makefile | 1 + RELEASES.md | 4 + src/agent/Agent.cc | 240 ++++-- src/agent/Agent.h | 20 +- src/agent/NamedPipe.cc | 189 ++++- src/agent/NamedPipe.h | 44 +- src/agent/main.cc | 11 +- src/agent/subdir.mk | 3 + src/include/winpty.h | 217 +++-- src/include/winpty_constants.h | 75 ++ src/libwinpty/AgentLocation.cc | 8 +- .../LibWinptyException.h} | 45 +- src/libwinpty/WinptyInternal.h | 69 ++ src/libwinpty/winpty.cc | 803 ++++++++++++------ src/{unix-adapter/Event.h => shared/Mutex.h} | 60 +- src/tests/trivial_test.cc | 53 +- src/unix-adapter/InputHandler.cc | 32 +- src/unix-adapter/InputHandler.h | 9 +- src/unix-adapter/OutputHandler.cc | 42 +- src/unix-adapter/OutputHandler.h | 10 +- src/unix-adapter/main.cc | 118 ++- src/winpty.gyp | 9 +- 22 files changed, 1431 insertions(+), 631 deletions(-) create mode 100755 src/include/winpty_constants.h rename src/{unix-adapter/DualWakeup.h => libwinpty/LibWinptyException.h} (57%) mode change 100644 => 100755 create mode 100755 src/libwinpty/WinptyInternal.h rename src/{unix-adapter/Event.h => shared/Mutex.h} (52%) mode change 100644 => 100755 diff --git a/Makefile b/Makefile index 921c3c85..c5da675e 100644 --- a/Makefile +++ b/Makefile @@ -131,6 +131,7 @@ install-doc : install-include : mkdir -p $(PREFIX)/include/winpty install -m 644 -p src/include/winpty.h $(PREFIX)/include/winpty + install -m 644 -p src/include/winpty_constants.h $(PREFIX)/include/winpty .PHONY : install install : \ diff --git a/RELEASES.md b/RELEASES.md index 4a9f3a9b..f5b4b4e4 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -1,3 +1,7 @@ +# Next Version + +The winpty library has a new API that should be easier for embedding. + # Version 0.3.0 (2016-05-20) User-visible changes: diff --git a/src/agent/Agent.cc b/src/agent/Agent.cc index 4da726ef..8e34bdc7 100644 --- a/src/agent/Agent.cc +++ b/src/agent/Agent.cc @@ -19,26 +19,35 @@ // IN THE SOFTWARE. #include "Agent.h" -#include "Win32Console.h" -#include "ConsoleInput.h" -#include "Terminal.h" -#include "NamedPipe.h" -#include "ConsoleFont.h" -#include "../shared/DebugClient.h" -#include "../shared/AgentMsg.h" -#include "../shared/Buffer.h" -#include "../shared/winpty_snprintf.h" -#include "../shared/WinptyAssert.h" -#include "../shared/StringUtil.h" -#include "../shared/WindowsVersion.h" + +#include + +#include #include #include #include -#include -#include -#include + #include #include +#include + +#include "../include/winpty_constants.h" + +#include "../shared/AgentMsg.h" +#include "../shared/Buffer.h" +#include "../shared/DebugClient.h" +#include "../shared/GenRandom.h" +#include "../shared/StringBuilder.h" +#include "../shared/StringUtil.h" +#include "../shared/WindowsVersion.h" +#include "../shared/WinptyAssert.h" +#include "../shared/winpty_snprintf.h" + +#include "ConsoleFont.h" +#include "ConsoleInput.h" +#include "NamedPipe.h" +#include "Terminal.h" +#include "Win32Console.h" const int SC_CONSOLE_MARK = 0xFFF2; const int SC_CONSOLE_SELECT_ALL = 0xFFF5; @@ -111,10 +120,35 @@ static bool detectWhetherMarkMovesCursor(Win32Console &console) return ret; } +static inline WriteBuffer newPacket() { + WriteBuffer packet; + packet.putRawValue(0); // Reserve space for size. + return packet; +} + +static HANDLE duplicateHandle(HANDLE h) { + HANDLE ret = nullptr; + if (!DuplicateHandle( + GetCurrentProcess(), h, + GetCurrentProcess(), &ret, + 0, FALSE, DUPLICATE_SAME_ACCESS)) { + ASSERT(false && "DuplicateHandle failed!"); + } + return ret; +} + +// It's safe to truncate a handle from 64-bits to 32-bits, or to sign-extend it +// back to 64-bits. See the MSDN article, "Interprocess Communication Between +// 32-bit and 64-bit Applications". +// https://msdn.microsoft.com/en-us/library/windows/desktop/aa384203.aspx +static int64_t int64FromHandle(HANDLE h) { + return static_cast(reinterpret_cast(h)); +} + } // anonymous namespace Agent::Agent(LPCWSTR controlPipeName, - LPCWSTR dataPipeName, + uint64_t agentFlags, int initialCols, int initialRows) : m_ptySize(initialCols, initialRows) @@ -146,9 +180,17 @@ Agent::Agent(LPCWSTR controlPipeName, m_console->setTextAttribute(7); m_console->clearAllLines(m_console->bufferInfo()); - m_controlPipe = &makePipe(controlPipeName); - m_dataPipe = &makePipe(dataPipeName); - m_terminal.reset(new Terminal(*m_dataPipe)); + m_controlPipe = &connectToControlPipe(controlPipeName); + m_coninPipe = &createDataServerPipe(false, L"conin"); + m_conoutPipe = &createDataServerPipe(true, L"conout"); + + // Send an initial response packet to winpty.dll containing pipe names. + auto setupPacket = newPacket(); + setupPacket.putWString(m_coninPipe->name()); + setupPacket.putWString(m_conoutPipe->name()); + writePacket(setupPacket); + + m_terminal.reset(new Terminal(*m_conoutPipe)); m_consoleInput.reset(new ConsoleInput(*this)); resetConsoleTracking(Terminal::OmitClear, m_console->windowRect()); @@ -192,21 +234,39 @@ Agent::~Agent() // bytes before it are complete keypresses. void Agent::sendDsr() { - m_dataPipe->write("\x1B[6n"); + // TODO: Disable this in console mode. + m_conoutPipe->write("\x1B[6n"); } -NamedPipe &Agent::makePipe(LPCWSTR pipeName) +NamedPipe &Agent::connectToControlPipe(LPCWSTR pipeName) { NamedPipe &pipe = createNamedPipe(); - if (!pipe.connectToServer(pipeName)) { - trace("error: could not connect to %s", - utf8FromWide(pipeName).c_str()); - ::exit(1); - } + pipe.connectToServer(pipeName, NamedPipe::OpenMode::Duplex); pipe.setReadBufferSize(64 * 1024); return pipe; } +// Returns a new server named pipe. It has not yet been connected. +NamedPipe &Agent::createDataServerPipe(bool write, const wchar_t *kind) +{ + const auto name = + (WStringBuilder(128) + << L"\\\\.\\pipe\\winpty-data-" + << kind << L'-' + << GenRandom().uniqueName()).str_moved(); + 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; +} + void Agent::resetConsoleTracking( Terminal::SendClearFlag sendClear, const SmallRect &windowRect) { @@ -227,10 +287,12 @@ void Agent::resetConsoleTracking( void Agent::onPipeIo(NamedPipe &namedPipe) { - if (&namedPipe == m_controlPipe) { + if (&namedPipe == m_conoutPipe) { + pollConoutPipe(); + } else if (&namedPipe == m_coninPipe) { + pollConinPipe(); + } else if (&namedPipe == m_controlPipe) { pollControlPipe(); - } else if (&namedPipe == m_dataPipe) { - pollDataPipe(); } } @@ -272,14 +334,10 @@ void Agent::pollControlPipe() void Agent::handlePacket(ReadBuffer &packet) { - int type = packet.getInt32(); - int32_t result = -1; + const int type = packet.getInt32(); switch (type) { - case AgentMsg::Ping: - result = 0; - break; case AgentMsg::StartProcess: - result = handleStartProcessPacket(packet); + handleStartProcessPacket(packet); break; case AgentMsg::SetSize: // TODO: I think it might make sense to collapse consecutive SetSize @@ -287,33 +345,28 @@ void Agent::handlePacket(ReadBuffer &packet) // messages faster than they can be processed, and some GUIs might // generate a flood of them, so if we can read multiple SetSize packets // at once, we can ignore the early ones. - result = handleSetSizePacket(packet); - break; - case AgentMsg::GetExitCode: - packet.assertEof(); - result = m_childExitCode; - break; - case AgentMsg::GetProcessId: - packet.assertEof(); - if (m_childProcess == NULL) - result = -1; - else - result = GetProcessId(m_childProcess); - break; - case AgentMsg::SetConsoleMode: - m_terminal->setConsoleMode(packet.getInt32()); - result = 0; + handleSetSizePacket(packet); break; default: trace("Unrecognized message, id:%d", type); } - m_controlPipe->write(&result, sizeof(result)); } -int Agent::handleStartProcessPacket(ReadBuffer &packet) +void Agent::writePacket(WriteBuffer &packet) +{ + const auto &bytes = packet.buf(); + packet.replaceRawValue(0, bytes.size()); + m_controlPipe->write(bytes.data(), bytes.size()); +} + +void Agent::handleStartProcessPacket(ReadBuffer &packet) { - ASSERT(m_childProcess == NULL); + ASSERT(m_childProcess == nullptr); + ASSERT(!m_closingConoutPipe); + const uint64_t spawnFlags = packet.getInt64(); + const bool wantProcessHandle = packet.getInt32(); + const bool wantThreadHandle = packet.getInt32(); const auto program = packet.getWString(); const auto cmdline = packet.getWString(); const auto cwd = packet.getWString(); @@ -325,48 +378,65 @@ int Agent::handleStartProcessPacket(ReadBuffer &packet) auto desktopV = vectorWithNulFromString(desktop); auto envV = vectorFromString(env); - LPCWSTR programArg = program.empty() ? NULL : program.c_str(); - LPWSTR cmdlineArg = cmdline.empty() ? NULL : cmdlineV.data(); - LPCWSTR cwdArg = cwd.empty() ? NULL : cwd.c_str(); - LPWSTR envArg = env.empty() ? NULL : envV.data(); + LPCWSTR programArg = program.empty() ? nullptr : program.c_str(); + LPWSTR cmdlineArg = cmdline.empty() ? nullptr : cmdlineV.data(); + LPCWSTR cwdArg = cwd.empty() ? nullptr : cwd.c_str(); + LPWSTR envArg = env.empty() ? nullptr : envV.data(); STARTUPINFOW sui = {}; PROCESS_INFORMATION pi = {}; sui.cb = sizeof(sui); - sui.lpDesktop = desktop.empty() ? NULL : desktopV.data(); + sui.lpDesktop = desktop.empty() ? nullptr : desktopV.data(); const BOOL success = - CreateProcessW(programArg, cmdlineArg, NULL, NULL, + CreateProcessW(programArg, cmdlineArg, nullptr, nullptr, /*bInheritHandles=*/FALSE, /*dwCreationFlags=*/CREATE_UNICODE_ENVIRONMENT | /*CREATE_NEW_PROCESS_GROUP*/0, envArg, cwdArg, &sui, &pi); - const int ret = success ? 0 : GetLastError(); + const int lastError = success ? 0 : GetLastError(); trace("CreateProcess: %s %u", (success ? "success" : "fail"), static_cast(pi.dwProcessId)); + int64_t replyProcess = 0; + int64_t replyThread = 0; + if (success) { + if (wantProcessHandle) { + replyProcess = int64FromHandle(duplicateHandle(pi.hProcess)); + } + if (wantThreadHandle) { + replyThread = int64FromHandle(duplicateHandle(pi.hThread)); + } CloseHandle(pi.hThread); m_childProcess = pi.hProcess; + m_autoShutdown = (spawnFlags & WINPTY_SPAWN_FLAG_AUTO_SHUTDOWN); } - return ret; + // Write reply. + auto reply = newPacket(); + reply.putInt32(success); + reply.putInt32(lastError); + reply.putInt64(replyProcess); + reply.putInt64(replyThread); + writePacket(reply); } -int Agent::handleSetSizePacket(ReadBuffer &packet) +void Agent::handleSetSizePacket(ReadBuffer &packet) { int cols = packet.getInt32(); int rows = packet.getInt32(); packet.assertEof(); resizeWindow(cols, rows); - return 0; + auto reply = newPacket(); + writePacket(reply); } -void Agent::pollDataPipe() +void Agent::pollConinPipe() { - const std::string newData = m_dataPipe->readAllToString(); + const std::string newData = m_coninPipe->readAllToString(); if (hasDebugFlag("input_separated_bytes")) { // This debug flag is intended to help with testing incomplete escape // sequences and multibyte UTF-8 encodings. (I wonder if the normal @@ -377,14 +447,17 @@ void Agent::pollDataPipe() } else { m_consoleInput->writeInput(newData); } +} +void Agent::pollConoutPipe() +{ // If the child process had exited, then close the data socket if we've // finished sending all of the collected output. - if (m_closingDataPipe && - !m_dataPipe->isClosed() && - m_dataPipe->bytesToSend() == 0) { + if (m_closingConoutPipe && + !m_conoutPipe->isClosed() && + m_conoutPipe->bytesToSend() == 0) { trace("Closing data pipe after data is sent"); - m_dataPipe->closePipe(); + m_conoutPipe->closePipe(); } } @@ -415,31 +488,36 @@ void Agent::onPollTimeout() // escape sequence (e.g. pressing ESC). m_consoleInput->flushIncompleteEscapeCode(); + const bool shouldScrapeContent = !m_closingConoutPipe; + // Check if the child process has exited. - if (WaitForSingleObject(m_childProcess, 0) == WAIT_OBJECT_0) { - DWORD exitCode; - if (GetExitCodeProcess(m_childProcess, &exitCode)) + if (m_autoShutdown && + m_childProcess != nullptr && + WaitForSingleObject(m_childProcess, 0) == WAIT_OBJECT_0) { + DWORD exitCode = 0; + if (GetExitCodeProcess(m_childProcess, &exitCode)) { m_childExitCode = exitCode; + } CloseHandle(m_childProcess); - m_childProcess = NULL; + m_childProcess = nullptr; // Close the data socket to signal to the client that the child // process has exited. If there's any data left to send, send it // before closing the socket. - m_closingDataPipe = true; + m_closingConoutPipe = true; } // Scrape for output *after* the above exit-check to ensure that we collect // the child process's final output. - if (!m_dataPipe->isClosed()) { + if (shouldScrapeContent) { syncConsoleContentAndSize(false); } - if (m_closingDataPipe && - !m_dataPipe->isClosed() && - m_dataPipe->bytesToSend() == 0) { + if (m_closingConoutPipe && + !m_conoutPipe->isClosed() && + m_conoutPipe->bytesToSend() == 0) { trace("Closing data pipe after child exit"); - m_dataPipe->closePipe(); + m_conoutPipe->closePipe(); } } @@ -653,7 +731,7 @@ void Agent::syncConsoleTitle() if (newTitle != m_currentTitle) { std::string command = std::string("\x1b]0;") + wstringToUtf8String(newTitle) + "\x07"; - m_dataPipe->write(command.c_str()); + m_conoutPipe->write(command.c_str()); m_currentTitle = newTitle; } } diff --git a/src/agent/Agent.h b/src/agent/Agent.h index 0890e5a3..67679b24 100644 --- a/src/agent/Agent.h +++ b/src/agent/Agent.h @@ -39,6 +39,7 @@ class Win32Console; class ConsoleInput; class ReadBuffer; +class WriteBuffer; class NamedPipe; struct ConsoleScreenBufferInfo; @@ -53,23 +54,26 @@ class Agent : public EventLoop, public DsrSender { public: Agent(LPCWSTR controlPipeName, - LPCWSTR dataPipeName, + uint64_t agentFlags, int initialCols, int initialRows); virtual ~Agent(); void sendDsr(); private: - NamedPipe &makePipe(LPCWSTR pipeName); + NamedPipe &connectToControlPipe(LPCWSTR pipeName); + NamedPipe &createDataServerPipe(bool write, const wchar_t *kind); void resetConsoleTracking( Terminal::SendClearFlag sendClear, const SmallRect &windowRect); private: void pollControlPipe(); void handlePacket(ReadBuffer &packet); - int handleStartProcessPacket(ReadBuffer &packet); - int handleSetSizePacket(ReadBuffer &packet); - void pollDataPipe(); + void writePacket(WriteBuffer &packet); + void handleStartProcessPacket(ReadBuffer &packet); + void handleSetSizePacket(ReadBuffer &packet); + void pollConinPipe(); + void pollConoutPipe(); void updateMouseInputFlags(bool forceTrace=false); protected: @@ -99,11 +103,13 @@ class Agent : public EventLoop, public DsrSender bool m_consoleMouseInputEnabled = false; bool m_consoleQuickEditEnabled = false; NamedPipe *m_controlPipe = nullptr; - NamedPipe *m_dataPipe = nullptr; - bool m_closingDataPipe = false; + NamedPipe *m_coninPipe = nullptr; + NamedPipe *m_conoutPipe = nullptr; + bool m_closingConoutPipe = false; std::unique_ptr m_terminal; std::unique_ptr m_consoleInput; HANDLE m_childProcess = nullptr; + bool m_autoShutdown = false; int m_childExitCode = -1; int m_syncRow = 0; diff --git a/src/agent/NamedPipe.cc b/src/agent/NamedPipe.cc index 793d2425..9a25c3b7 100644 --- a/src/agent/NamedPipe.cc +++ b/src/agent/NamedPipe.cc @@ -26,43 +26,71 @@ #include "NamedPipe.h" #include "../shared/DebugClient.h" #include "../shared/StringUtil.h" +#include "../shared/WindowsSecurity.h" #include "../shared/WinptyAssert.h" // Returns true if anything happens (data received, data sent, pipe error). bool NamedPipe::serviceIo(std::vector *waitHandles) { + bool justConnected = false; const auto kError = ServiceResult::Error; const auto kProgress = ServiceResult::Progress; + const auto kNoProgress = ServiceResult::NoProgress; if (m_handle == NULL) { return false; } - const auto readProgress = m_inputWorker->service(); - const auto writeProgress = m_outputWorker->service(); + if (m_connectEvent.get() != nullptr) { + // We're still connecting this server pipe. Check whether the pipe is + // now connected. If it isn't, add the pipe to the list of handles to + // wait on. + DWORD actual = 0; + BOOL success = + GetOverlappedResult(m_handle, &m_connectOver, &actual, FALSE); + if (!success && GetLastError() == ERROR_PIPE_CONNECTED) { + // I'm not sure this can happen, but it's easy to handle if it + // does. + success = TRUE; + } + if (!success) { + ASSERT(GetLastError() == ERROR_IO_INCOMPLETE && + "Pended ConnectNamedPipe call failed"); + waitHandles->push_back(m_connectEvent.get()); + } else { + trace("Server pipe [%s] connected", + utf8FromWide(m_name).c_str()); + m_connectEvent.dispose(); + startPipeWorkers(); + justConnected = true; + } + } + const auto readProgress = m_inputWorker ? m_inputWorker->service() : kNoProgress; + const auto writeProgress = m_outputWorker ? m_outputWorker->service() : kNoProgress; if (readProgress == kError || writeProgress == kError) { closePipe(); return true; } - if (m_inputWorker->getWaitEvent() != NULL) { + if (m_inputWorker && m_inputWorker->getWaitEvent() != nullptr) { waitHandles->push_back(m_inputWorker->getWaitEvent()); } - if (m_outputWorker->getWaitEvent() != NULL) { + if (m_outputWorker && m_outputWorker->getWaitEvent() != nullptr) { waitHandles->push_back(m_outputWorker->getWaitEvent()); } - return readProgress == kProgress || writeProgress == kProgress; + return justConnected + || readProgress == kProgress + || writeProgress == kProgress; } -NamedPipe::IoWorker::IoWorker(NamedPipe *namedPipe) : - m_namedPipe(namedPipe), - m_pending(false), - m_currentIoSize(0) -{ - m_event = CreateEventW(NULL, TRUE, FALSE, NULL); - ASSERT(m_event != NULL); +// manual reset, initially unset +static OwnedHandle createEvent() { + HANDLE ret = CreateEventW(nullptr, TRUE, FALSE, nullptr); + ASSERT(ret != nullptr && "CreateEventW failed"); + return OwnedHandle(ret); } -NamedPipe::IoWorker::~IoWorker() +NamedPipe::IoWorker::IoWorker(NamedPipe &namedPipe) : + m_namedPipe(namedPipe), + m_event(createEvent()) { - CloseHandle(m_event); } NamedPipe::ServiceResult NamedPipe::IoWorker::service() @@ -70,7 +98,7 @@ NamedPipe::ServiceResult NamedPipe::IoWorker::service() ServiceResult progress = ServiceResult::NoProgress; if (m_pending) { DWORD actual = 0; - BOOL ret = GetOverlappedResult(m_namedPipe->m_handle, &m_over, &actual, FALSE); + BOOL ret = GetOverlappedResult(m_namedPipe.m_handle, &m_over, &actual, FALSE); if (!ret) { if (GetLastError() == ERROR_IO_INCOMPLETE) { // There is a pending I/O. @@ -80,7 +108,7 @@ NamedPipe::ServiceResult NamedPipe::IoWorker::service() return ServiceResult::Error; } } - ResetEvent(m_event); + ResetEvent(m_event.get()); m_pending = false; completeIo(actual); m_currentIoSize = 0; @@ -92,10 +120,10 @@ NamedPipe::ServiceResult NamedPipe::IoWorker::service() m_currentIoSize = nextSize; DWORD actual = 0; memset(&m_over, 0, sizeof(m_over)); - m_over.hEvent = m_event; + m_over.hEvent = m_event.get(); BOOL ret = isRead - ? ReadFile(m_namedPipe->m_handle, m_buffer, nextSize, &actual, &m_over) - : WriteFile(m_namedPipe->m_handle, m_buffer, nextSize, &actual, &m_over); + ? ReadFile(m_namedPipe.m_handle, m_buffer, nextSize, &actual, &m_over) + : WriteFile(m_namedPipe.m_handle, m_buffer, nextSize, &actual, &m_over); if (!ret) { if (GetLastError() == ERROR_IO_PENDING) { // There is a pending I/O. @@ -106,7 +134,7 @@ NamedPipe::ServiceResult NamedPipe::IoWorker::service() return ServiceResult::Error; } } - ResetEvent(m_event); + ResetEvent(m_event.get()); completeIo(actual); m_currentIoSize = 0; progress = ServiceResult::Progress; @@ -121,27 +149,27 @@ void NamedPipe::IoWorker::waitForCanceledIo() { if (m_pending) { DWORD actual = 0; - GetOverlappedResult(m_namedPipe->m_handle, &m_over, &actual, TRUE); + GetOverlappedResult(m_namedPipe.m_handle, &m_over, &actual, TRUE); m_pending = false; } } HANDLE NamedPipe::IoWorker::getWaitEvent() { - return m_pending ? m_event : NULL; + return m_pending ? m_event.get() : NULL; } void NamedPipe::InputWorker::completeIo(DWORD size) { - m_namedPipe->m_inQueue.append(m_buffer, size); + m_namedPipe.m_inQueue.append(m_buffer, size); } bool NamedPipe::InputWorker::shouldIssueIo(DWORD *size, bool *isRead) { *isRead = true; - if (m_namedPipe->isClosed()) { + if (m_namedPipe.isClosed()) { return false; - } else if (m_namedPipe->m_inQueue.size() < m_namedPipe->readBufferSize()) { + } else if (m_namedPipe.m_inQueue.size() < m_namedPipe.readBufferSize()) { *size = kIoSize; return true; } else { @@ -157,8 +185,8 @@ void NamedPipe::OutputWorker::completeIo(DWORD size) bool NamedPipe::OutputWorker::shouldIssueIo(DWORD *size, bool *isRead) { *isRead = false; - if (!m_namedPipe->m_outQueue.empty()) { - auto &out = m_namedPipe->m_outQueue; + if (!m_namedPipe.m_outQueue.empty()) { + auto &out = m_namedPipe.m_outQueue; const DWORD writeSize = std::min(out.size(), kIoSize); std::copy(&out[0], &out[writeSize], m_buffer); out.erase(0, writeSize); @@ -174,9 +202,58 @@ DWORD NamedPipe::OutputWorker::getPendingIoSize() return m_pending ? m_currentIoSize : 0; } -bool NamedPipe::connectToServer(LPCWSTR pipeName) +void NamedPipe::openServerPipe(LPCWSTR pipeName, OpenMode::t openMode, + int outBufferSize, int inBufferSize) { + ASSERT(isClosed()); + ASSERT((openMode & OpenMode::Duplex) != 0); + 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 auto sd = createPipeSecurityDescriptorOwnerFullControl(); + ASSERT(sd && "error creating data pipe SECURITY_DESCRIPTOR"); + SECURITY_ATTRIBUTES sa = {}; + sa.nLength = sizeof(sa); + sa.lpSecurityDescriptor = sd.get(); + HANDLE handle = CreateNamedPipeW( + pipeName, + /*dwOpenMode=*/winOpenMode, + /*dwPipeMode=*/rejectRemoteClientsPipeFlag(), + /*nMaxInstances=*/1, + /*nOutBufferSize=*/outBufferSize, + /*nInBufferSize=*/inBufferSize, + /*nDefaultTimeOut=*/30000, + &sa); + trace("opened server pipe [%s], handle == %p", + utf8FromWide(pipeName).c_str(), handle); + ASSERT(handle != INVALID_HANDLE_VALUE && "Could not open server pipe"); + m_name = pipeName; + m_handle = handle; + m_openMode = openMode; + + // Start an asynchronous connection attempt. + m_connectEvent = createEvent(); + memset(&m_connectOver, 0, sizeof(m_connectOver)); + m_connectOver.hEvent = m_connectEvent.get(); + BOOL success = ConnectNamedPipe(m_handle, &m_connectOver); + const auto err = GetLastError(); + if (!success && err == ERROR_PIPE_CONNECTED) { + success = TRUE; + } + if (success) { + trace("Server pipe [%s] connected", utf8FromWide(pipeName).c_str()); + m_connectEvent.dispose(); + startPipeWorkers(); + } else if (err != ERROR_IO_PENDING) { + ASSERT(false && "ConnectNamedPipe call failed"); + } +} + +void NamedPipe::connectToServer(LPCWSTR pipeName, OpenMode::t openMode) { ASSERT(isClosed()); + ASSERT((openMode & OpenMode::Duplex) != 0); HANDLE handle = CreateFileW( pipeName, GENERIC_READ | GENERIC_WRITE, @@ -185,18 +262,28 @@ bool NamedPipe::connectToServer(LPCWSTR pipeName) OPEN_EXISTING, SECURITY_SQOS_PRESENT | SECURITY_IDENTIFICATION | FILE_FLAG_OVERLAPPED, NULL); - trace("connection to [%s], handle == %p", + trace("connected to [%s], handle == %p", utf8FromWide(pipeName).c_str(), handle); - if (handle == INVALID_HANDLE_VALUE) - return false; + ASSERT(handle != INVALID_HANDLE_VALUE && "Could not connect to pipe"); + m_name = pipeName; m_handle = handle; - m_inputWorker = new InputWorker(this); - m_outputWorker = new OutputWorker(this); - return true; + m_openMode = openMode; + startPipeWorkers(); +} + +void NamedPipe::startPipeWorkers() +{ + if (m_openMode & OpenMode::Reading) { + m_inputWorker.reset(new InputWorker(*this)); + } + if (m_openMode & OpenMode::Writing) { + m_outputWorker.reset(new OutputWorker(*this)); + } } size_t NamedPipe::bytesToSend() { + ASSERT(m_openMode & OpenMode::Writing); auto ret = m_outQueue.size(); if (m_outputWorker != NULL) { ret += m_outputWorker->getPendingIoSize(); @@ -206,6 +293,7 @@ size_t NamedPipe::bytesToSend() void NamedPipe::write(const void *data, size_t size) { + ASSERT(m_openMode & OpenMode::Writing); m_outQueue.append(reinterpret_cast(data), size); } @@ -216,21 +304,25 @@ void NamedPipe::write(const char *text) size_t NamedPipe::readBufferSize() { + ASSERT(m_openMode & OpenMode::Reading); return m_readBufferSize; } void NamedPipe::setReadBufferSize(size_t size) { + ASSERT(m_openMode & OpenMode::Reading); m_readBufferSize = size; } size_t NamedPipe::bytesAvailable() { + ASSERT(m_openMode & OpenMode::Reading); return m_inQueue.size(); } size_t NamedPipe::peek(void *data, size_t size) { + ASSERT(m_openMode & OpenMode::Reading); const auto out = reinterpret_cast(data); const size_t ret = std::min(size, m_inQueue.size()); std::copy(&m_inQueue[0], &m_inQueue[size], out); @@ -246,6 +338,7 @@ size_t NamedPipe::read(void *data, size_t size) std::string NamedPipe::readToString(size_t size) { + ASSERT(m_openMode & OpenMode::Reading); size_t retSize = std::min(size, m_inQueue.size()); std::string ret = m_inQueue.substr(0, retSize); m_inQueue.erase(0, retSize); @@ -254,6 +347,7 @@ std::string NamedPipe::readToString(size_t size) std::string NamedPipe::readAllToString() { + ASSERT(m_openMode & OpenMode::Reading); std::string ret = m_inQueue; m_inQueue.clear(); return ret; @@ -261,20 +355,23 @@ std::string NamedPipe::readAllToString() void NamedPipe::closePipe() { - if (m_handle == NULL) + if (m_handle == NULL) { return; + } CancelIo(m_handle); - m_inputWorker->waitForCanceledIo(); - m_outputWorker->waitForCanceledIo(); - delete m_inputWorker; - delete m_outputWorker; + if (m_connectEvent.get() != nullptr) { + DWORD actual = 0; + GetOverlappedResult(m_handle, &m_connectOver, &actual, TRUE); + m_connectEvent.dispose(); + } + if (m_inputWorker) { + m_inputWorker->waitForCanceledIo(); + m_inputWorker.reset(); + } + if (m_outputWorker) { + m_outputWorker->waitForCanceledIo(); + m_outputWorker.reset(); + } CloseHandle(m_handle); m_handle = NULL; - m_inputWorker = NULL; - m_outputWorker = NULL; -} - -bool NamedPipe::isClosed() -{ - return m_handle == NULL; } diff --git a/src/agent/NamedPipe.h b/src/agent/NamedPipe.h index f0d31a70..779b9b87 100644 --- a/src/agent/NamedPipe.h +++ b/src/agent/NamedPipe.h @@ -22,9 +22,13 @@ #define NAMEDPIPE_H #include + +#include #include #include +#include "../shared/OwnedHandle.h" + class EventLoop; class NamedPipe @@ -35,6 +39,7 @@ class NamedPipe NamedPipe() {} ~NamedPipe() { closePipe(); } bool serviceIo(std::vector *waitHandles); + void startPipeWorkers(); enum class ServiceResult { NoProgress, Error, Progress }; @@ -42,17 +47,17 @@ class NamedPipe class IoWorker { public: - IoWorker(NamedPipe *namedPipe); - virtual ~IoWorker(); + IoWorker(NamedPipe &namedPipe); + virtual ~IoWorker() {} ServiceResult service(); void waitForCanceledIo(); HANDLE getWaitEvent(); protected: - NamedPipe *m_namedPipe; - bool m_pending; - DWORD m_currentIoSize; - HANDLE m_event; - OVERLAPPED m_over; + NamedPipe &m_namedPipe; + bool m_pending = false; + DWORD m_currentIoSize = 0; + OwnedHandle m_event; + OVERLAPPED m_over = {}; enum { kIoSize = 64 * 1024 }; char m_buffer[kIoSize]; virtual void completeIo(DWORD size) = 0; @@ -62,7 +67,7 @@ class NamedPipe class InputWorker : public IoWorker { public: - InputWorker(NamedPipe *namedPipe) : IoWorker(namedPipe) {} + InputWorker(NamedPipe &namedPipe) : IoWorker(namedPipe) {} protected: virtual void completeIo(DWORD size); virtual bool shouldIssueIo(DWORD *size, bool *isRead); @@ -71,7 +76,7 @@ class NamedPipe class OutputWorker : public IoWorker { public: - OutputWorker(NamedPipe *namedPipe) : IoWorker(namedPipe) {} + OutputWorker(NamedPipe &namedPipe) : IoWorker(namedPipe) {} DWORD getPendingIoSize(); protected: virtual void completeIo(DWORD size); @@ -79,7 +84,15 @@ class NamedPipe }; public: - bool connectToServer(LPCWSTR pipeName); + struct OpenMode { + typedef int t; + enum { None = 0, Reading = 1, Writing = 2, Duplex = 3 }; + }; + + std::wstring name() const { return m_name; } + void openServerPipe(LPCWSTR pipeName, OpenMode::t openMode, + int outBufferSize, int inBufferSize); + void connectToServer(LPCWSTR pipeName, OpenMode::t openMode); size_t bytesToSend(); void write(const void *data, size_t size); void write(const char *text); @@ -91,16 +104,21 @@ class NamedPipe std::string readToString(size_t size); std::string readAllToString(); void closePipe(); - bool isClosed(); + bool isClosed() { return m_handle == nullptr; } + bool isConnecting() { return m_connectEvent.get() != nullptr; } private: // Input/output buffers + std::wstring m_name; + OVERLAPPED m_connectOver = {}; + OwnedHandle m_connectEvent; + OpenMode::t m_openMode = OpenMode::None; size_t m_readBufferSize = 64 * 1024; std::string m_inQueue; std::string m_outQueue; HANDLE m_handle = nullptr; - InputWorker *m_inputWorker = nullptr; - OutputWorker *m_outputWorker = nullptr; + std::unique_ptr m_inputWorker; + std::unique_ptr m_outputWorker; }; #endif // NAMEDPIPE_H diff --git a/src/agent/main.cc b/src/agent/main.cc index b1789dc4..5bf414bd 100644 --- a/src/agent/main.cc +++ b/src/agent/main.cc @@ -31,7 +31,7 @@ #include "../shared/WinptyVersion.h" const char USAGE[] = -"Usage: %ls controlPipeName dataPipeName cols rows\n" +"Usage: %ls controlPipeName flags cols rows\n" "\n" "Ordinarily, this program is launched by winpty.dll and is not directly\n" "useful to winpty users. However, it also has options intended for\n" @@ -45,8 +45,11 @@ const char USAGE[] = " Include MOUSE_INPUT_RECORDs in the dump output\n" " --version Print the winpty version\n"; -int main() -{ +static uint64_t winpty_atoi64(const char *str) { + return strtoll(str, NULL, 10); +} + +int main() { dumpWindowsVersion(); dumpVersionToTrace(); @@ -80,7 +83,7 @@ int main() } Agent agent(argv[1], - argv[2], + winpty_atoi64(utf8FromWide(argv[2]).c_str()), atoi(utf8FromWide(argv[3]).c_str()), atoi(utf8FromWide(argv[4]).c_str())); agent.run(); diff --git a/src/agent/subdir.mk b/src/agent/subdir.mk index 50fcf98f..c78366f0 100644 --- a/src/agent/subdir.mk +++ b/src/agent/subdir.mk @@ -38,7 +38,10 @@ AGENT_OBJECTS = \ build/agent/agent/main.o \ build/agent/shared/Buffer.o \ build/agent/shared/DebugClient.o \ + build/agent/shared/GenRandom.o \ + build/agent/shared/OwnedHandle.o \ build/agent/shared/StringUtil.o \ + build/agent/shared/WindowsSecurity.o \ build/agent/shared/WindowsVersion.o \ build/agent/shared/WinptyAssert.o \ build/agent/shared/WinptyException.o \ diff --git a/src/include/winpty.h b/src/include/winpty.h index 26a19ceb..6f24c0e2 100644 --- a/src/include/winpty.h +++ b/src/include/winpty.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011-2012 Ryan Prichard + * Copyright (c) 2011-2016 Ryan Prichard * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to @@ -23,9 +23,15 @@ #ifndef WINPTY_H #define WINPTY_H -#include #include +#include "winpty_constants.h" + +/* On 32-bit Windows, winpty functions have the default __cdecl (not __stdcall) + * calling convention. (64-bit Windows has only a single calling convention.) + * When compiled with __declspec(dllexport), with either MinGW or MSVC, the + * winpty functions are unadorned--no underscore prefix or '@nn' suffix--so + * GetProcAddress can be used easily. */ #ifdef COMPILING_WINPTY_DLL #define WINPTY_API __declspec(dllexport) #else @@ -36,70 +42,181 @@ extern "C" { #endif +/* The winpty API uses wide characters, instead of UTF-8, to avoid conversion + * complications related to surrogates. Windows generally tolerates unpaired + * surrogates in text, which makes conversion to and from UTF-8 ambiguous and + * complicated. (There are different UTF-8 variants that deal with UTF-16 + * surrogates differently.) */ + + + +/***************************************************************************** + * Error handling. */ + +/* All the APIs have an optional winpty_error_t output parameter. If a + * non-NULL argument is specified, then either the API writes NULL to the + * value (on success) or writes a newly allocated winpty_error_t object. The + * object must be freed using winpty_error_free. */ + +/* An error object. */ +typedef struct winpty_error_s winpty_error_t; +typedef winpty_error_t *winpty_error_ptr_t; + +/* An error code -- one of WINPTY_ERROR_xxx. */ +typedef DWORD winpty_result_t; + +/* Gets the error code from the error object. */ +WINPTY_API winpty_result_t winpty_error_code(winpty_error_ptr_t err); + +/* Returns a textual representation of the error. The string is freed when + * the error is freed. */ +WINPTY_API LPCWSTR winpty_error_msg(winpty_error_ptr_t err); + +/* Free the error object. Every error returned from the winpty API must be + * freed. */ +WINPTY_API void winpty_error_free(winpty_error_ptr_t err); + + + +/***************************************************************************** + * Configuration of a new agent. */ + +/* The winpty_config_t object is not thread-safe. */ +typedef struct winpty_config_s winpty_config_t; + +/* Allocate a winpty_config_t value. Returns NULL on error. There are no + * required settings -- the object may immediately be used. agentFlags is a + * set of zero or more WINPTY_FLAG_xxx values. An unrecognized flag is an + * error. */ +WINPTY_API winpty_config_t * +winpty_config_new(UINT64 agentFlags, winpty_error_ptr_t *err /*OPTIONAL*/); + +/* Free the cfg object after passing it to winpty_open. */ +WINPTY_API void winpty_config_free(winpty_config_t *cfg); + +WINPTY_API BOOL +winpty_config_set_initial_size(winpty_config_t *cfg, int cols, int rows, + winpty_error_ptr_t *err /*OPTIONAL*/); + +/* Amount of time to wait for the agent to startup and to wait for any given + * agent RPC request. Must be greater than 0. Can be INFINITE. */ +WINPTY_API BOOL +winpty_config_set_agent_timeout(winpty_config_t *cfg, DWORD timeoutMs, + winpty_error_ptr_t *err /*OPTIONAL*/); + + + +/***************************************************************************** + * Start the agent. */ + +/* The winpty_t object is thread-safe. */ typedef struct winpty_s winpty_t; -/* - * winpty API. - */ +/* Starts the agent. Returns NULL on error. This process will connect to the + * agent over a control pipe, and the agent will open CONIN and CONOUT server + * pipes. The agent blocks until these pipes are connected, so the client must + * connect to them before invoking an agent RPC (or else deadlock). */ +WINPTY_API winpty_t * +winpty_open(const winpty_config_t *cfg, + winpty_error_ptr_t *err /*OPTIONAL*/); -/* - * Starts a new winpty instance with the given size. +/* A handle to the agent process. This value is valid for the lifetime of the + * winpty_t object. Do not close it. */ +WINPTY_API HANDLE winpty_agent_process(winpty_t *wp); + + + +/***************************************************************************** + * I/O pipes. */ + +/* Returns the names of named pipes used for terminal I/O. Each input or + * output direction uses a different half-duplex pipe. The agent creates + * these pipes, and the client can connect to them using ordinary I/O methods. + * The strings are freed when the winpty_t object is freed. * - * This function creates a new agent process and connects to it. - */ -WINPTY_API winpty_t *winpty_open(int cols, int rows); + * N.B.: CreateFile does not block when connecting to a local server pipe. If + * the server pipe does not exist or is already connected, then it fails + * instantly. */ +WINPTY_API LPCWSTR winpty_conin_name(winpty_t *wp); +WINPTY_API LPCWSTR winpty_conout_name(winpty_t *wp); + + + +/***************************************************************************** + * winpty agent RPC call: process creation. */ + +/* The winpty_spawn_config_t object is not thread-safe. */ +typedef struct winpty_spawn_config_s winpty_spawn_config_t; + +/* winpty_spawn_config strings do not need to live as long as the config + * object. They are copied. Returns NULL on error. spawnFlags is a set of + * zero or more WINPTY_SPAWN_FLAG_xxx values. An unrecognized flag is an + * error. + * + * env is a a pointer to an environment block like that passed to + * CreateProcess--a contiguous array of NUL-terminated "VAR=VAL" strings + * followed by a final NUL terminator. */ +WINPTY_API winpty_spawn_config_t * +winpty_spawn_config_new(UINT64 spawnFlags, + LPCWSTR appname /*OPTIONAL*/, + LPCWSTR cmdline /*OPTIONAL*/, + LPCWSTR cwd /*OPTIONAL*/, + LPCWSTR env /*OPTIONAL*/, + winpty_error_ptr_t *err /*OPTIONAL*/); + +/* Free the cfg object after passing it to winpty_spawn. */ +WINPTY_API void winpty_spawn_config_free(winpty_spawn_config_t *cfg); /* - * Start a child process. Either (but not both) of appname and cmdline may - * be NULL. cwd and env may be NULL. env is a pointer to an environment - * block like that passed to CreateProcess. + * Spawns the new process. + * + * The function initializes all output parameters to zero or NULL. * - * This function never modifies the cmdline, unlike CreateProcess. + * On success, the function returns TRUE. For each of process_handle and + * thread_handle that is non-NULL, the HANDLE returned from CreateProcess is + * duplicated from the agent and returned to the winpty client. The client is + * responsible for closing these HANDLES. * - * Only one child process may be started. After the child process exits, the - * agent will scrape the console output one last time, then close the data pipe - * once all remaining data has been sent. + * On failure, the function returns FALSE, and if err is non-NULL, then *err + * is set to an error object. * - * Returns 0 on success or a Win32 error code on failure. + * If the agent's CreateProcess call failed, then *create_process_error is set + * to GetLastError(), and the WINPTY_ERROR_SPAWN_CREATE_PROCESS_FAILED error + * is returned. + * + * N.B.: GetProcessId works even if the process has exited. The PID is not + * recycled until the NT process object is freed. + * (https://blogs.msdn.microsoft.com/oldnewthing/20110107-00/?p=11803) */ -WINPTY_API int winpty_start_process(winpty_t *pc, - const wchar_t *appname, - const wchar_t *cmdline, - const wchar_t *cwd, - const wchar_t *env); +WINPTY_API BOOL +winpty_spawn(winpty_t *wp, + const winpty_spawn_config_t *cfg, + HANDLE *process_handle /*OPTIONAL*/, + HANDLE *thread_handle /*OPTIONAL*/, + DWORD *create_process_error /*OPTIONAL*/, + winpty_error_ptr_t *err /*OPTIONAL*/); -/* - * Returns the exit code of the process started with winpty_start_process, - * or -1 none is available. - */ -WINPTY_API int winpty_get_exit_code(winpty_t *pc); -/* - * Returns the process id of the process started with winpty_start_process, - * or -1 none is available. - */ -WINPTY_API int winpty_get_process_id(winpty_t *pc); -/* - * Returns an overlapped-mode pipe handle that can be read and written - * like a Unix terminal. - */ -WINPTY_API HANDLE winpty_get_data_pipe(winpty_t *pc); +/***************************************************************************** + * winpty agent RPC calls: everything else */ -/* - * Change the size of the Windows console. - */ -WINPTY_API int winpty_set_size(winpty_t *pc, int cols, int rows); +/* Change the size of the Windows console. Returns an error code. */ +WINPTY_API BOOL +winpty_set_size(winpty_t *wp, int cols, int rows, + winpty_error_ptr_t *err /*OPTIONAL*/); -/* - * Toggle the console mode. If in console mode, no terminal escape sequences are send. - */ -WINPTY_API int winpty_set_console_mode(winpty_t *pc, int mode); +/* Frees the winpty_t object and the OS resources contained in it. This + * call breaks the connection with the agent, which should then close its + * console, terminating the processes attached to it. + * + * It is a programmer error to call this function if any other threads are + * using the winpty_t object. Undefined behavior results. */ +WINPTY_API void winpty_free(winpty_t *wp); -/* - * Closes the winpty. - */ -WINPTY_API void winpty_close(winpty_t *pc); + + +/****************************************************************************/ #ifdef __cplusplus } diff --git a/src/include/winpty_constants.h b/src/include/winpty_constants.h new file mode 100755 index 00000000..3d799629 --- /dev/null +++ b/src/include/winpty_constants.h @@ -0,0 +1,75 @@ +/* + * Copyright (c) 2016 Ryan Prichard + * + * 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. + */ + +#ifndef WINPTY_CONSTANTS_H +#define WINPTY_CONSTANTS_H + +// TODO: Review the winpty and spawn flags... + +/* + * You may want to include winpty.h instead, which includes this header. + * + * This file is split out from winpty.h so that the agent can access the + * winpty flags without also declaring the libwinpty APIs. + */ + +/***************************************************************************** + * Error codes. */ + +#define WINPTY_ERROR_SUCCESS 0 +#define WINPTY_ERROR_OUT_OF_MEMORY 1 +#define WINPTY_ERROR_SPAWN_CREATE_PROCESS_FAILED 2 +#define WINPTY_ERROR_LOST_CONNECTION 3 +#define WINPTY_ERROR_AGENT_EXE_MISSING 4 +#define WINPTY_ERROR_UNSPECIFIED 5 +#define WINPTY_ERROR_AGENT_DIED 6 +#define WINPTY_ERROR_AGENT_TIMEOUT 7 +#define WINPTY_ERROR_AGENT_CREATION_FAILED 8 + + + +/***************************************************************************** + * Configuration of a new agent. */ + +#define WINPTY_FLAG_MASK 0ull + + + +/***************************************************************************** + * winpty agent RPC call: process creation. */ + +/* If the spawn is marked "auto-shutdown", then the agent shuts down console + * output once the process exits. The agent stops polling for new console + * output, and once all pending data has been written to the output pipe, the + * agent closes the pipe. (At that point, the pipe may still have data in it, + * which the client may read. Once all the data has been read, further reads + * return EOF.) */ +#define WINPTY_SPAWN_FLAG_AUTO_SHUTDOWN 1ull + +/* All the spawn flags. */ +#define WINPTY_SPAWN_FLAG_MASK (0ull \ + | WINPTY_SPAWN_FLAG_AUTO_SHUTDOWN \ +) + + + +#endif /* WINPTY_CONSTANTS_H */ diff --git a/src/libwinpty/AgentLocation.cc b/src/libwinpty/AgentLocation.cc index 0bdb9427..82d00b2d 100755 --- a/src/libwinpty/AgentLocation.cc +++ b/src/libwinpty/AgentLocation.cc @@ -26,6 +26,8 @@ #include "../shared/WinptyAssert.h" +#include "LibWinptyException.h" + #define AGENT_EXE L"winpty-agent.exe" static HMODULE getCurrentModule() { @@ -64,6 +66,10 @@ static bool pathExists(const std::wstring &path) { std::wstring findAgentProgram() { std::wstring progDir = dirname(getModuleFileName(getCurrentModule())); std::wstring ret = progDir + (L"\\" AGENT_EXE); - ASSERT(pathExists(ret)); + if (!pathExists(ret)) { + throw LibWinptyException( + WINPTY_ERROR_AGENT_EXE_MISSING, + (L"agent executable does not exist: '" + ret + L"'").c_str()); + } return ret; } diff --git a/src/unix-adapter/DualWakeup.h b/src/libwinpty/LibWinptyException.h old mode 100644 new mode 100755 similarity index 57% rename from src/unix-adapter/DualWakeup.h rename to src/libwinpty/LibWinptyException.h index 9fcef0b1..f2fdc228 --- a/src/unix-adapter/DualWakeup.h +++ b/src/libwinpty/LibWinptyException.h @@ -1,4 +1,4 @@ -// Copyright (c) 2015 Ryan Prichard +// Copyright (c) 2016 Ryan Prichard // // Permission is hereby granted, free of charge, to any person obtaining a copy // of this software and associated documentation files (the "Software"), to @@ -18,32 +18,37 @@ // FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS // IN THE SOFTWARE. -#ifndef UNIX_ADAPTER_DUAL_WAKEUP_H -#define UNIX_ADAPTER_DUAL_WAKEUP_H +#ifndef LIB_WINPTY_EXCEPTION_H +#define LIB_WINPTY_EXCEPTION_H -#include "Event.h" -#include "WakeupFd.h" +#include "../include/winpty.h" -class DualWakeup { +#include "../shared/WinptyException.h" + +#include +#include + +class LibWinptyException : public WinptyException { public: - void set() { - m_event.set(); - m_wakeupfd.set(); - } - void reset() { - m_event.reset(); - m_wakeupfd.reset(); + LibWinptyException(winpty_result_t code, const wchar_t *what) : + m_code(code), m_what(std::make_shared(what)) {} + + winpty_result_t code() const WINPTY_NOEXCEPT { + return m_code; } - HANDLE handle() { - return m_event.handle(); + + const wchar_t *what() const WINPTY_NOEXCEPT { + return m_what->c_str(); } - int fd() { - return m_wakeupfd.fd(); + + std::shared_ptr whatSharedStr() const WINPTY_NOEXCEPT { + return m_what; } private: - Event m_event; - WakeupFd m_wakeupfd; + winpty_result_t m_code; + // Using a shared_ptr ensures that copying the object raises no exception. + std::shared_ptr m_what; }; -#endif // UNIX_ADAPTER_DUAL_WAKEUP_H +#endif // LIB_WINPTY_EXCEPTION_H diff --git a/src/libwinpty/WinptyInternal.h b/src/libwinpty/WinptyInternal.h new file mode 100755 index 00000000..7f5b915c --- /dev/null +++ b/src/libwinpty/WinptyInternal.h @@ -0,0 +1,69 @@ +// Copyright (c) 2016 Ryan Prichard +// +// 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. + +#ifndef LIBWINPTY_WINPTY_INTERNAL_H +#define LIBWINPTY_WINPTY_INTERNAL_H + +#include +#include + +#include "../include/winpty.h" + +#include "../shared/Mutex.h" +#include "../shared/OwnedHandle.h" + +// The structures in this header are not intended to be accessed directly by +// client programs. + +struct winpty_error_s { + winpty_result_t code; + const wchar_t *msgStatic; + // Use a pointer to a std::shared_ptr so that the struct remains simple + // enough to statically initialize, for the benefit of static error + // objects like kOutOfMemory. + std::shared_ptr *msgDynamic; +}; + +struct winpty_config_s { + uint64_t flags = 0; + int cols = 80; + int rows = 25; + DWORD timeoutMs = 30000; +}; + +struct winpty_s { + Mutex mutex; + OwnedHandle agentProcess; + OwnedHandle controlPipe; + DWORD agentTimeoutMs = 0; + OwnedHandle ioEvent; + std::wstring coninPipeName; + std::wstring conoutPipeName; +}; + +struct winpty_spawn_config_s { + uint64_t winptyFlags = 0; + std::wstring appname; + std::wstring cmdline; + std::wstring cwd; + std::wstring env; +}; + +#endif // LIBWINPTY_WINPTY_INTERNAL_H diff --git a/src/libwinpty/winpty.cc b/src/libwinpty/winpty.cc index c7555f00..9906dd52 100644 --- a/src/libwinpty/winpty.cc +++ b/src/libwinpty/winpty.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2011-2015 Ryan Prichard +// Copyright (c) 2011-2016 Ryan Prichard // // Permission is hereby granted, free of charge, to any person obtaining a copy // of this software and associated documentation files (the "Software"), to @@ -20,8 +20,6 @@ #include -#include - #include #include #include @@ -30,6 +28,8 @@ #include #include +#include "../include/winpty.h" + #include "../shared/AgentMsg.h" #include "../shared/Buffer.h" #include "../shared/DebugClient.h" @@ -44,93 +44,362 @@ #include "../shared/WinptyVersion.h" #include "AgentLocation.h" +#include "LibWinptyException.h" +#include "WinptyInternal.h" + -// TODO: Error handling, handle out-of-memory. -struct winpty_s { - winpty_s(); - HANDLE controlPipe; - HANDLE dataPipe; +/***************************************************************************** + * Error handling -- translate C++ exceptions to an optional error object + * output and log the result. */ + +static const winpty_error_s kOutOfMemory = { + WINPTY_ERROR_OUT_OF_MEMORY, + L"Out of memory", + nullptr }; -winpty_s::winpty_s() : controlPipe(NULL), dataPipe(NULL) -{ +static const winpty_error_s kBadRpcPacket = { + WINPTY_ERROR_UNSPECIFIED, + L"Bad RPC packet", + nullptr +}; + +static const winpty_error_s kUncaughtException = { + WINPTY_ERROR_UNSPECIFIED, + L"Uncaught C++ exception", + nullptr +}; + +/* Gets the error code from the error object. */ +WINPTY_API winpty_result_t winpty_error_code(winpty_error_ptr_t err) { + return err != nullptr ? err->code : WINPTY_ERROR_SUCCESS; } -// Call ConnectNamedPipe and block, even for an overlapped pipe. If the -// pipe is overlapped, create a temporary event for use connecting. -static bool connectNamedPipe(HANDLE handle, bool overlapped) -{ - OVERLAPPED over, *pover = NULL; - if (overlapped) { - pover = &over; - memset(&over, 0, sizeof(over)); - over.hEvent = CreateEventW(NULL, TRUE, FALSE, NULL); - ASSERT(over.hEvent != NULL); +/* Returns a textual representation of the error. The string is freed when + * the error is freed. */ +WINPTY_API LPCWSTR winpty_error_msg(winpty_error_ptr_t err) { + if (err != nullptr) { + if (err->msgStatic != nullptr) { + return err->msgStatic; + } else { + ASSERT(err->msgDynamic != nullptr); + std::wstring *msgPtr = err->msgDynamic->get(); + ASSERT(msgPtr != nullptr); + return msgPtr->c_str(); + } + } else { + return L"Success"; + } +} + +/* Free the error object. Every error returned from the winpty API must be + * freed. */ +WINPTY_API void winpty_error_free(winpty_error_ptr_t err) { + if (err != nullptr && err->msgDynamic != nullptr) { + delete err->msgDynamic; + delete err; + } +} + +static void translateException(winpty_error_ptr_t *&err) { + winpty_error_ptr_t ret = nullptr; + try { + try { + throw; + } catch (const ReadBuffer::DecodeError &e) { + ret = const_cast(&kBadRpcPacket); + } catch (const LibWinptyException &e) { + std::unique_ptr obj(new winpty_error_t); + obj->code = e.code(); + obj->msgStatic = nullptr; + obj->msgDynamic = + new std::shared_ptr(e.whatSharedStr()); + ret = obj.release(); + } catch (const WinptyException &e) { + std::unique_ptr obj(new winpty_error_t); + std::shared_ptr msg(new std::wstring(e.what())); + obj->code = WINPTY_ERROR_UNSPECIFIED; + obj->msgStatic = nullptr; + obj->msgDynamic = new std::shared_ptr(msg); + ret = obj.release(); + } + } catch (const std::bad_alloc &e) { + ret = const_cast(&kOutOfMemory); + } catch (...) { + ret = const_cast(&kUncaughtException); } - bool success = ConnectNamedPipe(handle, pover); - if (overlapped && !success && GetLastError() == ERROR_IO_PENDING) { - DWORD actual; - success = GetOverlappedResult(handle, pover, &actual, TRUE); + trace("libwinpty error: code=%u msg='%s'", + static_cast(ret->code), + utf8FromWide(winpty_error_msg(ret)).c_str()); + if (err != nullptr) { + *err = ret; + } else { + winpty_error_free(ret); } - if (!success && GetLastError() == ERROR_PIPE_CONNECTED) +} + +#define API_TRY \ + if (err != nullptr) { *err = nullptr; } \ + try + +#define API_CATCH(ret) \ + catch (...) { translateException(err); return (ret); } + + + +/***************************************************************************** + * Configuration of a new agent. */ + +WINPTY_API winpty_config_t * +winpty_config_new(UINT64 flags, winpty_error_ptr_t *err /*OPTIONAL*/) { + API_TRY { + ASSERT((flags & WINPTY_FLAG_MASK) == flags); + std::unique_ptr ret(new winpty_config_t); + ret->flags = flags; + return ret.release(); + } API_CATCH(nullptr) +} + +WINPTY_API void winpty_config_free(winpty_config_t *cfg) { + delete cfg; +} + +WINPTY_API BOOL +winpty_config_set_initial_size(winpty_config_t *cfg, int cols, int rows, + winpty_error_ptr_t *err /*OPTIONAL*/) { + API_TRY { + ASSERT(cfg != nullptr && cols > 0 && rows > 0); + cfg->cols = cols; + cfg->rows = rows; + return TRUE; + } API_CATCH(FALSE); +} + +WINPTY_API BOOL +winpty_config_set_agent_timeout(winpty_config_t *cfg, DWORD timeoutMs, + winpty_error_ptr_t *err /*OPTIONAL*/) { + API_TRY { + ASSERT(cfg != nullptr && timeoutMs > 0); + cfg->timeoutMs = timeoutMs; + return TRUE; + } API_CATCH(FALSE) +} + + + +/***************************************************************************** + * Agent I/O. */ + +namespace { + +// Once an I/O operation fails with ERROR_IO_PENDING, the caller *must* wait +// for it to complete, even after calling CancelIo on it! See +// https://blogs.msdn.microsoft.com/oldnewthing/20110202-00/?p=11613. This +// class enforces that requirement. +class PendingIo { + HANDLE m_file; + OVERLAPPED &m_over; + bool m_finished; +public: + // The file handle and OVERLAPPED object must live as long as the PendingIo + // object. + PendingIo(HANDLE file, OVERLAPPED &over) : + m_file(file), m_over(over), m_finished(false) {} + ~PendingIo() { + if (!m_finished) { + // We're not usually that interested in CancelIo's return value. + // In any case, we must not throw an exception in this dtor. + CancelIo(&m_over); + waitForCompletion(); + } + } + BOOL waitForCompletion(DWORD &actual) WINPTY_NOEXCEPT { + m_finished = true; + return GetOverlappedResult(m_file, &m_over, &actual, TRUE); + } + BOOL waitForCompletion() WINPTY_NOEXCEPT { + DWORD actual = 0; + return waitForCompletion(actual); + } +}; + +} // anonymous namespace + +static void handlePendingIo(winpty_t &wp, OVERLAPPED &over, BOOL &success, + DWORD &actual) { + if (!success && GetLastError() == ERROR_IO_PENDING) { + PendingIo io(wp.controlPipe.get(), over); + const HANDLE waitHandles[2] = { wp.ioEvent.get(), + wp.agentProcess.get() }; + DWORD waitRet = WaitForMultipleObjects( + 2, waitHandles, FALSE, wp.agentTimeoutMs); + // TODO: interesting edge case to test; what if the client + // disconnects after we wake up and before we call + // GetOverlappedResult? I predict either: + // - the connect succeeds + // - the connect fails with ERROR_BROKEN_PIPE + if (waitRet != WAIT_OBJECT_0) { + // The I/O is still pending. Cancel it, close the I/O event, and + // throw an exception. + if (waitRet == WAIT_OBJECT_0 + 1) { + throw LibWinptyException(WINPTY_ERROR_AGENT_DIED, L"agent died"); + } else if (waitRet == WAIT_TIMEOUT) { + throw LibWinptyException(WINPTY_ERROR_AGENT_TIMEOUT, + L"agent timed out"); + } else if (waitRet == WAIT_FAILED) { + throwWindowsError(L"WaitForMultipleObjects failed"); + } else { + ASSERT(false && + "unexpected WaitForMultipleObjects return value"); + } + } + success = io.waitForCompletion(actual); + } +} + +static void handlePendingIo(winpty_t &wp, OVERLAPPED &over, BOOL &success) { + DWORD actual = 0; + handlePendingIo(wp, over, success, actual); +} + +static void handleReadWriteErrors(winpty_t &wp, BOOL success, + const wchar_t *genericErrMsg) { + if (!success) { + // TODO: We failed during the write. We *probably* should permanently + // shut things down, disconnect at least the control pipe. + // TODO: Which errors, *specifically*, do we care about? + const DWORD lastError = GetLastError(); + if (lastError == ERROR_BROKEN_PIPE || lastError == ERROR_NO_DATA || + lastError == ERROR_PIPE_NOT_CONNECTED) { + throw LibWinptyException(WINPTY_ERROR_LOST_CONNECTION, + L"lost connection to agent"); + } else { + throwWindowsError(genericErrMsg); + } + } +} + +// Calls ConnectNamedPipe to wait until the agent connects to the control pipe. +static void +connectControlPipe(winpty_t &wp) { + OVERLAPPED over = {}; + over.hEvent = wp.ioEvent.get(); + BOOL success = ConnectNamedPipe(wp.controlPipe.get(), &over); + handlePendingIo(wp, over, success); + if (!success && GetLastError() == ERROR_PIPE_CONNECTED) { success = TRUE; - if (overlapped) - CloseHandle(over.hEvent); - return success; + } + if (!success) { + throwWindowsError(L"ConnectNamedPipe failed"); + } } -static inline WriteBuffer newPacket() -{ +static void writeData(winpty_t &wp, const void *data, size_t amount) { + // Perform a single pipe write. + DWORD actual = 0; + OVERLAPPED over = {}; + over.hEvent = wp.ioEvent.get(); + BOOL success = WriteFile(wp.controlPipe.get(), data, amount, + &actual, &over); + if (!success) { + handlePendingIo(wp, over, success, actual); + handleReadWriteErrors(wp, success, L"WriteFile failed"); + ASSERT(success); + } + // TODO: Can a partial write actually happen somehow? + ASSERT(actual == amount && "WriteFile wrote fewer bytes than requested"); +} + +static inline WriteBuffer newPacket() { WriteBuffer packet; packet.putRawValue(0); // Reserve space for size. return packet; } -static void writePacket(winpty_t *pc, WriteBuffer &packet) -{ - packet.replaceRawValue(0, packet.buf().size()); +static void writePacket(winpty_t &wp, WriteBuffer &packet) { const auto &buf = packet.buf(); + packet.replaceRawValue(0, buf.size()); + writeData(wp, buf.data(), buf.size()); +} + +static size_t readData(winpty_t &wp, void *data, size_t amount) { DWORD actual = 0; - ASSERT(buf.size() <= std::numeric_limits::max()); - const BOOL success = WriteFile(pc->controlPipe, buf.data(), buf.size(), - &actual, nullptr); - ASSERT(success && actual == buf.size()); + OVERLAPPED over = {}; + over.hEvent = wp.ioEvent.get(); + BOOL success = ReadFile(wp.controlPipe.get(), data, amount, + &actual, &over); + if (!success) { + handlePendingIo(wp, over, success, actual); + handleReadWriteErrors(wp, success, L"ReadFile failed"); + } + return actual; } -static int32_t readInt32(winpty_t *pc) -{ - int32_t result; - DWORD actual; - BOOL success = ReadFile(pc->controlPipe, &result, sizeof(int32_t), &actual, NULL); - ASSERT(success && actual == sizeof(int32_t)); - return result; +static void readAll(winpty_t &wp, void *data, size_t amount) { + while (amount > 0) { + size_t chunk = readData(wp, data, amount); + data = reinterpret_cast(data) + chunk; + amount -= chunk; + } } -static HANDLE createNamedPipe(const std::wstring &name, bool overlapped) -{ - try { - const auto sd = createPipeSecurityDescriptorOwnerFullControl(); - SECURITY_ATTRIBUTES sa = {}; - sa.nLength = sizeof(sa); - sa.lpSecurityDescriptor = sd.get(); - return CreateNamedPipeW(name.c_str(), - /*dwOpenMode=*/ - PIPE_ACCESS_DUPLEX | - FILE_FLAG_FIRST_PIPE_INSTANCE | - (overlapped ? FILE_FLAG_OVERLAPPED : 0), - /*dwPipeMode=*/ - rejectRemoteClientsPipeFlag(), - /*nMaxInstances=*/1, - /*nOutBufferSize=*/0, - /*nInBufferSize=*/0, - /*nDefaultTimeOut=*/3000, - &sa); - } catch (const WinptyException &e) { - trace("createNamedPipe: exception thrown: %s", - utf8FromWide(e.what()).c_str()); - return INVALID_HANDLE_VALUE; +static uint64_t readUInt64(winpty_t &wp) { + uint64_t ret = 0; + readAll(wp, &ret, sizeof(ret)); + return ret; +} + +// Returns a reply packet's payload. +static ReadBuffer readPacket(winpty_t &wp) { + const uint64_t packetSize = readUInt64(wp); + if (packetSize < sizeof(packetSize) || packetSize > SIZE_MAX) { + throwWinptyException(L"Agent RPC error: invalid packet size"); + } + const size_t payloadSize = packetSize - sizeof(packetSize); + std::vector bytes(payloadSize); + readAll(wp, bytes.data(), bytes.size()); + return ReadBuffer(std::move(bytes)); +} + +static OwnedHandle createControlPipe(const std::wstring &name) { + const auto sd = createPipeSecurityDescriptorOwnerFullControl(); + if (!sd) { + throwWinptyException( + L"could not create the control pipe's SECURITY_DESCRIPTOR"); + } + SECURITY_ATTRIBUTES sa = {}; + sa.nLength = sizeof(sa); + sa.lpSecurityDescriptor = sd.get(); + HANDLE ret = CreateNamedPipeW(name.c_str(), + /*dwOpenMode=*/ + PIPE_ACCESS_DUPLEX | + FILE_FLAG_FIRST_PIPE_INSTANCE | + FILE_FLAG_OVERLAPPED, + /*dwPipeMode=*/rejectRemoteClientsPipeFlag(), + /*nMaxInstances=*/1, + /*nOutBufferSize=*/8192, + /*nInBufferSize=*/256, + /*nDefaultTimeOut=*/30000, + &sa); + if (ret == INVALID_HANDLE_VALUE) { + throwWindowsError(L"CreateNamedPipeW failed"); + } + return OwnedHandle(ret); +} + + + +/***************************************************************************** + * Start the agent. */ + +static OwnedHandle createEvent() { + // manual reset, initially unset + HANDLE h = CreateEventW(nullptr, TRUE, FALSE, nullptr); + if (h == nullptr) { + throwWindowsError(L"CreateEventW failed"); } + return OwnedHandle(h); } struct BackgroundDesktop { @@ -146,8 +415,7 @@ BackgroundDesktop::BackgroundDesktop() : { } -static std::wstring getObjectName(HANDLE object) -{ +static std::wstring getObjectName(HANDLE object) { BOOL success; DWORD lengthNeeded = 0; GetUserObjectInformationW(object, UOI_NAME, @@ -166,8 +434,7 @@ static std::wstring getObjectName(HANDLE object) // For debugging purposes, provide a way to keep the console on the main window // station, visible. -static bool shouldShowConsoleWindow() -{ +static bool shouldShowConsoleWindow() { char buf[32]; return GetEnvironmentVariableA("WINPTY_SHOW_CONSOLE", buf, sizeof(buf)) > 0; } @@ -206,8 +473,7 @@ static bool shouldCreateBackgroundDesktop() { // Get a non-interactive window station for the agent. // TODO: review security w.r.t. windowstation and desktop. -static BackgroundDesktop setupBackgroundDesktop() -{ +static BackgroundDesktop setupBackgroundDesktop() { BackgroundDesktop ret; if (shouldCreateBackgroundDesktop()) { const HWINSTA originalStation = GetProcessWindowStation(); @@ -231,8 +497,7 @@ static BackgroundDesktop setupBackgroundDesktop() return ret; } -static void restoreOriginalDesktop(const BackgroundDesktop &desktop) -{ +static void restoreOriginalDesktop(const BackgroundDesktop &desktop) { if (desktop.station != NULL) { SetProcessWindowStation(desktop.originalStation); CloseDesktop(desktop.desktop); @@ -240,8 +505,7 @@ static void restoreOriginalDesktop(const BackgroundDesktop &desktop) } } -static std::wstring getDesktopFullName() -{ +static std::wstring getDesktopFullName() { // MSDN says that the handle returned by GetThreadDesktop does not need // to be passed to CloseDesktop. HWINSTA station = GetProcessWindowStation(); @@ -265,19 +529,17 @@ static bool shouldSpecifyHideFlag() { return ret; } -static void startAgentProcess(const BackgroundDesktop &desktop, - const std::wstring &controlPipeName, - const std::wstring &dataPipeName, - int cols, int rows, - HANDLE &agentProcess, - DWORD &agentPid) -{ +static OwnedHandle startAgentProcess( + const BackgroundDesktop &desktop, + const std::wstring &controlPipeName, + uint64_t flags, int cols, int rows, + DWORD &agentPid) { const std::wstring exePath = findAgentProgram(); const std::wstring cmdline = (WStringBuilder(256) << L"\"" << exePath << L"\" " - << controlPipeName << L' ' << dataPipeName << L' ' - << cols << L' ' << rows).str_moved(); + << controlPipeName << L' ' + << flags << L' ' << cols << L' ' << rows).str_moved(); auto cmdlineV = vectorWithNulFromString(cmdline); auto desktopV = vectorWithNulFromString(desktop.desktopName); @@ -285,7 +547,7 @@ static void startAgentProcess(const BackgroundDesktop &desktop, // Start the agent. STARTUPINFOW sui = {}; sui.cb = sizeof(sui); - sui.lpDesktop = desktop.station == NULL ? NULL : desktopV.data(); + sui.lpDesktop = desktop.station == nullptr ? nullptr : desktopV.data(); if (shouldSpecifyHideFlag()) { sui.dwFlags |= STARTF_USESHOWWINDOW; sui.wShowWindow = SW_HIDE; @@ -294,146 +556,133 @@ static void startAgentProcess(const BackgroundDesktop &desktop, const BOOL success = CreateProcessW(exePath.c_str(), cmdlineV.data(), - NULL, NULL, + nullptr, nullptr, /*bInheritHandles=*/FALSE, /*dwCreationFlags=*/CREATE_NEW_CONSOLE, - NULL, NULL, + nullptr, nullptr, &sui, &pi); - if (success) { - trace("Created agent successfully, pid=%u, cmdline=%s", - static_cast(pi.dwProcessId), - utf8FromWide(cmdline).c_str()); - } else { - unsigned int err = GetLastError(); - trace("Error creating agent, err=%#x, cmdline=%s", - err, utf8FromWide(cmdline).c_str()); - fprintf(stderr, "Error %#x starting %s\n", err, - utf8FromWide(cmdline).c_str()); - exit(1); + if (!success) { + const DWORD lastError = GetLastError(); + const auto errStr = + (WStringBuilder(256) + << L"winpty-agent CreateProcess failed: cmdline='" << cmdline + << L"' err=0x" << whexOfInt(lastError)).str_moved(); + throw LibWinptyException( + WINPTY_ERROR_AGENT_CREATION_FAILED, errStr.c_str()); } - CloseHandle(pi.hThread); - - agentProcess = pi.hProcess; + trace("Created agent successfully, pid=%u, cmdline=%s", + static_cast(pi.dwProcessId), + utf8FromWide(cmdline).c_str()); agentPid = pi.dwProcessId; + return OwnedHandle(pi.hProcess); } -static bool verifyPipeClientPid(HANDLE serverPipe, DWORD agentPid) -{ +static void verifyPipeClientPid(HANDLE serverPipe, DWORD agentPid) { const auto client = getNamedPipeClientProcessId(serverPipe); const auto err = GetLastError(); const auto success = std::get<0>(client); if (success == GetNamedPipeClientProcessId_Result::Success) { const auto clientPid = std::get<1>(client); if (clientPid != agentPid) { - trace("Security check failed: pipe client pid (%u) does not " - "match agent pid (%u)", - static_cast(clientPid), - static_cast(agentPid)); - return false; + WStringBuilder errMsg; + errMsg << L"Security check failed: pipe client pid (" << clientPid + << L") does not match agent pid (" << agentPid << L")"; + throwWinptyException(errMsg.c_str()); } - return true; } else if (success == GetNamedPipeClientProcessId_Result::UnsupportedOs) { trace("Pipe client PID security check skipped: " "GetNamedPipeClientProcessId unsupported on this OS version"); - return true; } else { - trace("GetNamedPipeClientProcessId failed: %u", - static_cast(err)); - return false; + throwWindowsError(L"GetNamedPipeClientProcessId failed", err); } } -WINPTY_API winpty_t *winpty_open(int cols, int rows) -{ - dumpWindowsVersion(); - dumpVersionToTrace(); - - winpty_t *pc = new winpty_t; - - // Start pipes. - const auto basePipeName = - L"\\\\.\\pipe\\winpty-" + GenRandom().uniqueName(); - const std::wstring controlPipeName = basePipeName + L"-control"; - const std::wstring dataPipeName = basePipeName + L"-data"; - pc->controlPipe = createNamedPipe(controlPipeName, false); - if (pc->controlPipe == INVALID_HANDLE_VALUE) { - delete pc; - return NULL; - } - pc->dataPipe = createNamedPipe(dataPipeName, true); - if (pc->dataPipe == INVALID_HANDLE_VALUE) { - delete pc; - return NULL; - } +WINPTY_API winpty_t * +winpty_open(const winpty_config_t *cfg, + winpty_error_ptr_t *err /*OPTIONAL*/) { + API_TRY { + ASSERT(cfg != nullptr); + dumpWindowsVersion(); + dumpVersionToTrace(); + + std::unique_ptr wp(new winpty_t); + wp->agentTimeoutMs = cfg->timeoutMs; + wp->ioEvent = createEvent(); + + // Create control server pipe. + const auto pipeName = + L"\\\\.\\pipe\\winpty-control-" + GenRandom().uniqueName(); + wp->controlPipe = createControlPipe(pipeName); + + // Setup a background desktop for the agent. + // TODO: Respect WINPTY_FLAG_ALLOW_CURPROC_DESKTOP_CREATION for XP/Vista. + BackgroundDesktop desktop = setupBackgroundDesktop(); + + // Start the agent and connect the control pipe. + DWORD agentPid = 0; + wp->agentProcess = startAgentProcess( + desktop, pipeName, cfg->flags, cfg->cols, cfg->rows, + agentPid); + connectControlPipe(*wp.get()); + + // Close handles to the background desktop and restore the original + // window station. This must wait until we know the agent is running + // -- if we close these handles too soon, then the desktop and + // windowstation will be destroyed before the agent can connect with + // them. + restoreOriginalDesktop(desktop); + + verifyPipeClientPid(wp->controlPipe.get(), agentPid); + + // Get the CONIN/CONOUT pipe names. + auto packet = readPacket(*wp.get()); + wp->coninPipeName = packet.getWString(); + wp->conoutPipeName = packet.getWString(); + packet.assertEof(); + + return wp.release(); + } API_CATCH(nullptr) +} - // Setup a background desktop for the agent. - BackgroundDesktop desktop = setupBackgroundDesktop(); +WINPTY_API HANDLE winpty_agent_process(winpty_t *wp) { + ASSERT(wp != nullptr); + return wp->agentProcess.get(); +} - // Start the agent. - HANDLE agentProcess = NULL; - DWORD agentPid = INFINITE; - startAgentProcess(desktop, controlPipeName, dataPipeName, cols, rows, - agentProcess, agentPid); - OwnedHandle autoClose(agentProcess); - - // TODO: Frequently, I see the CreateProcess call return successfully, - // but the agent immediately dies. The following pipe connect calls then - // hang. These calls should probably timeout. Maybe this code could also - // poll the agent process handle? - - // Connect the pipes. - bool success; - success = connectNamedPipe(pc->controlPipe, false); - if (!success) { - delete pc; - return NULL; - } - success = connectNamedPipe(pc->dataPipe, true); - if (!success) { - delete pc; - return NULL; - } - // Close handles to the background desktop and restore the original window - // station. This must wait until we know the agent is running -- if we - // close these handles too soon, then the desktop and windowstation will be - // destroyed before the agent can connect with them. - restoreOriginalDesktop(desktop); - - // Check that the pipe clients are correct. - if (!verifyPipeClientPid(pc->controlPipe, agentPid) || - !verifyPipeClientPid(pc->dataPipe, agentPid)) { - delete pc; - return NULL; - } - // TODO: This comment is now out-of-date. The named pipes now have a DACL - // that should prevent arbitrary users from connecting, even just to read. - // - // The default security descriptor for a named pipe allows anyone to connect - // to the pipe to read, but not to write. Only the "creator owner" and - // various system accounts can write to the pipe. By sending and receiving - // a dummy message on the control pipe, we should confirm that something - // trusted (i.e. the agent we just started) successfully connected and wrote - // to one of our pipes. - auto packet = newPacket(); - packet.putInt32(AgentMsg::Ping); - writePacket(pc, packet); - if (readInt32(pc) != 0) { - delete pc; - return NULL; +/***************************************************************************** + * I/O pipes. */ + +static const wchar_t *cstrFromWStringOrNull(const std::wstring &str) { + try { + return str.c_str(); + } catch (const std::bad_alloc &e) { + return nullptr; } +} + +WINPTY_API LPCWSTR winpty_conin_name(winpty_t *wp) { + ASSERT(wp != nullptr); + return cstrFromWStringOrNull(wp->coninPipeName); +} - return pc; +WINPTY_API LPCWSTR winpty_conout_name(winpty_t *wp) { + ASSERT(wp != nullptr); + return cstrFromWStringOrNull(wp->conoutPipeName); } + + +/***************************************************************************** + * winpty agent RPC call: process creation. */ + // Return a std::wstring containing every character of the environment block. // Typically, the block is non-empty, so the std::wstring returned ends with // two NUL terminators. (These two terminators are counted in size(), so // calling c_str() produces a triply-terminated string.) -static std::wstring wstringFromEnvBlock(const wchar_t *env) -{ +static std::wstring wstringFromEnvBlock(const wchar_t *env) { std::wstring envStr; if (env != NULL) { const wchar_t *p = env; @@ -464,66 +713,130 @@ static std::wstring wstringFromEnvBlock(const wchar_t *env) return envStr; } -WINPTY_API int winpty_start_process(winpty_t *pc, - const wchar_t *appname, - const wchar_t *cmdline, - const wchar_t *cwd, - const wchar_t *env) -{ - auto packet = newPacket(); - packet.putInt32(AgentMsg::StartProcess); - packet.putWString(appname ? appname : L""); - packet.putWString(cmdline ? cmdline : L""); - packet.putWString(cwd ? cwd : L""); - packet.putWString(wstringFromEnvBlock(env)); - packet.putWString(getDesktopFullName()); - writePacket(pc, packet); - return readInt32(pc); -} - -WINPTY_API int winpty_get_exit_code(winpty_t *pc) -{ - auto packet = newPacket(); - packet.putInt32(AgentMsg::GetExitCode); - writePacket(pc, packet); - return readInt32(pc); +WINPTY_API winpty_spawn_config_t * +winpty_spawn_config_new(UINT64 winptyFlags, + LPCWSTR appname /*OPTIONAL*/, + LPCWSTR cmdline /*OPTIONAL*/, + LPCWSTR cwd /*OPTIONAL*/, + LPCWSTR env /*OPTIONAL*/, + winpty_error_ptr_t *err /*OPTIONAL*/) { + API_TRY { + ASSERT((winptyFlags & WINPTY_SPAWN_FLAG_MASK) == winptyFlags); + std::unique_ptr cfg(new winpty_spawn_config_t); + cfg->winptyFlags = winptyFlags; + if (appname != nullptr) { cfg->appname = appname; } + if (cmdline != nullptr) { cfg->cmdline = cmdline; } + if (cwd != nullptr) { cfg->cwd = cwd; } + if (env != nullptr) { cfg->env = wstringFromEnvBlock(env); } + return cfg.release(); + } API_CATCH(nullptr) } -WINPTY_API int winpty_get_process_id(winpty_t *pc) -{ - auto packet = newPacket(); - packet.putInt32(AgentMsg::GetProcessId); - writePacket(pc, packet); - return readInt32(pc); +WINPTY_API void winpty_spawn_config_free(winpty_spawn_config_t *cfg) { + delete cfg; } -WINPTY_API HANDLE winpty_get_data_pipe(winpty_t *pc) -{ - return pc->dataPipe; +// It's safe to truncate a handle from 64-bits to 32-bits, or to sign-extend it +// back to 64-bits. See the MSDN article, "Interprocess Communication Between +// 32-bit and 64-bit Applications". +// https://msdn.microsoft.com/en-us/library/windows/desktop/aa384203.aspx +static inline HANDLE handleFromInt64(int64_t i) { + return reinterpret_cast(static_cast(i)); } -WINPTY_API int winpty_set_size(winpty_t *pc, int cols, int rows) -{ - auto packet = newPacket(); - packet.putInt32(AgentMsg::SetSize); - packet.putInt32(cols); - packet.putInt32(rows); - writePacket(pc, packet); - return readInt32(pc); +WINPTY_API BOOL +winpty_spawn(winpty_t *wp, + const winpty_spawn_config_t *cfg, + HANDLE *process_handle /*OPTIONAL*/, + HANDLE *thread_handle /*OPTIONAL*/, + DWORD *create_process_error /*OPTIONAL*/, + winpty_error_ptr_t *err /*OPTIONAL*/) { + API_TRY { + ASSERT(wp != nullptr && cfg != nullptr); + LockGuard lock(wp->mutex); + + if (process_handle != nullptr) { *process_handle = nullptr; } + if (thread_handle != nullptr) { *thread_handle = nullptr; } + if (create_process_error != nullptr) { *create_process_error = 0; } + + // Send spawn request. + auto packet = newPacket(); + packet.putInt32(AgentMsg::StartProcess); + packet.putInt64(cfg->winptyFlags); + packet.putInt32(process_handle != nullptr); + packet.putInt32(thread_handle != nullptr); + packet.putWString(cfg->appname); + packet.putWString(cfg->cmdline); + packet.putWString(cfg->cwd); + packet.putWString(cfg->env); + packet.putWString(getDesktopFullName()); + writePacket(*wp, packet); + + // Receive reply. + auto reply = readPacket(*wp); + const auto success = reply.getInt32(); + const DWORD lastError = reply.getInt32(); + const HANDLE process = handleFromInt64(reply.getInt64()); + const HANDLE thread = handleFromInt64(reply.getInt64()); + reply.assertEof(); + + // TODO: Maybe this is good enough, but there are code paths that leak + // handles... + if (process_handle != nullptr && process != nullptr) { + if (!DuplicateHandle(wp->agentProcess.get(), process, + GetCurrentProcess(), + process_handle, 0, FALSE, + DUPLICATE_CLOSE_SOURCE | DUPLICATE_SAME_ACCESS)) { + // TODO: Shut down the winpty instance? This code path is never supposed to happen... + throwWindowsError(L"DuplicateHandle of process handle"); + } + } + if (thread_handle != nullptr && thread != nullptr) { + if (!DuplicateHandle(wp->agentProcess.get(), thread, + GetCurrentProcess(), + thread_handle, 0, FALSE, + DUPLICATE_CLOSE_SOURCE | DUPLICATE_SAME_ACCESS)) { + // TODO: Shut down the winpty instance? This code path is never supposed to happen... + throwWindowsError(L"DuplicateHandle of thread handle"); + } + } + + if (!success) { + // TODO: include an error number + if (create_process_error != nullptr) { + *create_process_error = lastError; + } + throw LibWinptyException(WINPTY_ERROR_SPAWN_CREATE_PROCESS_FAILED, + L"CreateProcess failed"); + } + return TRUE; + } API_CATCH(FALSE) } -WINPTY_API void winpty_close(winpty_t *pc) -{ - CloseHandle(pc->controlPipe); - CloseHandle(pc->dataPipe); - delete pc; + + +/***************************************************************************** + * winpty agent RPC calls: everything else */ + +WINPTY_API BOOL +winpty_set_size(winpty_t *wp, int cols, int rows, + winpty_error_ptr_t *err /*OPTIONAL*/) { + API_TRY { + ASSERT(wp != nullptr && cols > 0 && rows > 0); + LockGuard lock(wp->mutex); + auto packet = newPacket(); + packet.putInt32(AgentMsg::SetSize); + packet.putInt32(cols); + packet.putInt32(rows); + writePacket(*wp, packet); + readPacket(*wp).assertEof(); + return TRUE; + } API_CATCH(FALSE) } -WINPTY_API int winpty_set_console_mode(winpty_t *pc, int mode) -{ - auto packet = newPacket(); - packet.putInt32(AgentMsg::SetConsoleMode); - packet.putInt32(mode); - writePacket(pc, packet); - return readInt32(pc); +WINPTY_API void winpty_free(winpty_t *wp) { + // At least in principle, CloseHandle can fail, so this deletion can + // fail. It won't throw an exception, but maybe there's an error that + // should be propagated? + delete wp; } diff --git a/src/unix-adapter/Event.h b/src/shared/Mutex.h old mode 100644 new mode 100755 similarity index 52% rename from src/unix-adapter/Event.h rename to src/shared/Mutex.h index 958eb2ec..98215365 --- a/src/unix-adapter/Event.h +++ b/src/shared/Mutex.h @@ -18,41 +18,37 @@ // FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS // IN THE SOFTWARE. -#ifndef UNIX_ADAPTER_EVENT_H -#define UNIX_ADAPTER_EVENT_H +// Recent 4.x MinGW and MinGW-w64 gcc compilers lack std::mutex and +// std::lock_guard. I have a 5.2.0 MinGW-w64 compiler packaged through MSYS2 +// that *is* new enough, but that's one compiler against several deficient +// ones. Wrap CRITICAL_SECTION instead. + +#ifndef WINPTY_SHARED_MUTEX_H +#define WINPTY_SHARED_MUTEX_H #include -#include -// A manual reset, initially unset event. Automatically closes on destruction. -class Event { +class Mutex { + CRITICAL_SECTION m_mutex; +public: + Mutex() { InitializeCriticalSection(&m_mutex); } + ~Mutex() { DeleteCriticalSection(&m_mutex); } + void lock() { EnterCriticalSection(&m_mutex); } + void unlock() { LeaveCriticalSection(&m_mutex); } + + Mutex(const Mutex &other) = delete; + Mutex &operator=(const Mutex &other) = delete; +}; + +template +class LockGuard { + T &m_lock; public: - Event() { - m_handle = CreateEventW(NULL, TRUE, FALSE, NULL); - assert(m_handle != NULL); - } - ~Event() { - CloseHandle(m_handle); - } - HANDLE handle() { - return m_handle; - } - void set() { - BOOL success = SetEvent(m_handle); - assert(success && "SetEvent failed"); - } - void reset() { - BOOL success = ResetEvent(m_handle); - assert(success && "ResetEvent failed"); - } - -private: - // Do not allow copying the Event object. - Event(const Event &other); - Event &operator=(const Event &other); - -private: - HANDLE m_handle; + LockGuard(T &lock) : m_lock(lock) { m_lock.lock(); } + ~LockGuard() { m_lock.unlock(); } + + LockGuard(const LockGuard &other) = delete; + LockGuard &operator=(const LockGuard &other) = delete; }; -#endif // UNIX_ADAPTER_EVENT_H +#endif // WINPTY_SHARED_MUTEX_H diff --git a/src/tests/trivial_test.cc b/src/tests/trivial_test.cc index c542f7ae..76a1dcb5 100644 --- a/src/tests/trivial_test.cc +++ b/src/tests/trivial_test.cc @@ -30,11 +30,6 @@ #include "../include/winpty.h" #include "../shared/DebugClient.h" -// Create a manual reset, initially unset event. -static HANDLE createEvent() { - return CreateEventW(NULL, TRUE, FALSE, NULL); -} - static std::vector filterContent( const std::vector &content) { std::vector result; @@ -62,50 +57,60 @@ static std::vector filterContent( return result; } -// Read bytes from the overlapped file handle until the file is closed or +// Read bytes from the non-overlapped file handle until the file is closed or // until an I/O error occurs. static std::vector readAll(HANDLE handle) { - const HANDLE event = createEvent(); unsigned char buf[1024]; std::vector result; while (true) { - OVERLAPPED over; - memset(&over, 0, sizeof(over)); - over.hEvent = event; DWORD amount = 0; - BOOL ret = ReadFile(handle, buf, sizeof(buf), &amount, &over); - if (!ret && GetLastError() == ERROR_IO_PENDING) - ret = GetOverlappedResult(handle, &over, &amount, TRUE); - if (!ret || amount == 0) + BOOL ret = ReadFile(handle, buf, sizeof(buf), &amount, nullptr); + if (!ret || amount == 0) { break; + } result.insert(result.end(), buf, buf + amount); } - CloseHandle(event); return result; } static void parentTest() { wchar_t program[1024]; wchar_t cmdline[1024]; - GetModuleFileNameW(NULL, program, 1024); + GetModuleFileNameW(nullptr, program, 1024); snwprintf(cmdline, sizeof(cmdline) / sizeof(cmdline[0]), L"\"%s\" CHILD", program); - winpty_t *pty = winpty_open(80, 25); - assert(pty != NULL); - int ret = winpty_start_process(pty, program, cmdline, NULL, NULL); - assert(ret == 0); + auto agentCfg = winpty_config_new(0, nullptr); + assert(agentCfg != nullptr); + auto pty = winpty_open(agentCfg, nullptr); + assert(pty != nullptr); + winpty_config_free(agentCfg); + + HANDLE conout = CreateFileW( + winpty_conout_name(pty), + GENERIC_READ, 0, nullptr, OPEN_EXISTING, 0, nullptr); + assert(conout != INVALID_HANDLE_VALUE); + + auto spawnCfg = winpty_spawn_config_new( + WINPTY_SPAWN_FLAG_AUTO_SHUTDOWN, program, cmdline, + nullptr, nullptr, nullptr); + assert(spawnCfg != nullptr); + HANDLE process = nullptr; + BOOL spawnSuccess = winpty_spawn( + pty, spawnCfg, &process, nullptr, nullptr, nullptr); + assert(spawnSuccess && process != nullptr); - HANDLE input = winpty_get_data_pipe(pty); - auto content = readAll(input); + auto content = readAll(conout); content = filterContent(content); std::vector expectedContent = { 'H', 'I', '\n', 'X', 'Y', '\n' }; - assert(winpty_get_exit_code(pty) == 42); + DWORD exitCode = 0; + assert(GetExitCodeProcess(process, &exitCode) && exitCode == 42); + CloseHandle(process); assert(content == expectedContent); - winpty_close(pty); + winpty_free(pty); } static void childTest() { diff --git a/src/unix-adapter/InputHandler.cc b/src/unix-adapter/InputHandler.cc index 3f1bed85..b3a2284e 100644 --- a/src/unix-adapter/InputHandler.cc +++ b/src/unix-adapter/InputHandler.cc @@ -30,12 +30,11 @@ #include #include "../shared/DebugClient.h" -#include "Event.h" #include "Util.h" #include "WakeupFd.h" -InputHandler::InputHandler(HANDLE winpty, WakeupFd &completionWakeup) : - m_winpty(winpty), +InputHandler::InputHandler(HANDLE conin, WakeupFd &completionWakeup) : + m_conin(conin), m_completionWakeup(completionWakeup), m_threadHasBeenJoined(false), m_shouldShutdown(0), @@ -55,7 +54,6 @@ void InputHandler::shutdown() { } void InputHandler::threadProc() { - Event ioEvent; std::vector buffer(4096); fd_set readfds; FD_ZERO(&readfds); @@ -91,30 +89,10 @@ void InputHandler::threadProc() { break; } - DWORD written; - OVERLAPPED over = {0}; - over.hEvent = ioEvent.handle(); - BOOL ret = WriteFile(m_winpty, + DWORD written = 0; + BOOL ret = WriteFile(m_conin, &buffer[0], numRead, - &written, - &over); - if (!ret && GetLastError() == ERROR_IO_PENDING) { - const HANDLE handles[] = { - ioEvent.handle(), - m_wakeup.handle(), - }; - const DWORD waitRet = - WaitForMultipleObjects(2, handles, FALSE, INFINITE); - if (waitRet == WAIT_OBJECT_0 + 1) { - trace("InputHandler: shutting down, canceling I/O"); - assert(m_shouldShutdown); - CancelIo(m_winpty); - GetOverlappedResult(m_winpty, &over, &written, TRUE); - break; - } - assert(waitRet == WAIT_OBJECT_0); - ret = GetOverlappedResult(m_winpty, &over, &written, TRUE); - } + &written, NULL); if (!ret || written != static_cast(numRead)) { if (!ret && GetLastError() == ERROR_BROKEN_PIPE) { trace("InputHandler: pipe closed: written=%u", diff --git a/src/unix-adapter/InputHandler.h b/src/unix-adapter/InputHandler.h index 61d5cf5b..0afd9303 100644 --- a/src/unix-adapter/InputHandler.h +++ b/src/unix-adapter/InputHandler.h @@ -25,13 +25,12 @@ #include #include -#include "DualWakeup.h" #include "WakeupFd.h" -// Connect Cygwin blocking tty STDIN_FILENO to winpty overlapped I/O. +// Connect Cygwin blocking tty STDIN_FILENO to winpty CONIN. class InputHandler { public: - InputHandler(HANDLE winpty, WakeupFd &completionWakeup); + InputHandler(HANDLE conin, WakeupFd &completionWakeup); ~InputHandler() { shutdown(); } bool isComplete() { return m_threadCompleted; } void startShutdown() { m_shouldShutdown = 1; m_wakeup.set(); } @@ -44,10 +43,10 @@ class InputHandler { } void threadProc(); - HANDLE m_winpty; + HANDLE m_conin; pthread_t m_thread; WakeupFd &m_completionWakeup; - DualWakeup m_wakeup; + WakeupFd m_wakeup; bool m_threadHasBeenJoined; volatile sig_atomic_t m_shouldShutdown; volatile sig_atomic_t m_threadCompleted; diff --git a/src/unix-adapter/OutputHandler.cc b/src/unix-adapter/OutputHandler.cc index 64b5163c..b9ce52ef 100644 --- a/src/unix-adapter/OutputHandler.cc +++ b/src/unix-adapter/OutputHandler.cc @@ -29,15 +29,13 @@ #include #include "../shared/DebugClient.h" -#include "Event.h" #include "Util.h" #include "WakeupFd.h" -OutputHandler::OutputHandler(HANDLE winpty, WakeupFd &completionWakeup) : - m_winpty(winpty), +OutputHandler::OutputHandler(HANDLE conout, WakeupFd &completionWakeup) : + m_conout(conout), m_completionWakeup(completionWakeup), m_threadHasBeenJoined(false), - m_shouldShutdown(0), m_threadCompleted(0) { assert(isatty(STDOUT_FILENO)); @@ -45,7 +43,6 @@ OutputHandler::OutputHandler(HANDLE winpty, WakeupFd &completionWakeup) : } void OutputHandler::shutdown() { - startShutdown(); if (!m_threadHasBeenJoined) { int ret = pthread_join(m_thread, NULL); assert(ret == 0 && "pthread_join failed"); @@ -54,41 +51,12 @@ void OutputHandler::shutdown() { } void OutputHandler::threadProc() { - Event ioEvent; std::vector buffer(4096); while (true) { - // Handle shutdown - m_wakeup.reset(); - if (m_shouldShutdown) { - trace("OutputHandler: shutting down"); - break; - } - - // Read from the pipe. - DWORD numRead; - OVERLAPPED over = {0}; - over.hEvent = ioEvent.handle(); - BOOL ret = ReadFile(m_winpty, + DWORD numRead = 0; + BOOL ret = ReadFile(m_conout, &buffer[0], buffer.size(), - &numRead, - &over); - if (!ret && GetLastError() == ERROR_IO_PENDING) { - const HANDLE handles[] = { - ioEvent.handle(), - m_wakeup.handle(), - }; - const DWORD waitRet = - WaitForMultipleObjects(2, handles, FALSE, INFINITE); - if (waitRet == WAIT_OBJECT_0 + 1) { - trace("OutputHandler: shutting down, canceling I/O"); - assert(m_shouldShutdown); - CancelIo(m_winpty); - GetOverlappedResult(m_winpty, &over, &numRead, TRUE); - break; - } - assert(waitRet == WAIT_OBJECT_0); - ret = GetOverlappedResult(m_winpty, &over, &numRead, TRUE); - } + &numRead, NULL); if (!ret || numRead == 0) { if (!ret && GetLastError() == ERROR_BROKEN_PIPE) { trace("OutputHandler: pipe closed: numRead=%u", diff --git a/src/unix-adapter/OutputHandler.h b/src/unix-adapter/OutputHandler.h index 10ff5ebf..e8c65a69 100644 --- a/src/unix-adapter/OutputHandler.h +++ b/src/unix-adapter/OutputHandler.h @@ -25,16 +25,14 @@ #include #include -#include "Event.h" #include "WakeupFd.h" -// Connect winpty overlapped I/O to Cygwin blocking STDOUT_FILENO. +// Connect winpty CONOUT to Cygwin blocking STDOUT_FILENO. class OutputHandler { public: - OutputHandler(HANDLE winpty, WakeupFd &completionWakeup); + OutputHandler(HANDLE conout, WakeupFd &completionWakeup); ~OutputHandler() { shutdown(); } bool isComplete() { return m_threadCompleted; } - void startShutdown() { m_shouldShutdown = 1; m_wakeup.set(); } void shutdown(); private: @@ -44,12 +42,10 @@ class OutputHandler { } void threadProc(); - HANDLE m_winpty; + HANDLE m_conout; pthread_t m_thread; WakeupFd &m_completionWakeup; - Event m_wakeup; bool m_threadHasBeenJoined; - volatile sig_atomic_t m_shouldShutdown; volatile sig_atomic_t m_threadCompleted; }; diff --git a/src/unix-adapter/main.cc b/src/unix-adapter/main.cc index 863f6349..e7cf4f64 100644 --- a/src/unix-adapter/main.cc +++ b/src/unix-adapter/main.cc @@ -252,6 +252,17 @@ static char *heapWcsToMbs(const wchar_t *text) } } +static std::string wcsToMbs(const wchar_t *text) +{ + std::string ret; + const char *ptr = heapWcsToMbs(text); + if (ptr != NULL) { + ret = ptr; + delete [] ptr; + } + return ret; +} + void setupWin32Environment() { std::map varsToCopy; @@ -364,7 +375,7 @@ static void parseArguments(int argc, char *argv[], Arguments &out) } } -static std::string formatErrorMessage(DWORD err) +static std::string errorMessageToString(DWORD err) { // Use FormatMessageW rather than FormatMessageA, because we want to use // wcstombs to convert to the Cygwin locale, which might not match the @@ -385,13 +396,8 @@ static std::string formatErrorMessage(DWORD err) if (formatRet == 0 || wideMsgPtr == NULL) { return std::string(); } - char *const msgPtr = heapWcsToMbs(wideMsgPtr); + std::string msg = wcsToMbs(wideMsgPtr); LocalFree(wideMsgPtr); - if (msgPtr == NULL) { - return std::string(); - } - std::string msg = msgPtr; - delete [] msgPtr; const size_t pos = msg.find_last_not_of(" \r\n\t"); if (pos == std::string::npos) { msg.clear(); @@ -401,6 +407,21 @@ static std::string formatErrorMessage(DWORD err) return msg; } +static std::string formatErrorMessage(DWORD err) +{ + char buf[64]; + sprintf(buf, "error %#x", static_cast(err)); + std::string ret = errorMessageToString(err); + if (ret.empty()) { + ret += buf; + } else { + ret += " ("; + ret += buf; + ret += ")"; + } + return ret; +} + int main(int argc, char *argv[]) { setlocale(LC_ALL, ""); @@ -415,36 +436,62 @@ int main(int argc, char *argv[]) winsize sz; ioctl(STDIN_FILENO, TIOCGWINSZ, &sz); - winpty_t *winpty = winpty_open(sz.ws_col, sz.ws_row); - if (winpty == NULL) { - fprintf(stderr, "Error creating winpty.\n"); + winpty_config_t *agentCfg = winpty_config_new(0, NULL); + assert(agentCfg != NULL); + winpty_config_set_initial_size(agentCfg, sz.ws_col, sz.ws_row, NULL); + + winpty_error_ptr_t openErr = NULL; + winpty_t *wp = winpty_open(agentCfg, &openErr); + if (wp == NULL) { + fprintf(stderr, "Error creating winpty: %s\n", + wcsToMbs(winpty_error_msg(openErr)).c_str()); exit(1); } + winpty_config_free(agentCfg); + winpty_error_free(openErr); + + const wchar_t *coninName = winpty_conin_name(wp); + const wchar_t *conoutName = winpty_conout_name(wp); + HANDLE conin = + CreateFileW(coninName, GENERIC_WRITE, 0, NULL, OPEN_EXISTING, 0, NULL); + HANDLE conout = + CreateFileW(conoutName, GENERIC_READ, 0, NULL, OPEN_EXISTING, 0, NULL); + assert(conin != INVALID_HANDLE_VALUE); + assert(conout != INVALID_HANDLE_VALUE); + + HANDLE childHandle = NULL; { // Start the child process under the console. args.childArgv[0] = convertPosixPathToWin(args.childArgv[0]); std::string cmdLine = argvToCommandLine(args.childArgv); wchar_t *cmdLineW = heapMbsToWcs(cmdLine.c_str()); - const int ret = winpty_start_process(winpty, - NULL, - cmdLineW, - NULL, - NULL); - if (ret != 0) { - const std::string errorMsg = formatErrorMessage(ret); - if (!errorMsg.empty()) { - fprintf(stderr, "Could not start '%s': %s (error %#x)\n", + + winpty_spawn_config_t *spawnCfg = winpty_spawn_config_new( + WINPTY_SPAWN_FLAG_AUTO_SHUTDOWN, + NULL, cmdLineW, NULL, NULL, NULL); + assert(spawnCfg != NULL); + + winpty_error_ptr_t spawnErr = NULL; + DWORD lastError = 0; + BOOL spawnRet = winpty_spawn(wp, spawnCfg, &childHandle, NULL, + &lastError, &spawnErr); + winpty_spawn_config_free(spawnCfg); + + if (!spawnRet) { + winpty_result_t spawnCode = winpty_error_code(spawnErr); + if (spawnCode == WINPTY_ERROR_SPAWN_CREATE_PROCESS_FAILED) { + fprintf(stderr, "Could not start '%s': %s\n", cmdLine.c_str(), - errorMsg.c_str(), - static_cast(ret)); + formatErrorMessage(lastError).c_str()); } else { - fprintf(stderr, "Could not start '%s': error %#x\n", + fprintf(stderr, "Could not start '%s': internal error: %s\n", cmdLine.c_str(), - static_cast(ret)); + wcsToMbs(winpty_error_msg(spawnErr)).c_str()); } exit(1); } + winpty_error_free(spawnErr); delete [] cmdLineW; } @@ -472,8 +519,8 @@ int main(int argc, char *argv[]) CSI"?1000h" CSI"?1002h" CSI"?1003h" CSI"?1015h" CSI"?1006h"); } - OutputHandler outputHandler(winpty_get_data_pipe(winpty), mainWakeup()); - InputHandler inputHandler(winpty_get_data_pipe(winpty), mainWakeup()); + InputHandler inputHandler(conin, mainWakeup()); + OutputHandler outputHandler(conout, mainWakeup()); while (true) { fd_set readfds; @@ -488,21 +535,26 @@ int main(int argc, char *argv[]) ioctl(STDIN_FILENO, TIOCGWINSZ, &sz2); if (memcmp(&sz, &sz2, sizeof(sz)) != 0) { sz = sz2; - winpty_set_size(winpty, sz.ws_col, sz.ws_row); + winpty_set_size(wp, sz.ws_col, sz.ws_row, NULL); } } // Check for an I/O handler shutting down (possibly indicating that the // child process has exited). - if (outputHandler.isComplete() || inputHandler.isComplete()) { + if (inputHandler.isComplete() || outputHandler.isComplete()) { break; } } - outputHandler.shutdown(); - inputHandler.shutdown(); + // Kill the agent connection. This will kill the agent, closing the CONIN + // and CONOUT pipes on the agent pipe, prompting our I/O handler to shut + // down. + winpty_free(wp); - const int exitCode = winpty_get_exit_code(winpty); + inputHandler.shutdown(); + outputHandler.shutdown(); + CloseHandle(conin); + CloseHandle(conout); if (args.mouseInput) { // Reseting both encoding modes (1006 and 1015) is necessary, but @@ -513,7 +565,11 @@ int main(int argc, char *argv[]) } restoreTerminalMode(mode); - winpty_close(winpty); + DWORD exitCode = 0; + if (!GetExitCodeProcess(childHandle, &exitCode)) { + exitCode = 1; + } + CloseHandle(childHandle); return exitCode; } diff --git a/src/winpty.gyp b/src/winpty.gyp index ac31197f..82d78c96 100644 --- a/src/winpty.gyp +++ b/src/winpty.gyp @@ -38,6 +38,7 @@ 'WINPTY_AGENT_ASSERT', ], 'libraries' : [ + '-ladvapi32', '-lshell32', '-luser32', ], @@ -84,11 +85,17 @@ 'shared/Buffer.cc', 'shared/DebugClient.h', 'shared/DebugClient.cc', + 'shared/GenRandom.h', + 'shared/GenRandom.cc', 'shared/OsModule.h', + 'shared/OwnedHandle.h', + 'shared/OwnedHandle.cc', 'shared/StringBuilder.h', 'shared/StringUtil.cc', 'shared/StringUtil.h', 'shared/UnixCtrlChars.h', + 'shared/WindowsSecurity.cc', + 'shared/WindowsSecurity.h', 'shared/WindowsVersion.h', 'shared/WindowsVersion.cc', 'shared/WinptyAssert.h', @@ -132,9 +139,9 @@ 'shared/DebugClient.cc', 'shared/GenRandom.h', 'shared/GenRandom.cc', + 'shared/OsModule.h', 'shared/OwnedHandle.h', 'shared/OwnedHandle.cc', - 'shared/OsModule.h', 'shared/StringBuilder.h', 'shared/StringUtil.cc', 'shared/StringUtil.h',