llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Charles Zablit (charles-zablit) <details> <summary>Changes</summary> 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. --- Full diff: https://github.com/llvm/llvm-project/pull/181811.diff 7 Files Affected: - (modified) lldb/include/lldb/Host/ProcessLaunchInfo.h (+1-1) - (modified) lldb/include/lldb/Target/Process.h (+1-6) - (modified) lldb/source/Host/common/ProcessLaunchInfo.cpp (+3) - (modified) lldb/source/Host/windows/PseudoConsole.cpp (+2-2) - (modified) lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp (+1-2) - (modified) lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp (+6-5) - (modified) lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h (+1-2) ``````````diff diff --git a/lldb/include/lldb/Host/ProcessLaunchInfo.h b/lldb/include/lldb/Host/ProcessLaunchInfo.h index 023d150503da3..a07f35ac472e9 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> ReleasePTY() { 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 differentiation 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..3c354c9164a50 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 == nullptr) + 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..ce969e2ff8454 100644 --- a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp +++ b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp @@ -214,7 +214,7 @@ 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.ReleasePTY(); return error; } @@ -1130,10 +1130,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 +1145,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); `````````` </details> https://github.com/llvm/llvm-project/pull/181811 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
