https://github.com/charles-zablit updated https://github.com/llvm/llvm-project/pull/182302
>From b07fba958d69b44ae59700943ada439418d9a7d8 Mon Sep 17 00:00:00 2001 From: Charles Zablit <[email protected]> Date: Fri, 20 Feb 2026 16:46:39 +0000 Subject: [PATCH 1/5] [lldb][windows] fix a race condition when closing the ConPTY --- .../include/lldb/Core/ThreadedCommunication.h | 6 ++ lldb/include/lldb/Host/ProcessLaunchInfo.h | 2 + .../Host/windows/ConnectionConPTYWindows.h | 50 +++++++++++ .../windows/ConnectionGenericFileWindows.h | 1 + .../include/lldb/Host/windows/PseudoConsole.h | 46 +++++++++- lldb/source/Core/ThreadedCommunication.cpp | 6 +- lldb/source/Host/CMakeLists.txt | 1 + lldb/source/Host/common/ProcessLaunchInfo.cpp | 11 ++- .../Host/windows/ConnectionConPTYWindows.cpp | 84 +++++++++++++++++++ .../windows/ConnectionGenericFileWindows.cpp | 10 +++ .../Host/windows/ProcessLauncherWindows.cpp | 2 +- lldb/source/Host/windows/PseudoConsole.cpp | 24 ++++-- .../Process/Windows/Common/ProcessWindows.cpp | 9 +- 13 files changed, 238 insertions(+), 14 deletions(-) create mode 100644 lldb/include/lldb/Host/windows/ConnectionConPTYWindows.h create mode 100644 lldb/source/Host/windows/ConnectionConPTYWindows.cpp diff --git a/lldb/include/lldb/Core/ThreadedCommunication.h b/lldb/include/lldb/Core/ThreadedCommunication.h index 24412b2027932..352106d352a0d 100644 --- a/lldb/include/lldb/Core/ThreadedCommunication.h +++ b/lldb/include/lldb/Core/ThreadedCommunication.h @@ -216,6 +216,12 @@ class ThreadedCommunication : public Communication, public Broadcaster { /// void SynchronizeWithReadThread(); + /// Interrupts the current read. + /// + /// Unlike SynchronizeWithReadThread, this does not wait for the read loop to + /// finish processing outstanding data. + void InterruptRead(); + static llvm::StringRef GetStaticBroadcasterClass(); llvm::StringRef GetBroadcasterClass() const override { diff --git a/lldb/include/lldb/Host/ProcessLaunchInfo.h b/lldb/include/lldb/Host/ProcessLaunchInfo.h index e13eecc9463ea..7801662b244ad 100644 --- a/lldb/include/lldb/Host/ProcessLaunchInfo.h +++ b/lldb/include/lldb/Host/ProcessLaunchInfo.h @@ -136,6 +136,8 @@ class ProcessLaunchInfo : public ProcessInfo { /// always true on non Windows. bool ShouldUsePTY() const { #ifdef _WIN32 + if (!m_pty) + return false; return GetPTY().GetPseudoTerminalHandle() != ((HANDLE)(long long)-1) && GetNumFileActions() == 0; #else diff --git a/lldb/include/lldb/Host/windows/ConnectionConPTYWindows.h b/lldb/include/lldb/Host/windows/ConnectionConPTYWindows.h new file mode 100644 index 0000000000000..dc08833fc9d63 --- /dev/null +++ b/lldb/include/lldb/Host/windows/ConnectionConPTYWindows.h @@ -0,0 +1,50 @@ +//===-- ConnectionConPTY.h ------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLDB_HOST_WINDOWS_CONNECTIONCONPTYWINDOWS_H +#define LLDB_HOST_WINDOWS_CONNECTIONCONPTYWINDOWS_H + +#include "lldb/Host/windows/ConnectionGenericFileWindows.h" +#include "lldb/Host/windows/PseudoConsole.h" +#include "lldb/Host/windows/windows.h" +#include "lldb/Utility/Connection.h" +#include "lldb/lldb-types.h" +#include <mutex> + +namespace lldb_private { + +/// A read only Connection implementation for the Windows ConPTY. +class ConnectionConPTY : public ConnectionGenericFile { +public: + ConnectionConPTY(std::shared_ptr<PseudoConsole> pty); + + ~ConnectionConPTY(); + + lldb::ConnectionStatus Connect(llvm::StringRef s, Status *error_ptr) override; + + lldb::ConnectionStatus Disconnect(Status *error_ptr) override; + + /// Read from the ConPTY's pipe. + /// + /// Before reading, check if the ConPTY is closing and wait for it to close + /// before reading. This prevents race conditions when closing the ConPTY + /// during a read. After reading, remove the ConPTY VT init sequence if + /// present. + size_t Read(void *dst, size_t dst_len, const Timeout<std::micro> &timeout, + lldb::ConnectionStatus &status, Status *error_ptr) override; + + size_t Write(const void *src, size_t src_len, lldb::ConnectionStatus &status, + Status *error_ptr) override; + +protected: + std::shared_ptr<PseudoConsole> m_pty; + bool m_pty_vt_sequence_was_stripped = false; +}; +} // namespace lldb_private + +#endif diff --git a/lldb/include/lldb/Host/windows/ConnectionGenericFileWindows.h b/lldb/include/lldb/Host/windows/ConnectionGenericFileWindows.h index d8f06a7162b22..62ac1bb35c9f8 100644 --- a/lldb/include/lldb/Host/windows/ConnectionGenericFileWindows.h +++ b/lldb/include/lldb/Host/windows/ConnectionGenericFileWindows.h @@ -43,6 +43,7 @@ class ConnectionGenericFile : public lldb_private::Connection { protected: OVERLAPPED m_overlapped; + bool m_read_pending = false; HANDLE m_file; HANDLE m_event_handles[2]; bool m_owns_file; diff --git a/lldb/include/lldb/Host/windows/PseudoConsole.h b/lldb/include/lldb/Host/windows/PseudoConsole.h index 996faf434585d..c26ca12b9fc57 100644 --- a/lldb/include/lldb/Host/windows/PseudoConsole.h +++ b/lldb/include/lldb/Host/windows/PseudoConsole.h @@ -10,6 +10,7 @@ #define LIBLLDB_HOST_WINDOWS_PSEUDOCONSOLE_H_ #include "llvm/Support/Error.h" +#include <mutex> #include <string> #define PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE 0x20016 @@ -29,11 +30,31 @@ class PseudoConsole { PseudoConsole &operator=(const PseudoConsole &) = delete; PseudoConsole &operator=(PseudoConsole &&) = delete; + /// Creates and opens a new ConPTY instance with a default console size of + /// 80x25. Also sets up the associated STDIN/STDOUT pipes and drains any + /// initialization sequences emitted by Windows. + /// + /// \return + /// An llvm::Error if the ConPTY could not be created, or if ConPTY is + /// not available on this version of Windows, llvm::Error::success() + /// otherwise. llvm::Error OpenPseudoConsole(); - /// Close the ConPTY, its read/write handles and invalidate them. + /// Closes the ConPTY and invalidates its handle, without closing the STDIN + /// and STDOUT pipes. Closing the ConPTY signals EOF to any process currently + /// attached to it. void Close(); + /// Closes the STDIN and STDOUT pipe handles and invalidates them + void ClosePipes(); + + /// Returns whether the ConPTY and its pipes are currently open and valid. + /// + /// \return + /// True if the ConPTY handle, STDIN write handle, and STDOUT read handle + /// are all valid, false otherwise. + bool IsConnected() const; + /// The ConPTY HPCON handle accessor. /// /// This object retains ownership of the HPCON when this accessor is used. @@ -71,10 +92,33 @@ class PseudoConsole { /// then drain all output before launching the actual debuggee. llvm::Error DrainInitSequences(); + /// Returns a reference to the mutex used to synchronize access to the + /// ConPTY state. + std::mutex &GetMutex() { return m_mutex; }; + + /// Returns a reference to the condition variable used to signal state changes + /// to threads waiting on the ConPTY (e.g. waiting for output or shutdown). + std::condition_variable &GetCV() { return m_cv; }; + + /// Returns whether the ConPTY is in the process of shutting down. + /// + /// \return + /// A reference to the atomic bool that is set to true when the ConPTY + /// is stopping. Callers should check this in their read/write loops to + /// exit gracefully. + const std::atomic<bool> &IsStopping() const { return m_stopping; }; + + /// Sets the stopping flag to \p value, signalling to threads waiting on the + /// ConPTY that they should stop. + void SetStopping(bool value) { m_stopping = value; }; + protected: HANDLE m_conpty_handle = ((HANDLE)(long long)-1); HANDLE m_conpty_output = ((HANDLE)(long long)-1); HANDLE m_conpty_input = ((HANDLE)(long long)-1); + std::mutex m_mutex{}; + std::condition_variable m_cv{}; + std::atomic<bool> m_stopping = false; }; } // namespace lldb_private diff --git a/lldb/source/Core/ThreadedCommunication.cpp b/lldb/source/Core/ThreadedCommunication.cpp index 09f78f13f34d3..b24f953426afc 100644 --- a/lldb/source/Core/ThreadedCommunication.cpp +++ b/lldb/source/Core/ThreadedCommunication.cpp @@ -360,6 +360,10 @@ void ThreadedCommunication::SetReadThreadBytesReceivedCallback( m_callback_baton = callback_baton; } +void ThreadedCommunication::InterruptRead() { + m_connection_sp->InterruptRead(); +} + void ThreadedCommunication::SynchronizeWithReadThread() { // Only one thread can do the synchronization dance at a time. std::lock_guard<std::mutex> guard(m_synchronize_mutex); @@ -374,7 +378,7 @@ void ThreadedCommunication::SynchronizeWithReadThread() { return; // Notify the read thread. - m_connection_sp->InterruptRead(); + InterruptRead(); // Wait for the synchronization event. EventSP event_sp; diff --git a/lldb/source/Host/CMakeLists.txt b/lldb/source/Host/CMakeLists.txt index 8c198c655e0a6..651f3f09c471c 100644 --- a/lldb/source/Host/CMakeLists.txt +++ b/lldb/source/Host/CMakeLists.txt @@ -67,6 +67,7 @@ add_host_subdirectory(posix if (CMAKE_SYSTEM_NAME MATCHES "Windows") add_subdirectory(windows/PythonPathSetup) add_host_subdirectory(windows + windows/ConnectionConPTYWindows.cpp windows/ConnectionGenericFileWindows.cpp windows/FileSystem.cpp windows/Host.cpp diff --git a/lldb/source/Host/common/ProcessLaunchInfo.cpp b/lldb/source/Host/common/ProcessLaunchInfo.cpp index ea036d45e7c80..2f67a417996ac 100644 --- a/lldb/source/Host/common/ProcessLaunchInfo.cpp +++ b/lldb/source/Host/common/ProcessLaunchInfo.cpp @@ -33,7 +33,11 @@ using namespace lldb_private; ProcessLaunchInfo::ProcessLaunchInfo() : ProcessInfo(), m_working_dir(), m_plugin_name(), m_flags(0), - m_file_actions(), m_pty(new PTY), m_monitor_callback(nullptr) {} + m_file_actions(), m_monitor_callback(nullptr) { +#ifndef _WIN32 + m_pty = std::make_shared<PTY>(); +#endif +} ProcessLaunchInfo::ProcessLaunchInfo(const FileSpec &stdin_file_spec, const FileSpec &stdout_file_spec, @@ -41,7 +45,10 @@ ProcessLaunchInfo::ProcessLaunchInfo(const FileSpec &stdin_file_spec, const FileSpec &working_directory, uint32_t launch_flags) : ProcessInfo(), m_working_dir(), m_plugin_name(), m_flags(launch_flags), - m_file_actions(), m_pty(new PTY) { + m_file_actions() { +#ifndef _WIN32 + m_pty = std::make_shared<PTY>(); +#endif if (stdin_file_spec) { FileAction file_action; const bool read = true; diff --git a/lldb/source/Host/windows/ConnectionConPTYWindows.cpp b/lldb/source/Host/windows/ConnectionConPTYWindows.cpp new file mode 100644 index 0000000000000..c6959c8588af0 --- /dev/null +++ b/lldb/source/Host/windows/ConnectionConPTYWindows.cpp @@ -0,0 +1,84 @@ +//===-- ConnectionConPTY.cpp ----------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "lldb/Host/windows/ConnectionConPTYWindows.h" +#include "lldb/Utility/Status.h" + +using namespace lldb; +using namespace lldb_private; + +/// Strips the ConPTY initialization sequences that Windows unconditionally +/// emits when a process is first attached to a pseudo console. +/// +/// These are emitted by ConPTY's host process (conhost.exe) at process attach +/// time, not by the debuggee. They are always the first bytes on the output +/// pipe and are always present as a contiguous prefix. +/// +/// \param dst Buffer containing the data read from the ConPTY output pipe. +/// Modified in place: if the initialization sequences are present +/// as a prefix, they are removed by shifting the remaining bytes +/// to the front of the buffer. +/// \param len On input, the number of valid bytes in \p dst. On output, +/// reduced by the number of bytes stripped. +/// \return +/// \p true if the sequence was found and stripped. +static bool StripConPTYInitSequences(void *dst, size_t &len) { + static const char sequences[] = "\x1b[?9001l\x1b[?1004l"; + static const size_t sequences_len = sizeof(sequences) - 1; + + char *buf = static_cast<char *>(dst); + if (len >= sequences_len && memcmp(buf, sequences, sequences_len) == 0) { + memmove(buf, buf + sequences_len, len - sequences_len); + len -= sequences_len; + return true; + } + return false; +} + +ConnectionConPTY::ConnectionConPTY(std::shared_ptr<PseudoConsole> pty) + : m_pty(pty), ConnectionGenericFile(pty->GetSTDOUTHandle(), false) {}; + +ConnectionConPTY::~ConnectionConPTY() {} + +lldb::ConnectionStatus ConnectionConPTY::Connect(llvm::StringRef s, + Status *error_ptr) { + if (m_pty->IsConnected()) + return eConnectionStatusSuccess; + return eConnectionStatusNoConnection; +} + +lldb::ConnectionStatus ConnectionConPTY::Disconnect(Status *error_ptr) { + m_pty->Close(); + return eConnectionStatusSuccess; +} + +size_t ConnectionConPTY::Read(void *dst, size_t dst_len, + const Timeout<std::micro> &timeout, + lldb::ConnectionStatus &status, + Status *error_ptr) { + std::unique_lock<std::mutex> guard(m_pty->GetMutex()); + if (m_pty->IsStopping().load()) { + m_pty->GetCV().wait(guard, [this] { return !m_pty->IsStopping().load(); }); + } + + size_t bytes_read = + ConnectionGenericFile::Read(dst, dst_len, timeout, status, error_ptr); + + if (bytes_read > 0 && !m_pty_vt_sequence_was_stripped) { + if (StripConPTYInitSequences(dst, bytes_read)) + m_pty_vt_sequence_was_stripped = true; + } + + return bytes_read; +} + +size_t ConnectionConPTY::Write(const void *src, size_t src_len, + lldb::ConnectionStatus &status, + Status *error_ptr) { + llvm_unreachable("not implemented"); +} diff --git a/lldb/source/Host/windows/ConnectionGenericFileWindows.cpp b/lldb/source/Host/windows/ConnectionGenericFileWindows.cpp index 6a6153d6e34a0..28727c82f6e37 100644 --- a/lldb/source/Host/windows/ConnectionGenericFileWindows.cpp +++ b/lldb/source/Host/windows/ConnectionGenericFileWindows.cpp @@ -176,9 +176,17 @@ size_t ConnectionGenericFile::Read(void *dst, size_t dst_len, goto finish; } + if (m_read_pending) { + if (::GetOverlappedResult(m_file, &m_overlapped, &bytes_read, FALSE)) { + m_read_pending = false; + return bytes_read; + } + } + m_overlapped.hEvent = m_event_handles[kBytesAvailableEvent]; result = ::ReadFile(m_file, dst, dst_len, NULL, &m_overlapped); + m_read_pending = true; if (result || ::GetLastError() == ERROR_IO_PENDING) { if (!result) { // The expected return path. The operation is pending. Wait for the @@ -234,6 +242,8 @@ size_t ConnectionGenericFile::Read(void *dst, size_t dst_len, goto finish; finish: + if (return_info.GetStatus() != eConnectionStatusInterrupted) + m_read_pending = false; status = return_info.GetStatus(); if (error_ptr) *error_ptr = return_info.GetError().Clone(); diff --git a/lldb/source/Host/windows/ProcessLauncherWindows.cpp b/lldb/source/Host/windows/ProcessLauncherWindows.cpp index cfd84731f0eb6..f22e4739ca7b6 100644 --- a/lldb/source/Host/windows/ProcessLauncherWindows.cpp +++ b/lldb/source/Host/windows/ProcessLauncherWindows.cpp @@ -124,7 +124,6 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo &launch_info, startupinfoex.StartupInfo.cb = sizeof(STARTUPINFOEXW); startupinfoex.StartupInfo.dwFlags |= STARTF_USESTDHANDLES; - HPCON hPC = launch_info.GetPTY().GetPseudoTerminalHandle(); bool use_pty = launch_info.ShouldUsePTY(); HANDLE stdin_handle = GetStdioHandle(launch_info, STDIN_FILENO); @@ -148,6 +147,7 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo &launch_info, std::vector<HANDLE> inherited_handles; if (use_pty) { + HPCON hPC = launch_info.GetPTY().GetPseudoTerminalHandle(); if (auto err = attributelist.SetupPseudoConsole(hPC)) { error = Status::FromError(std::move(err)); return HostProcess(); diff --git a/lldb/source/Host/windows/PseudoConsole.cpp b/lldb/source/Host/windows/PseudoConsole.cpp index 15bc8fc9d32fe..da4b1310ea17c 100644 --- a/lldb/source/Host/windows/PseudoConsole.cpp +++ b/lldb/source/Host/windows/PseudoConsole.cpp @@ -65,7 +65,10 @@ struct Kernel32 { static Kernel32 kernel32; -PseudoConsole::~PseudoConsole() { Close(); } +PseudoConsole::~PseudoConsole() { + Close(); + ClosePipes(); +} llvm::Error PseudoConsole::OpenPseudoConsole() { if (!kernel32.IsConPTYAvailable()) @@ -128,19 +131,28 @@ llvm::Error PseudoConsole::OpenPseudoConsole() { return llvm::Error::success(); } +bool PseudoConsole::IsConnected() const { + return m_conpty_handle != INVALID_HANDLE_VALUE && + m_conpty_input != INVALID_HANDLE_VALUE && + m_conpty_output != INVALID_HANDLE_VALUE; +} + void PseudoConsole::Close() { - Sleep(50); // FIXME: This mitigates a race condition when closing the - // PseudoConsole. It's possible that there is still data in the - // pipe when we try to close it. We should wait until the data has - // been consumed. + SetStopping(true); + std::unique_lock<std::mutex> guard(m_mutex); if (m_conpty_handle != INVALID_HANDLE_VALUE) kernel32.ClosePseudoConsole(m_conpty_handle); + m_conpty_handle = INVALID_HANDLE_VALUE; + SetStopping(false); + m_cv.notify_all(); +} + +void PseudoConsole::ClosePipes() { if (m_conpty_input != INVALID_HANDLE_VALUE) CloseHandle(m_conpty_input); if (m_conpty_output != INVALID_HANDLE_VALUE) CloseHandle(m_conpty_output); - m_conpty_handle = INVALID_HANDLE_VALUE; m_conpty_input = INVALID_HANDLE_VALUE; m_conpty_output = INVALID_HANDLE_VALUE; } diff --git a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp index 8c1919eca7dda..2662e16090469 100644 --- a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp +++ b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp @@ -24,7 +24,7 @@ #include "lldb/Host/HostProcess.h" #include "lldb/Host/Pipe.h" #include "lldb/Host/PseudoTerminal.h" -#include "lldb/Host/windows/ConnectionGenericFileWindows.h" +#include "lldb/Host/windows/ConnectionConPTYWindows.h" #include "lldb/Host/windows/HostThreadWindows.h" #include "lldb/Symbol/ObjectFile.h" #include "lldb/Target/DynamicLoader.h" @@ -651,8 +651,11 @@ void ProcessWindows::OnExitProcess(uint32_t exit_code) { Log *log = GetLog(WindowsLog::Process); LLDB_LOG(log, "Process {0} exited with code {1}", GetID(), exit_code); - if (m_pty) + if (m_pty) { + m_pty->SetStopping(true); + m_stdio_communication.InterruptRead(); m_pty->Close(); + } TargetSP target = CalculateTarget(); if (target) { @@ -1134,7 +1137,7 @@ void ProcessWindows::SetPseudoConsoleHandle() { if (m_pty == nullptr) return; m_stdio_communication.SetConnection( - std::make_unique<ConnectionGenericFile>(m_pty->GetSTDOUTHandle(), false)); + std::make_unique<ConnectionConPTY>(m_pty)); if (m_stdio_communication.IsConnected()) { m_stdio_communication.SetReadThreadBytesReceivedCallback( STDIOReadThreadBytesReceived, this); >From eb56478182e71833003bd45b9f8e23b0594ea575 Mon Sep 17 00:00:00 2001 From: Charles Zablit <[email protected]> Date: Fri, 20 Feb 2026 17:46:10 +0000 Subject: [PATCH 2/5] Apply suggestions from code review Co-authored-by: Nerixyz <[email protected]> --- lldb/include/lldb/Host/windows/ConnectionConPTYWindows.h | 2 +- lldb/source/Host/windows/ConnectionConPTYWindows.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lldb/include/lldb/Host/windows/ConnectionConPTYWindows.h b/lldb/include/lldb/Host/windows/ConnectionConPTYWindows.h index dc08833fc9d63..7f1524445ae4c 100644 --- a/lldb/include/lldb/Host/windows/ConnectionConPTYWindows.h +++ b/lldb/include/lldb/Host/windows/ConnectionConPTYWindows.h @@ -1,4 +1,4 @@ -//===-- ConnectionConPTY.h ------------------------------------------------===// +//===----------------------------------------------------------------------===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. diff --git a/lldb/source/Host/windows/ConnectionConPTYWindows.cpp b/lldb/source/Host/windows/ConnectionConPTYWindows.cpp index c6959c8588af0..99805359e78ef 100644 --- a/lldb/source/Host/windows/ConnectionConPTYWindows.cpp +++ b/lldb/source/Host/windows/ConnectionConPTYWindows.cpp @@ -1,4 +1,4 @@ -//===-- ConnectionConPTY.cpp ----------------------------------------------===// +//===----------------------------------------------------------------------===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. >From d60bf6b5c0c0d46ea578145f1fc9ed129d82bdd2 Mon Sep 17 00:00:00 2001 From: Charles Zablit <[email protected]> Date: Fri, 20 Feb 2026 17:46:39 +0000 Subject: [PATCH 3/5] Apply suggestion from @Nerixyz Co-authored-by: Nerixyz <[email protected]> --- lldb/include/lldb/Host/windows/PseudoConsole.h | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lldb/include/lldb/Host/windows/PseudoConsole.h b/lldb/include/lldb/Host/windows/PseudoConsole.h index c26ca12b9fc57..197e091aeb8c2 100644 --- a/lldb/include/lldb/Host/windows/PseudoConsole.h +++ b/lldb/include/lldb/Host/windows/PseudoConsole.h @@ -49,10 +49,6 @@ class PseudoConsole { void ClosePipes(); /// Returns whether the ConPTY and its pipes are currently open and valid. - /// - /// \return - /// True if the ConPTY handle, STDIN write handle, and STDOUT read handle - /// are all valid, false otherwise. bool IsConnected() const; /// The ConPTY HPCON handle accessor. >From eefd84831d795a9d6d2a44e72ba59312b6980417 Mon Sep 17 00:00:00 2001 From: Charles Zablit <[email protected]> Date: Fri, 20 Feb 2026 18:04:55 +0000 Subject: [PATCH 4/5] fixup! [lldb][windows] fix a race condition when closing the ConPTY --- lldb/include/lldb/Host/windows/PseudoConsole.h | 2 +- lldb/source/Host/windows/ConnectionConPTYWindows.cpp | 11 +++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/lldb/include/lldb/Host/windows/PseudoConsole.h b/lldb/include/lldb/Host/windows/PseudoConsole.h index 197e091aeb8c2..12d21a567958b 100644 --- a/lldb/include/lldb/Host/windows/PseudoConsole.h +++ b/lldb/include/lldb/Host/windows/PseudoConsole.h @@ -102,7 +102,7 @@ class PseudoConsole { /// A reference to the atomic bool that is set to true when the ConPTY /// is stopping. Callers should check this in their read/write loops to /// exit gracefully. - const std::atomic<bool> &IsStopping() const { return m_stopping; }; + const bool &IsStopping() const { return m_stopping; }; /// Sets the stopping flag to \p value, signalling to threads waiting on the /// ConPTY that they should stop. diff --git a/lldb/source/Host/windows/ConnectionConPTYWindows.cpp b/lldb/source/Host/windows/ConnectionConPTYWindows.cpp index 99805359e78ef..f3dc0aef1a105 100644 --- a/lldb/source/Host/windows/ConnectionConPTYWindows.cpp +++ b/lldb/source/Host/windows/ConnectionConPTYWindows.cpp @@ -23,14 +23,17 @@ using namespace lldb_private; /// Modified in place: if the initialization sequences are present /// as a prefix, they are removed by shifting the remaining bytes /// to the front of the buffer. +/// \param dst_len The size of \p dst. /// \param len On input, the number of valid bytes in \p dst. On output, /// reduced by the number of bytes stripped. /// \return /// \p true if the sequence was found and stripped. -static bool StripConPTYInitSequences(void *dst, size_t &len) { +static bool StripConPTYInitSequences(void *dst, size_t dst_len, size_t &len) { static const char sequences[] = "\x1b[?9001l\x1b[?1004l"; static const size_t sequences_len = sizeof(sequences) - 1; + assert(dst_len >= len - sequences_len); + char *buf = static_cast<char *>(dst); if (len >= sequences_len && memcmp(buf, sequences, sequences_len) == 0) { memmove(buf, buf + sequences_len, len - sequences_len); @@ -62,15 +65,15 @@ size_t ConnectionConPTY::Read(void *dst, size_t dst_len, lldb::ConnectionStatus &status, Status *error_ptr) { std::unique_lock<std::mutex> guard(m_pty->GetMutex()); - if (m_pty->IsStopping().load()) { - m_pty->GetCV().wait(guard, [this] { return !m_pty->IsStopping().load(); }); + if (m_pty->IsStopping()) { + m_pty->GetCV().wait(guard, [this] { return !m_pty->IsStopping(); }); } size_t bytes_read = ConnectionGenericFile::Read(dst, dst_len, timeout, status, error_ptr); if (bytes_read > 0 && !m_pty_vt_sequence_was_stripped) { - if (StripConPTYInitSequences(dst, bytes_read)) + if (StripConPTYInitSequences(dst, dst_len, bytes_read)) m_pty_vt_sequence_was_stripped = true; } >From 006267becd326dd66537f90a2c2c728dbd195ed1 Mon Sep 17 00:00:00 2001 From: Charles Zablit <[email protected]> Date: Fri, 20 Feb 2026 18:22:23 +0000 Subject: [PATCH 5/5] fixup! [lldb][windows] fix a race condition when closing the ConPTY --- .../Host/windows/ConnectionConPTYWindows.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lldb/source/Host/windows/ConnectionConPTYWindows.cpp b/lldb/source/Host/windows/ConnectionConPTYWindows.cpp index f3dc0aef1a105..7d6fed56044c8 100644 --- a/lldb/source/Host/windows/ConnectionConPTYWindows.cpp +++ b/lldb/source/Host/windows/ConnectionConPTYWindows.cpp @@ -31,14 +31,14 @@ using namespace lldb_private; static bool StripConPTYInitSequences(void *dst, size_t dst_len, size_t &len) { static const char sequences[] = "\x1b[?9001l\x1b[?1004l"; static const size_t sequences_len = sizeof(sequences) - 1; - - assert(dst_len >= len - sequences_len); - char *buf = static_cast<char *>(dst); - if (len >= sequences_len && memcmp(buf, sequences, sequences_len) == 0) { - memmove(buf, buf + sequences_len, len - sequences_len); - len -= sequences_len; - return true; + if (len >= sequences_len) { + assert(dst_len >= len - sequences_len); + if (memcmp(buf, sequences, sequences_len) == 0) { + memmove(buf, buf + sequences_len, len - sequences_len); + len -= sequences_len; + return true; + } } return false; } _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
