Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PyROOT] Memory leak in _TDirectoryFile_Get #16178

Open
1 task done
silverweed opened this issue Aug 6, 2024 · 2 comments
Open
1 task done

[PyROOT] Memory leak in _TDirectoryFile_Get #16178

silverweed opened this issue Aug 6, 2024 · 2 comments

Comments

@silverweed
Copy link
Contributor

Check duplicate issues.

  • Checked for duplicates

Description

The pythonization of TDirectoryFile has a _TDirectoryFile_Get function (used to implement Get()) that may return a TObject whose memory is not owned by either c++ or python, causing a memory leak.

I attempted to fix the issue with a simple ROOT.SetOwnership, but that causes some tests to fail, specifically pyunittests-pyroot-pyz-tdirectoryfile-attrsyntax-get.

Reproducer

  1. in python, open a TFile containing many different objects (e.g. histograms)
  2. Get all those objects using file.Get(name) in a for loop (relinquishing their reference at every iteration)
  3. see the memory increase forever.

You can test the bug with the following script:

import resource
import ROOT

def print_memory_usage(message):
    print(f"{message:50} {resource.getrusage(resource.RUSAGE_SELF).ru_maxrss}")

print_memory_usage("start")
histogram_names = open("histogram_names.txt").read().splitlines()
print_memory_usage("read histogram names")

fn = "NTUP_PHYSVAL.40023485._000001.pool.root.1"
with ROOT.TFile.Open(fn) as f:
    print_memory_usage("open ROOT file")
    for i, histogram_name in enumerate(sorted(histogram_names)):
        h = f.Get(histogram_name)
        if i % 1000 == 0:
            print_memory_usage(f"read {i+1} histograms")
    print_memory_usage("read all histograms")
print_memory_usage("outside context maneger (closing ROOT file)")

The files used were provided by the user who reported the issue, see the forum post

ROOT version

master

Installation method

built from source

Operating system

Linux

Additional context

This bug was reported on the forum

@guitargeek
Copy link
Contributor

This also affects ROOT 6.30 right? So it's not related to the recent PyROOT developments like the cppyy upgrade.

@silverweed
Copy link
Contributor Author

@guitargeek yes, it affects also 6.30.

@guitargeek guitargeek changed the title [pyroot] Memory leak in _TDirectoryFile_Get [PyROOT] Memory leak in _TDirectoryFile_Get Aug 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants