[rfile] Add support for reading TTree

[silverweed]

This Pull request:

adds TTree reading support in RFile by registering it to the internal TFile when calling Get<TTree>.

Checklist:

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

[hageboeck]

Could you close the file and check the tree after closing? It likely breaks, but is the tree still touchable (albeit in an invalid state)?
(I assume it's the latter)

Can the tree complain that its file is gone?

[silverweed]

Accessing the tree after closing the file causes a segfault - same as with TFile.
If we want to fix it it could not be an RFile-related fix but it should be either in TTree of TFile probably...

[hageboeck]

I fully understand, but we should ask ourselves the question whether handing out a unique_ptr<TTree> makes sense in that case. It promises something that we (as of now) cannot guarantee.
@dpiparo what do you think?

[hageboeck]

So maybe, as a way forward, we can see to merging this PR, but we should work on a solution later on that doesn't lead to crashes.

[silverweed]

I agree that the unique_ptr is somewhat semantically wrong, but I would refrain from compromising the API for this special case; I think we should absolutely try to better detect this on the TFile/TTree side and print out a nice error message rather than crashing (though I suspect if it were that simple it would have already been done a long time ago..)

[hageboeck]

Maybe it can be done in the RFile side:

• When destructing RFile, check the list of the underlying TFile.
• Finding something that inherits from TTree, you could call addDirectoryFunction(tree, nullptr);
• This should remove it from the list of managed objects, so the TTree should survive the file closing.

If TTree::SetDirectory(nullptr) does the right thing (a brief look at the code seems to go in the right direction), you should now have a standalone tree.

Is this worth trying in the scope of this PR?

[silverweed]

Perhaps, but a quick test revealed that probably that's not sufficient, and even if it were we would be paying a performance price for every RFile deletion, which is less than ideal...

[pcanal]

One thing (aka a hack) RFile could do is mark the TTree object as being on the stack (ie. inaccurate but with the desired side-effect), this would lead the Close of the TFIle to no longer attempt to delete the TFile. [Side note: in essence the TFile holds a link that is akin to a shared_ptr while the user hold a link that akin to a weak_ptr ... except that the link is not nullptr-ed when the TTree is deleted ... Where the analogy fails is that user can also explicitly delete the TTree]

[silverweed]

Besides it being a hack, wouldn't the hack also cause the TTree not to be automatically written upon file close?
I'd rather keep the same behavior between TFile and RFile and solve (or at least mitigate) the problem on the lower level so it benefits both

[pcanal]

Besides it being a hack, wouldn't the hack also cause the TTree not to be automatically written upon file close?

No. It literally only affects whether the object is deleted or not when 'Clear'ing the list of owner objects. (Note with TFile the Write of the TTree meta-data (the object itself) is not implicit).

I'd rather keep the same behavior between TFile and RFile and solve (or at least mitigate) the problem on the lower level so it benefits both

The significant difference is that RFile returns a unique_ptr ... which actually already both mitigate and worsen the issue.

The problematic pattern is:

auto file = TFile::Open(....);
auto tree = file->Get<TTree>(...);
...
delete file;
delete tree; // or do anything with the TTree

The recommendation for TFile is to either not delete the TTree or delete it early:

auto file = TFile::Open(....);
auto tree = file->Get<TTree>(...);
...
delete tree; // This is fine, the TFile is warned of the deletion ... alternatively you can skip this line
delete file;

With RFile the variable are both unique_ptr and thus will be delete in the 'right' order unless the user intervenes and directly or indirectly does:

      auto file = ROOT::Experimental::RFile::Open(...);
      auto tree = file->Get<TTree>(...);
      // Calling either:
      file->Close();
      // or
      file.reset();
      // lead to a double delete by `tree`

If we support RFile writing TTree there is also question on when/where the Write of the TTree meta-data (the TTree object itself) is triggered (if it is during the deletion of the RFile then we are in trouble).

[github-actions]

Test Results

    22 files      22 suites   3d 17h 39m 21s ⏱️
 3 774 tests  3 773 ✅ 0 💤 1 ❌
75 059 runs  75 058 ✅ 0 💤 1 ❌

For more details on these failures, see this check.

Results for commit 71d7dfc.

♻️ This comment has been updated with latest results.

[hageboeck]

Approved if we fix the ruff failure and open an issue regarding the dangling pointer to the TTree if the file is closed.