root icon indicating copy to clipboard operation
root copied to clipboard

[cling] Fix lifetime of `ClingMMapper` [v6.30]

Open hahnjo opened this issue 1 year ago • 1 comments

The ClingMMapper must remain available until all ClingMemoryManagers are destructed, which is typically during shutdown of IncrementalJIT. This was not the case for the global object MMapperInstance that was introduced during the upgrade to LLVM 13 because libCling variables are destructed before running TROOT atexit handlers that shut down the JIT. In practice, it happened to work but this will change with the upgrade to LLVM 18 where we see crashes in some tests, potentially because of upstream commit https://github.com/llvm/llvm-project/commit/47f5c54f997a59bb2c65abe6b8b811f6e7553456

See also commits e0f6c04660 ("Prevent static destruction from ending DefaultMMapper too early") and 80c14bb948 ("Extend lifetime of SectionMemoryManager::DefaultMMapper, again") for the same problem that we previously patched in our copy of LLVM.

This differs from commit fd97311519a5d64f0110686db46e0d912503751c in master because LLVM doesn't support passing move-only lambdas.


This aims to backport https://github.com/root-project/root/pull/16314, but it's a bit more complicated with LLVM 13. We need to decide if we want this or just leave v6.30 and v6.28 alone.

hahnjo avatar Aug 27 '24 13:08 hahnjo

Test Results

     9 files       9 suites   1d 9h 57m 1s :stopwatch:  2 588 tests  2 588 :white_check_mark: 0 :zzz: 0 :x: 21 786 runs  21 786 :white_check_mark: 0 :zzz: 0 :x:

Results for commit 6b26004e.

github-actions[bot] avatar Aug 27 '24 16:08 github-actions[bot]

@dpiparo @vgvassilev I'd need your opinions here if we want to backport to 6.30 (and eventually 6.28). The changes are more intrusive because we cannot store the std::unique_ptr in the lambda...

hahnjo avatar Sep 06 '24 12:09 hahnjo