[jmcarcell]

This Pull request:

Changes or fixes:

TDirectoryFile.Get in Python reads the object with GetObjectChecked() and binds the returned pointer without taking ownership. Objects that are not added to the directory's list are therefore never deleted. This covers non-TObjects like std::vector or ROOT::RNTuple, and TObjects without DirectoryAutoAdd, like TGraph or TObjString. Calling Get again creates a new copy each time, so memory grows. For example, reading the RNTuple anchor of a file:

Direct leak of 72 byte(s) in 1 object(s) allocated from:
    #0 0x7f8f6c92d85c in operator new(unsigned long)
    #1 0x7b8f61cd6d5d in ROOT::new_ROOTcLcLRNTuple(void*)

Python now takes ownership of the returned object, except when the directory holds it in its list. That is the case for histograms, trees and subdirectories, which keep being owned by the directory as before. The fallback for when no key is found is not changed, because it returns objects that are already in memory and owned by someone else. Checking the directory's list (a THashList) adds about 4 µs to each Get call, and this does not depend on the number of objects in the directory.

I added a test to tdirectory_tfile.py that checks that a TGraph, a TObjString and a std::vector read with Get are owned by Python, and that a TH1F, a TTree and a subdirectory are not and are still usable from the directory after the Python object is deleted. The test fails without the fix.

Checklist:

• tested changes locally
• updated the docs (if necessary)

I also checked that calling Get many times no longer makes memory grow for TGraph, TObjString, std::vector and RNTuple, and that tdirectory_tfile.py passes under ASan with no double deletes.

This PR fixes #16178

[vepadulano]

Thank you! In principle LGTM, but see some comments for the tests

[github-actions]

Test Results

    24 files      24 suites   3d 23h 48m 20s ⏱️
 3 896 tests  3 894 ✅ 0 💤 2 ❌
83 416 runs  83 414 ✅ 0 💤 2 ❌

For more details on these failures, see this check.

Results for commit 2b10086.

♻️ This comment has been updated with latest results.

[vepadulano]

Code changes LGTM. I will wait also for others to approve. Meanwhile, I have another suggestion to make the test future-proof.

[vepadulano]

This has changed meaning now: obj is kept in the scope of the loop, so this check will always be True. Before, you were checking that after deleting obj the file was still holding it alive, which depends on whether it's registered with the file or not.