From 98b775ad69d72fa56ec2c5a52d4de40506272931 Mon Sep 17 00:00:00 2001 From: Aaron Jomy Date: Thu, 6 Aug 2026 20:57:59 +0200 Subject: [PATCH 1/3] [cling] Destroy the frontend on Interpreter teardown --- interpreter/cling/lib/Interpreter/CIFactory.cpp | 4 +++- interpreter/cling/lib/Interpreter/Interpreter.cpp | 6 ++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/interpreter/cling/lib/Interpreter/CIFactory.cpp b/interpreter/cling/lib/Interpreter/CIFactory.cpp index 6fec8ad234e84..fd0e845a6a8ae 100644 --- a/interpreter/cling/lib/Interpreter/CIFactory.cpp +++ b/interpreter/cling/lib/Interpreter/CIFactory.cpp @@ -1622,7 +1622,9 @@ namespace { for (auto& E : moduleExtensions) FrontendOpts.ModuleFileExtensions.push_back(E); - FrontendOpts.DisableFree = true; + // The clang driver adds -disable-free to every cc1 line. + // Interpreter::ShutDown() should free the frontend. + FrontendOpts.DisableFree = false; // Set up compiler language and target if (!SetupCompiler(CI.get(), COpts, InitLang, InitTarget)) diff --git a/interpreter/cling/lib/Interpreter/Interpreter.cpp b/interpreter/cling/lib/Interpreter/Interpreter.cpp index 2fc17556f4dc3..77bf4e346f508 100644 --- a/interpreter/cling/lib/Interpreter/Interpreter.cpp +++ b/interpreter/cling/lib/Interpreter/Interpreter.cpp @@ -625,11 +625,9 @@ namespace cling { CI->resetAndLeakPreprocessor(); CI->resetAndLeakSourceManager(); CI->resetAndLeakFileManager(); - } else { - CI->setPreprocessor(nullptr); - CI->setSourceManager(nullptr); - CI->setFileManager(nullptr); } + // ~CompilerInstance destroys the Preprocessor, SourceManager and + // FileManager in the right order. } LO.setCompilingModule(clang::LangOptions::CMK_None); From cfbe2b41e119026d3948d9fd67bf951064a7901b Mon Sep 17 00:00:00 2001 From: Aaron Jomy Date: Thu, 6 Aug 2026 20:57:59 +0200 Subject: [PATCH 2/3] [cling] Free retained JITLink allocations at interpreter teardown --- .../cling/lib/Interpreter/IncrementalJIT.cpp | 30 +++++++++++++------ 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/interpreter/cling/lib/Interpreter/IncrementalJIT.cpp b/interpreter/cling/lib/Interpreter/IncrementalJIT.cpp index d4ed021c4a099..32c2a34b2939c 100644 --- a/interpreter/cling/lib/Interpreter/IncrementalJIT.cpp +++ b/interpreter/cling/lib/Interpreter/IncrementalJIT.cpp @@ -263,7 +263,7 @@ namespace { bool needsToReserveAllocationSpace() override { return true; } }; - /// A JITLinkMemoryManager for Cling that never frees its allocations. + /// A JITLinkMemoryManager for Cling. class ClingJITLinkMemoryManager : public InProcessMemoryManager { public: using InProcessMemoryManager::InProcessMemoryManager; @@ -273,16 +273,28 @@ namespace { // Disabled until CallFunc is informed about unloading, and can // re-generate the wrapper (if the decl is still available). See // https://github.com/root-project/root/issues/10898 - - // We still have to release the allocations which resets their addresses - // to FinalizedAlloc::InvalidAddr, or the assertion in ~FinalizedAlloc - // will be unhappy... - for (auto &Alloc : Allocs) { - Alloc.release(); - } - // Pretend we successfully deallocated everything... + // + // Releasing the handles orphans each allocation's vector of JITLink + // dealloc actions. Retain them (required for CallFunc) and let the + // base class free everything when this manager is destroyed at + // interpreter teardown. + std::lock_guard G(m_RetainedMutex); + for (auto& Alloc : Allocs) + m_Retained.push_back(std::move(Alloc)); OnDeallocated(Error::success()); } + + ~ClingJITLinkMemoryManager() override { + if (!m_Retained.empty()) + InProcessMemoryManager::deallocate(std::move(m_Retained), + [](Error Err) { + consumeError(std::move(Err)); + }); + } + + private: + std::mutex m_RetainedMutex; + std::vector m_Retained; }; /// A DynamicLibrarySearchGenerator that uses ResourceTracker to remember From 7667b6fe55955215c60e2f9e6fc0d3184efeb760 Mon Sep 17 00:00:00 2001 From: Aaron Jomy Date: Sat, 8 Aug 2026 17:44:19 +0200 Subject: [PATCH 3/3] [cling] TEMP: trace interpreter teardown for the Windows CI crash --- .../cling/lib/Interpreter/Interpreter.cpp | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/interpreter/cling/lib/Interpreter/Interpreter.cpp b/interpreter/cling/lib/Interpreter/Interpreter.cpp index 77bf4e346f508..37e2f94ec7943 100644 --- a/interpreter/cling/lib/Interpreter/Interpreter.cpp +++ b/interpreter/cling/lib/Interpreter/Interpreter.cpp @@ -379,27 +379,41 @@ namespace cling { } } + +// Temporary teardown tracing for the Windows CI crash in rootcling +// (PR #23045). Remove before merge. +#define CLING_TEARDOWN_TRACE(POINT) \ + do { \ + fprintf(stderr, "### CLING-TEARDOWN-TRACE: %s\n", POINT); \ + fflush(stderr); \ + } while (0) Interpreter::~Interpreter() { + CLING_TEARDOWN_TRACE("~Interpreter enter"); // Do this first so m_StoredStates will be ignored if Interpreter::unload // is called later on. for (size_t i = 0, e = m_StoredStates.size(); i != e; ++i) delete m_StoredStates[i]; m_StoredStates.clear(); + CLING_TEARDOWN_TRACE("stored states cleared"); if (m_Executor) m_Executor->shuttingDown(); + CLING_TEARDOWN_TRACE("executor shuttingDown done"); // LookupHelper's ~Parser needs the PP from IncrParser's CI, so do this // first: m_LookupHelper.reset(); + CLING_TEARDOWN_TRACE("LookupHelper reset done"); ShutDown(); + CLING_TEARDOWN_TRACE("ShutDown returned"); // We want to keep the callback alive during the shutdown of Sema, CodeGen // and the ASTContext. For that to happen we shut down the IncrementalParser // explicitly, before the implicit destruction (through the unique_ptr) of // the callbacks. m_IncrParser.reset(nullptr); + CLING_TEARDOWN_TRACE("IncrementalParser reset done; ~Interpreter body end"); } Transaction* Interpreter::Initialize(bool NoRuntime, bool SyntaxOnly) { @@ -587,12 +601,15 @@ namespace cling { } void Interpreter::ShutDown() { + CLING_TEARDOWN_TRACE("ShutDown enter"); // Model the shutdown actions done in FrontendAction::EndSourceFile if (CompilerInstance* CI = getCIOrNull()) { CI->getDiagnostics().getClient()->EndSourceFile(); + CLING_TEARDOWN_TRACE("diag client EndSourceFile done"); if (CI->hasPreprocessor()) CI->getPreprocessor().EndSourceFile(); + CLING_TEARDOWN_TRACE("PP EndSourceFile done"); bool DisableFree = CI->getFrontendOpts().DisableFree; if (DisableFree) { @@ -600,9 +617,13 @@ namespace cling { CI->resetAndLeakASTContext(); llvm::BuryPointer(CI->takeASTConsumer().get()); } else { + CLING_TEARDOWN_TRACE("before setSema(nullptr)"); CI->setSema(nullptr); + CLING_TEARDOWN_TRACE("before setASTContext(nullptr)"); CI->setASTContext(nullptr); + CLING_TEARDOWN_TRACE("before setASTConsumer(nullptr)"); CI->setASTConsumer(nullptr); + CLING_TEARDOWN_TRACE("frontend freed"); } if (CI->getFrontendOpts().ShowStats) { @@ -617,7 +638,9 @@ namespace cling { // Cleanup the output streams, and erase the output files if instructed by // the FrontendAction. bool shouldEraseOutputFiles = CI->getDiagnostics().hasErrorOccurred(); + CLING_TEARDOWN_TRACE("before clearOutputFiles"); CI->clearOutputFiles(/*EraseFiles=*/shouldEraseOutputFiles); + CLING_TEARDOWN_TRACE("clearOutputFiles done"); LangOptions& LO = CI->getLangOpts(); if (LO.getCompilingModule() != clang::LangOptions::CMK_None) { @@ -631,6 +654,7 @@ namespace cling { } LO.setCompilingModule(clang::LangOptions::CMK_None); + CLING_TEARDOWN_TRACE("ShutDown exit"); } }