Author: Charles Zablit Date: 2026-02-17T22:10:27Z New Revision: 71f49eb026dd15ace88a3b233472ee288893afc1
URL: https://github.com/llvm/llvm-project/commit/71f49eb026dd15ace88a3b233472ee288893afc1 DIFF: https://github.com/llvm/llvm-project/commit/71f49eb026dd15ace88a3b233472ee288893afc1.diff LOG: [lldb][windows] release the ConPTY instead of copying its reference (#181811) When running multiple targets in lldb, the `ProcessLaunchInfo` instance gets re-used. This is a problem for the ConPTY instance which is owned by `launch_info` because, when creating a second target, we close and reopen the `m_pty` of the first target. This patch replaces the `GetPTYSP` method with `ReleasePTY`, which moves the `std::shared_ptr` out of `launch_info` instead of creating a new `std::shared_ptr`. The `m_pty` instance is moved into the target's process. This way, each target's `Process` instance owns its PTY instance and is responsible for closing it. --------- Co-authored-by: Adrian Prantl <[email protected]> Added: Modified: lldb/include/lldb/Host/ProcessLaunchInfo.h lldb/include/lldb/Target/Process.h lldb/source/Host/common/ProcessLaunchInfo.cpp lldb/source/Host/windows/PseudoConsole.cpp lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h Removed: ################################################################################ diff --git a/lldb/include/lldb/Host/ProcessLaunchInfo.h b/lldb/include/lldb/Host/ProcessLaunchInfo.h index 023d150503da3..d89fe68b2d0d4 100644 --- a/lldb/include/lldb/Host/ProcessLaunchInfo.h +++ b/lldb/include/lldb/Host/ProcessLaunchInfo.h @@ -130,7 +130,7 @@ class ProcessLaunchInfo : public ProcessInfo { PTY &GetPTY() const { return *m_pty; } - std::shared_ptr<PTY> GetPTYSP() const { return m_pty; } + std::shared_ptr<PTY> TakePTY() { return std::move(m_pty); } /// Returns whether if lldb should read information from the PTY. This is /// always true on non Windows. diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h index 7a15adebc63ee..c4eb8cf3694b1 100644 --- a/lldb/include/lldb/Target/Process.h +++ b/lldb/include/lldb/Target/Process.h @@ -2561,15 +2561,10 @@ void PruneThreadPlans(); /// When data is successfully read from the ConPTY, it is stored in /// m_stdout_data. There is no diff erentiation between stdout and stderr. /// - /// \param[in] pty - /// The ConPTY to use for process STDIO communication. It's - /// assumed to be valid. - /// /// \see lldb_private::Process::STDIOReadThreadBytesReceived() /// \see lldb_private::IOHandlerProcessSTDIOWindows /// \see lldb_private::PseudoConsole - virtual void - SetPseudoConsoleHandle(const std::shared_ptr<PseudoConsole> &pty) {}; + virtual void SetPseudoConsoleHandle() {}; #endif /// Associates a file descriptor with the process' STDIO handling diff --git a/lldb/source/Host/common/ProcessLaunchInfo.cpp b/lldb/source/Host/common/ProcessLaunchInfo.cpp index e82cc11187fe5..ea036d45e7c80 100644 --- a/lldb/source/Host/common/ProcessLaunchInfo.cpp +++ b/lldb/source/Host/common/ProcessLaunchInfo.cpp @@ -200,6 +200,9 @@ void ProcessLaunchInfo::SetDetachOnError(bool enable) { llvm::Error ProcessLaunchInfo::SetUpPtyRedirection() { Log *log = GetLog(LLDBLog::Process); + if (!m_pty) + m_pty = std::make_shared<PTY>(); + bool stdin_free = GetFileActionForFD(STDIN_FILENO) == nullptr; bool stdout_free = GetFileActionForFD(STDOUT_FILENO) == nullptr; bool stderr_free = GetFileActionForFD(STDERR_FILENO) == nullptr; diff --git a/lldb/source/Host/windows/PseudoConsole.cpp b/lldb/source/Host/windows/PseudoConsole.cpp index 413c8a3328445..2228c315665f4 100644 --- a/lldb/source/Host/windows/PseudoConsole.cpp +++ b/lldb/source/Host/windows/PseudoConsole.cpp @@ -72,8 +72,8 @@ llvm::Error PseudoConsole::OpenPseudoConsole() { return llvm::make_error<llvm::StringError>("ConPTY is not available", llvm::errc::io_error); - // close any previously opened handles - Close(); + assert(m_conpty_handle == INVALID_HANDLE_VALUE && + "ConPTY has already been opened"); HRESULT hr; HANDLE hInputRead = INVALID_HANDLE_VALUE; diff --git a/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp b/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp index 0e74cab23c99e..1bc9d3fb9978d 100644 --- a/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp +++ b/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp @@ -523,8 +523,7 @@ ProcessSP PlatformWindows::DebugProcess(ProcessLaunchInfo &launch_info, error = process_sp->Launch(launch_info); #ifdef _WIN32 if (error.Success()) { - if (launch_info.ShouldUsePTY()) - process_sp->SetPseudoConsoleHandle(launch_info.GetPTYSP()); + process_sp->SetPseudoConsoleHandle(); } else { Log *log = GetLog(LLDBLog::Platform); LLDB_LOGF(log, "Platform::%s LaunchProcess() failed: %s", __FUNCTION__, diff --git a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp index 373729c952071..9a53252b2b4ae 100644 --- a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp +++ b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp @@ -214,7 +214,11 @@ Status ProcessWindows::DoLaunch(Module *exe_module, error = LaunchProcess(launch_info, delegate); if (error.Success()) SetID(launch_info.GetProcessID()); - m_pty = launch_info.GetPTYSP(); + m_pty = launch_info.TakePTY(); + // At this point, Process owns the ConPTY. If ProcessLaunchInfo still has a + // reference to it, it might get closed prematurely if another target is + // created. + assert(m_pty.use_count() == 1 && "More than one reference to the ConPTY"); return error; } @@ -1130,10 +1134,11 @@ class IOHandlerProcessSTDIOWindows : public IOHandler { bool m_is_running = false; }; -void ProcessWindows::SetPseudoConsoleHandle( - const std::shared_ptr<PseudoConsole> &pty) { +void ProcessWindows::SetPseudoConsoleHandle() { + if (m_pty == nullptr) + return; m_stdio_communication.SetConnection( - std::make_unique<ConnectionGenericFile>(pty->GetSTDOUTHandle(), false)); + std::make_unique<ConnectionGenericFile>(m_pty->GetSTDOUTHandle(), false)); if (m_stdio_communication.IsConnected()) { m_stdio_communication.SetReadThreadBytesReceivedCallback( STDIOReadThreadBytesReceived, this); @@ -1144,7 +1149,7 @@ void ProcessWindows::SetPseudoConsoleHandle( std::lock_guard<std::mutex> guard(m_process_input_reader_mutex); if (!m_process_input_reader) m_process_input_reader = std::make_shared<IOHandlerProcessSTDIOWindows>( - this, pty->GetSTDINHandle()); + this, m_pty->GetSTDINHandle()); } } } diff --git a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h index becab6964a4aa..d3e3f9a5ed0ce 100644 --- a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h +++ b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h @@ -98,8 +98,7 @@ class ProcessWindows : public Process, public ProcessDebugger { Status DisableWatchpoint(lldb::WatchpointSP wp_sp, bool notify = true) override; - void - SetPseudoConsoleHandle(const std::shared_ptr<PseudoConsole> &pty) override; + void SetPseudoConsoleHandle() override; protected: ProcessWindows(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp); _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
