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

Memory leak for objects allocated in ctor of user defined class during I/O #16375

Open
1 task done
karuboniru opened this issue Sep 5, 2024 · 3 comments
Open
1 task done
Assignees
Labels

Comments

@karuboniru
Copy link

karuboniru commented Sep 5, 2024

Check duplicate issues.

  • Checked for duplicates

Description

If a class allocates memory in its ctor (and deallocates in dtor). And included in LinkDef.h without + after class name. During I/O, the ctor will be called but not the dtor.

Reproducer

Please refer to this code: https://github.com/karuboniru/root_memory_leak_report

It compiles to gen, read{,_old}. gen produces a .root file containing a tree of user defined class TestData. And read read_old read the tree to do something. read uses RDataFrame language and read_old uses old SetBranchAddress way.

In the constructor of TestData, it will allocate memory for class a_member. And deallocate it in its destructor. Any call to the constructor or destructor of class a_member will be logged.

If #pragma link C++ class TestData;, instead of #pragma link C++ class TestData+; is in the LinkDef.h, the read program will contain multiple call to a_member::a_member but only single or none call to a_member::~a_member, potenially causing memory leak per entry processed.

ROOT version

ROOT v6.32.04
Built for linuxx8664gcc on Aug 21 2024, 00:00:00
From heads/master@tags/v6-32-04
With clang version 18.1.8 (Fedora 18.1.8-3.fc41)
Binary directory: /usr/bin

Installation method

reproducable from self build (from copr), from fedora repo (official version) and ROOT from /cvmfs/sft.cern.ch/lcg/views/LCG_106/x86_64-el9-gcc13-opt/setup.sh

Operating system

Linux Fedora 41 and CentOS9

Additional context

This issue seems to be causing massive memory leak with GENIE. The GENIE related tools manually cleans the resources but it seems I can't do the same with RDataFrame.

@karuboniru karuboniru added the bug label Sep 5, 2024
@dpiparo
Copy link
Member

dpiparo commented Sep 5, 2024

Hi,

Could you explain why the classes are forced not to be selected without the "+", which brings many benefits, among which performance during the IO procedures?

@dpiparo dpiparo self-assigned this Sep 5, 2024
@karuboniru
Copy link
Author

In the case for GENIE neutrino generator, its a software with a long history, I am unable to trace back to the time when the LinkDef.h was written.

And it seems that adding the plus sign will break some compatibility with root files generated without the plus sign, so that's not going to be easy.

@pcanal
Copy link
Member

pcanal commented Sep 6, 2024

We are no longer updating the old I/O (the one you get without the +) as its test suite is minimal and we have no idea what users still using are relying on.

However you can straightforwardly migrate to the new I/O and solve the problem for you classes.

The result of the process applied to your example can be seen in the merge request against your example: karuboniru/root_memory_leak_report#1

This was created (beside the small tidying commits) by:

  1. Generating the dictionary without the +.
  2. Copy/pasting the Streamer member function for the class from the dictionary and switch to custom streamer by adding - to the LinDef: karuboniru/root_memory_leak_report@f454e98 (#1)

After this steps, there is no change in behavior yet.

  1. Go back to the previous code, Generate the dictionary with the '+', this gives the Streamer function that uses the new I/O. Save the code for the next step (Note that the code is entirely boiler plate and with 2 substitutions applies to any class.
  2. Go back to the new code.
  3. Increase the class version number
  4. Replace the write part of the Streamer function with the new I/O version. Merge the read part of the new I/O into to the read part, keeping the old code to only read the data from the old files.

This accomplish in karuboniru/root_memory_leak_report@37d09c9 (#1)?diff=split

After this step the new code will write and read new file without a memory lead. It will be able to read the old file with the same memory as before.

  1. Add the missing delete in the part of the Streamer reading the old file.

This is seen in karuboniru/root_memory_leak_report@8c6bb1d (#1)

After this we have:

  • Solved the memory leak
  • Enabled using the new I/O from then on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants