Skip to content

[cling] teardown: correctly free frontend and retained JITLink allocations - #23045

Open
aaronj0 wants to merge 3 commits into
root-project:masterfrom
aaronj0:cling-interpreter-teardown
Open

[cling] teardown: correctly free frontend and retained JITLink allocations#23045
aaronj0 wants to merge 3 commits into
root-project:masterfrom
aaronj0:cling-interpreter-teardown

Conversation

@aaronj0

@aaronj0 aaronj0 commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Based on the CppInterOp cling+ASan CI job running tests that create and destroy several interpreters per process, and LSan reports two leaks there:

In JITLink, with the upgrade to LLVM22, EHFrameRegistrationPlugin now pushes a deregistration WrapperFunctionCall into each allocation's FinalizedAllocInfo::DeallocActions vector. Our deallocate retains the code via FinalizedAlloc::release(), which orphans that vector's heap buffer.

For the frontend: ShutDown() leaks Sema/ASTContext through the DisableFree path and hides the pointers with llvm::BuryPointe. Its fixed buffer only holds 16 pointers, so once a process destroys a few interpreters it overflows and LSan starts reporting whole frontends, normally with a single interpreter until process exit would not hit this.

@aaronj0
aaronj0 requested a review from vgvassilev August 6, 2026 19:11
@aaronj0
aaronj0 requested review from dpiparo and hahnjo as code owners August 6, 2026 19:11
@aaronj0 aaronj0 self-assigned this Aug 6, 2026
@aaronj0
aaronj0 force-pushed the cling-interpreter-teardown branch from d604721 to d4b017a Compare August 6, 2026 19:17
@vgvassilev

Copy link
Copy Markdown
Member

Can you confirm root actually calls the cling interpreter destructor?

@aaronj0

aaronj0 commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Can you confirm root actually calls the cling interpreter destructor?

root.exe -l -b -q:

  Breakpoint 1.2, 0x00007ffff0d78870 in cling::Interpreter::~Interpreter() () from
  /home/maximus/ROOT/build-minimal-teardown/lib/libCling.so
  #0  0x00007ffff0d78870 in cling::Interpreter::~Interpreter() () from
  /home/maximus/ROOT/build-minimal-teardown/lib/libCling.so
  #1  0x00007ffff0bf7889 in std::default_delete<cling::Interpreter>::operator() (__ptr=<optimized out>,
  this=<optimized out>) at /usr/include/c++/13/bits/unique_ptr.h:93
  #2  std::unique_ptr<cling::Interpreter, std::default_delete<cling::Interpreter> >::~unique_ptr
  (this=0x5555555ff278, __in_chrg=<optimized out>) at /usr/include/c++/13/bits/unique_ptr.h:404
  #3  TCling::~TCling (this=0x5555555fefe0, __in_chrg=<optimized out>) at
  /home/maximus/ROOT/root-coff-fix/core/metacling/src/TCling.cxx:1694
  #4  0x00007ffff0bf79d1 in TCling::~TCling (this=0x5555555fefe0, __in_chrg=<optimized out>) at
  /home/maximus/ROOT/root-coff-fix/core/metacling/src/TCling.cxx:1694
  #5  0x00007ffff7c8b640 in TROOT::~TROOT (this=0x7ffff7edf720 <ROOT::Internal::GetROOT1()::alloc>,
  __in_chrg=<optimized out>) at /home/maximus/ROOT/root-coff-fix/core/base/src/TROOT.cxx:1172
  #6  0x00007ffff7c8986d in at_exit_of_TROOT () at
  /home/maximus/ROOT/root-coff-fix/core/base/src/TROOT.cxx:377
  #7  0x00007ffff7247a76 in __run_exit_handlers (status=0, listp=<optimized out>,
  run_list_atexit=run_list_atexit@entry=true, run_dtors=run_dtors@entry=true) at ./stdlib/exit.c:108
  #8  0x00007ffff7247bbe in __GI_exit (status=<optimized out>) at ./stdlib/exit.c:138
  #9  0x00007ffff7dab21f in TUnixSystem::Exit(int, bool) () from
  /home/maximus/ROOT/build-minimal-teardown/lib/libCore.so
  #10 0x00007ffff7c28097 in TApplication::Terminate(int) () from
  /home/maximus/ROOT/build-minimal-teardown/lib/libCore.so
  #11 0x00007ffff7fa2463 in TRint::Run(bool) () from
  /home/maximus/ROOT/build-minimal-teardown/lib/libRint.so
  #12 0x0000555555555303 in main ()

@vgvassilev

Copy link
Copy Markdown
Member

@smuzaffar can you check this pr in cmssw?

@aaronj0
aaronj0 force-pushed the cling-interpreter-teardown branch from d4b017a to e5bb010 Compare August 7, 2026 07:41
@aaronj0

aaronj0 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

force updated to trigger the CI which was down previously

@aaronj0 aaronj0 changed the title [cling] Interpreter teardown: correctly free frontend and retained JITLink allocations [cling] teardown: correctly free frontend and retained JITLink allocations Aug 7, 2026
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown

Test Results

    21 files      21 suites   3d 7h 37m 27s ⏱️
 3 835 tests  2 791 ✅   107 💤    937 ❌
72 466 runs  51 699 ✅ 2 200 💤 18 567 ❌

For more details on these failures, see this check.

Results for commit 7667b6f.

♻️ This comment has been updated with latest results.

@smuzaffar

smuzaffar commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

@smuzaffar can you check this pr in cmssw?

@vgvassilev , cmssw tests passed. There are few unit test failures but they are not related to this change

@vgvassilev

Copy link
Copy Markdown
Member

@smuzaffar can you check this pr in cmssw?

@vgvassilev , cmssw tests passed. There are few unit test failures but they are not related to this change

Thank you, @smuzaffar!

@aaronj0
aaronj0 force-pushed the cling-interpreter-teardown branch from e5bb010 to 7667b6f Compare August 8, 2026 15:52
Comment on lines -1625 to +1627
FrontendOpts.DisableFree = true;
// The clang driver adds -disable-free to every cc1 line.
// Interpreter::ShutDown() should free the frontend.
FrontendOpts.DisableFree = false;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAICT this is mostly an optimization where, instead of freeing, the "compiler" intentionally leaks because the OS will clean up the process anyway.

Comment on lines +288 to +292
if (!m_Retained.empty())
InProcessMemoryManager::deallocate(std::move(m_Retained),
[](Error Err) {
consumeError(std::move(Err));
});

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may still be too early, depending on which order the shutdown sequence is running and and whether something still has a pointer to one of the compiled functions...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants