From d50382792c051783294f86dd8a6af5c31dc7bb1a Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Fri, 25 Apr 2025 18:00:44 -0500 Subject: [PATCH 1/2] [cling] The LookupHelper routines need the ROOT lock. Those routines are at the very least looking up information into Clang and thus need to prevent concurrent updates. Some of the routines can also sometimes induces changes in Clang (eg. template instantiation). --- core/clingutils/src/TClingUtils.cxx | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/core/clingutils/src/TClingUtils.cxx b/core/clingutils/src/TClingUtils.cxx index 59fb2f5137635..9b3105c6ba9a3 100644 --- a/core/clingutils/src/TClingUtils.cxx +++ b/core/clingutils/src/TClingUtils.cxx @@ -54,6 +54,7 @@ #include "cling/Interpreter/Transaction.h" #include "cling/Interpreter/Interpreter.h" #include "cling/Utils/AST.h" +#include "cling/Interpreter/EnterUserCodeRAII.h" #include "llvm/Support/Path.h" #include "llvm/Support/FileSystem.h" @@ -569,6 +570,9 @@ void TClingLookupHelper::GetPartiallyDesugaredName(std::string &nameLong) bool TClingLookupHelper::IsAlreadyPartiallyDesugaredName(const std::string &nondef, const std::string &nameLong) { + // We are going to use and possibly update the interpreter information. + LockCompilationDuringUserCodeExecutionRAII LCDUCER(*interp); + const cling::LookupHelper& lh = fInterpreter->getLookupHelper(); clang::QualType t = lh.findType(nondef.c_str(), ToLHDS(WantDiags())); if (!t.isNull()) { @@ -584,6 +588,9 @@ bool TClingLookupHelper::IsAlreadyPartiallyDesugaredName(const std::string &nond bool TClingLookupHelper::IsDeclaredScope(const std::string &base, bool &isInlined) { + // We are going to use and possibly update the interpreter information. + LockCompilationDuringUserCodeExecutionRAII LCDUCER(*interp); + const cling::LookupHelper& lh = fInterpreter->getLookupHelper(); const clang::Decl *scope = lh.findScope(base.c_str(), ToLHDS(WantDiags()), nullptr); @@ -618,6 +625,9 @@ bool TClingLookupHelper::GetPartiallyDesugaredNameWithScopeHandling(const std::s if (fAutoParse) fAutoParse(tname.c_str()); + // We are going to use and possibly update the interpreter information. + LockCompilationDuringUserCodeExecutionRAII LCDUCER(*interp); + // Since we already check via other means (TClassTable which is populated by // the dictonary loading, and the gROOT list of classes and enums, which are // populated via TProtoClass/Enum), we should be able to disable the autoloading From 0397ec756ac003aa476ff084f6b1e13064533bfb Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Mon, 28 Apr 2025 09:28:29 -0500 Subject: [PATCH 2/2] cling: Add new header to access lock from clingutils --- core/clingutils/src/TClingUtils.cxx | 8 ++-- .../cling/Interpreter/InterpreterAccessRAII.h | 43 +++++++++++++++++++ 2 files changed, 47 insertions(+), 4 deletions(-) create mode 100644 interpreter/cling/include/cling/Interpreter/InterpreterAccessRAII.h diff --git a/core/clingutils/src/TClingUtils.cxx b/core/clingutils/src/TClingUtils.cxx index 9b3105c6ba9a3..0b1ad3499ae0d 100644 --- a/core/clingutils/src/TClingUtils.cxx +++ b/core/clingutils/src/TClingUtils.cxx @@ -54,7 +54,7 @@ #include "cling/Interpreter/Transaction.h" #include "cling/Interpreter/Interpreter.h" #include "cling/Utils/AST.h" -#include "cling/Interpreter/EnterUserCodeRAII.h" +#include "cling/Interpreter/InterpreterAccessRAII.h" #include "llvm/Support/Path.h" #include "llvm/Support/FileSystem.h" @@ -571,7 +571,7 @@ bool TClingLookupHelper::IsAlreadyPartiallyDesugaredName(const std::string &nond const std::string &nameLong) { // We are going to use and possibly update the interpreter information. - LockCompilationDuringUserCodeExecutionRAII LCDUCER(*interp); + cling::InterpreterAccessRAII LockAccess(*fInterpreter); const cling::LookupHelper& lh = fInterpreter->getLookupHelper(); clang::QualType t = lh.findType(nondef.c_str(), ToLHDS(WantDiags())); @@ -589,7 +589,7 @@ bool TClingLookupHelper::IsAlreadyPartiallyDesugaredName(const std::string &nond bool TClingLookupHelper::IsDeclaredScope(const std::string &base, bool &isInlined) { // We are going to use and possibly update the interpreter information. - LockCompilationDuringUserCodeExecutionRAII LCDUCER(*interp); + cling::InterpreterAccessRAII LockAccess(*fInterpreter); const cling::LookupHelper& lh = fInterpreter->getLookupHelper(); const clang::Decl *scope = lh.findScope(base.c_str(), ToLHDS(WantDiags()), nullptr); @@ -626,7 +626,7 @@ bool TClingLookupHelper::GetPartiallyDesugaredNameWithScopeHandling(const std::s if (fAutoParse) fAutoParse(tname.c_str()); // We are going to use and possibly update the interpreter information. - LockCompilationDuringUserCodeExecutionRAII LCDUCER(*interp); + cling::InterpreterAccessRAII LockAccess(*fInterpreter); // Since we already check via other means (TClassTable which is populated by // the dictonary loading, and the gROOT list of classes and enums, which are diff --git a/interpreter/cling/include/cling/Interpreter/InterpreterAccessRAII.h b/interpreter/cling/include/cling/Interpreter/InterpreterAccessRAII.h new file mode 100644 index 0000000000000..d300615fe4c42 --- /dev/null +++ b/interpreter/cling/include/cling/Interpreter/InterpreterAccessRAII.h @@ -0,0 +1,43 @@ +//------------------------------------------------------------------------------ +// CLING - the C++ LLVM-based InterpreterG :) +// author: Vassil Vassilev +// +// This file is dual-licensed: you can choose to license it under the University +// of Illinois Open Source License or the GNU Lesser General Public License. See +// LICENSE.TXT for details. +//------------------------------------------------------------------------------ + +#ifndef CLING_INTERPRETERACCESSRAII_H +#define CLING_INTERPRETERACCESSRAII_H + +#include "cling/Interpreter/Interpreter.h" +#include "cling/Interpreter/InterpreterCallbacks.h" + +namespace cling { +///\brief Locks and unlocks access to the interpreter. +struct InterpreterAccessRAII { + /// Callbacks used to un/lock. + InterpreterCallbacks* fCallbacks; + /// Info provided to UnlockCompilationDuringUserCodeExecution(). + void* fStateInfo = nullptr; + + InterpreterAccessRAII(InterpreterCallbacks* callbacks): + fCallbacks(callbacks) + { + if (fCallbacks) + // The return value is alway a nullptr. + fStateInfo = fCallbacks->LockCompilationDuringUserCodeExecution(); + } + + InterpreterAccessRAII(Interpreter& interp): + InterpreterAccessRAII(interp.getCallbacks()) {} + + ~InterpreterAccessRAII() + { + if (fCallbacks) + fCallbacks->UnlockCompilationDuringUserCodeExecution(fStateInfo); + } +}; +} + +#endif // CLING_ENTERUSERCODERAII_H