Workaround a memory leak with old ROOT IO #417
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is related to a long existing bug in ROOT, with related workaround in GENIE traced back to this commit from Apr 2006. But such workaround by manually calling the
Clean
method after each entry of event being processed is not possible with working with the latest ROOT's RDataFrame analysis framework like I did in the reweighting testing code.The bug is that a class is being read from a ROOT file and:
#pragma link C++ class ClassName+;
.Since the bug is related to the old IO implementation in ROOT they are not going to fix it. But there are 2 approaches to fix or workaround this:
LinkDef.h
Streamer
to free the memory before the pointer is overwritten by TBuffer.The first one seemd to be feasible, but the fatal issue is that the old and new IO don't compatible. The root file generated with new IO method won't be recognized by old dictionaty and vice versa. Developers from ROOT did provide an example to show how to partially workaround the incompatibility with the version number tricks to keep old files readable, but it would still break the compatibility from the other side.
So this PR is the conservative approach, the second one, the IO and file format is kept the same. This should not be breaking anything, the only difficulty it would introduce is when we change the defination of
NtpMCEventRecord
we would have to update the definition ofNtpMCEventRecord::Streamer
here accordingly. (But at that time we can really move to the new IO from ROOT since change the definition of a class will introduce incompatibility)