clayborg added inline comments.
================ Comment at: lldb/include/lldb/Interpreter/CommandObjectDelegate.h:30-32 + virtual lldb::CommandObjectSP DoGetDelegateCommand() = 0; + + CommandObject *GetDelegateCommand(); ---------------- So currently you are storing the command object into m_delegate_command_sp in GetDelegateCommand(). This can cause an issue when you have two debuggers (Xcode creates a new debugger for each debug window). If thread A calls this function, and fills in m_delegate_command_sp, then thread B fills in m_delegate_command_sp from another command interpreter, we could end up using the one from thread B. Since the only reason we are storing the value in m_delegate_command_sp is to keep the shared pointer alive, we should just return the shared pointer. So I would suggest removing DoGetDelegateCommand() and just have: ``` virtual llvm::Expected<lldb::CommandObjectSP> GetDelegateCommand() = 0; ``` Don't store it. Each function that calls it will keep it around as long as needed. If we use expected then we don't need to store the m_delegate_error later... ================ Comment at: lldb/source/Interpreter/CommandObjectDelegate.cpp:16-19 + m_delegate_command_sp = DoGetDelegateCommand(); + if (m_delegate_command_sp) + return m_delegate_command_sp.get(); + return nullptr; ---------------- This caching isn't really doing anything. If we have two debuggers, each one has its own command interpreter and this can actually cause things to fail. If thread A calls this, then thread B, then thread A continues to use the one populated by thread B. See inline comment in header file. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90729/new/ https://reviews.llvm.org/D90729 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
