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

[I/O] Cannot read RVecs written with v6.24 with TTreeReader in current master #9136

Closed
eguiraud opened this issue Oct 18, 2021 · 9 comments · Fixed by #9750
Closed

[I/O] Cannot read RVecs written with v6.24 with TTreeReader in current master #9136

eguiraud opened this issue Oct 18, 2021 · 9 comments · Fixed by #9750

Comments

@eguiraud
Copy link
Member

Reading with TTree works.

This affects RDataFrame critically.

To reproduce at the prompt:

/tmp source /home/blue/ROOT/v6.24/_build/bin/thisroot.fish
/tmp root -l # with v6.24
root [0] ROOT::RDataFrame(10).Define("x", "ROOT::RVec<int>{1,2,3}").Snapshot("t", "f.root");
root [1] .q
/tmp source /home/blue/ROOT/master/_build/bin/thisroot.fish
/tmp root -l # now with master
root [0] ROOT::RDataFrame("t", "f.root").Display("x")->Print()
Fatal: 0 violated at line 1509 of `../io/io/src/TGenCollectionProxy.cxx'
aborting
#0  0x00007f4f9c4a99ea in wait4 () from /usr/lib/libc.so.6
#1  0x00007f4f9c427a2b in do_system () from /usr/lib/libc.so.6
#2  0x00007f4f9cdcc21c in TUnixSystem::Exec (this=0x55b0a9ebf800, shellcmd=0x55b0ae8f76d0 "/home/blue/ROOT/master/_build/etc/gdb-backtrace.sh 143932 1>&2") at ../core/unix/src/TUnixSystem.cxx:2120
#3  0x00007f4f9cdccb0e in TUnixSystem::StackTrace (this=0x55b0a9ebf800) at ../core/unix/src/TUnixSystem.cxx:2411
#4  0x00007f4f9cc5e089 in DefaultErrorHandler (level=6000, abort_bool=true, location=0x7f4f9c10b012 "", msg=0x55b0af523450 "0 violated at line 1509 of `../io/io/src/TGenCollectionProxy.cxx'") at ../core/base/src/TErrorDefaultHandler.cxx:174
#5  0x00007f4f9cd3a560 in ErrorHandler(Int_t, const char *, const char *, typedef __va_list_tag __va_list_tag *) (level=6000, location=0x7f4f9c10b012 "", fmt=0x7f4f9ce795d8 "%s violated at line %d of `%s'", ap=0x7ffe675d1310) at ../core/foundation/src/TError.cxx:152
#6  0x00007f4f9cd3ab94 in Fatal (location=0x7f4f9c10b012 "", fmt=0x7f4f9ce795d8 "%s violated at line %d of `%s'") at ../core/foundation/src/TError.cxx:249
#7  0x00007f4f9bf0a0fa in TGenCollectionProxy__VectorNext () at ../io/io/src/TGenCollectionProxy.cxx:1509
#8  0x00007f4f9c009167 in TStreamerInfoActions::GenericLooper::Numeric<int, int>::ConvertAction (items=0x55b0af2ac580, start=0x55b0af0e38d0, end=0x55b0af0e38dc, loopconf=0x7ffe675d1550) at ../io/io/src/TStreamerInfoActions.cxx:2447
#9  0x00007f4f9bff45fd in TStreamerInfoActions::GenericLooper::ConvertBasicType<int, int, TStreamerInfoActions::GenericLooper::Numeric>::Action (buf=..., start=0x55b0af0e38d0, end=0x55b0af0e38dc, loopconf=0x7ffe675d1550, config=0x55b0af070740) at ../io/io/src/TStreamerInfoActions.cxx:2467
#10 0x00007f4f9bf90e3b in TStreamerInfoActions::GenericLooper::ReadNumericalCollection<TStreamerInfoActions::GenericLooper::ConvertBasicType<int, int, TStreamerInfoActions::GenericLooper::Numeric> > (buf=..., addr=0x55b0af1eba40, conf=0x55b0af070740) at ../io/io/src/TStreamerInfoActions.cxx:2617
#11 0x00007f4f9bf6c208 in TStreamerInfoActions::GenericLooper::ReadCollectionBasicType<int> (buf=..., addr=0x55b0af1eba40, conf=0x55b0af070740) at ../io/io/src/TStreamerInfoActions.cxx:2650
#12 0x00007f4f9be4b627 in TStreamerInfoActions::TConfiguredAction::operator() (this=0x55b0af5360e0, buffer=..., object=0x55b0af1eba40) at ../io/io/inc/TStreamerInfoActions.h:123
#13 0x00007f4f9be49407 in TBufferFile::ApplySequence (this=0x55b0af1d9620, sequence=..., obj=0x55b0af1eba40) at ../io/io/src/TBufferFile.cxx:3572
#14 0x00007f4f826ebd17 in TBranchElement::ReadLeavesMember (this=0x55b0ae204b20, b=...) at ../tree/tree/src/TBranchElement.cxx:4499
#15 0x00007f4f826d0f25 in TBranch::GetEntry (this=0x55b0ae204b20, entry=0, getall=0) at ../tree/tree/src/TBranch.cxx:1691
#16 0x00007f4f826e47af in TBranchElement::GetEntry (this=0x55b0ae204b20, entry=0, getall=0) at ../tree/tree/src/TBranchElement.cxx:2714
#17 0x00007f4f82e8de93 in ROOT::Detail::TBranchProxy::Read (this=0x55b0af313140) at ../tree/treeplayer/inc/TBranchProxy.h:155
#18 0x00007f4f82f3269a in (anonymous namespace)::TCollectionLessSTLReader::GetCP (this=0x55b0af126050, proxy=0x55b0af313140) at ../tree/treeplayer/src/TTreeReaderArray.cxx:112
#19 0x00007f4f82f32746 in (anonymous namespace)::TCollectionLessSTLReader::GetSize (this=0x55b0af126050, proxy=0x55b0af313140) at ../tree/treeplayer/src/TTreeReaderArray.cxx:126
#20 0x00007f4f93fe3cf4 in ?? ()
#21 0x00007ffe675d19d0 in ?? ()
#22 0x000055b0af126050 in ?? ()
#23 0x000055b0af260770 in ?? ()
#24 0x000055b0af260770 in ?? ()
#25 0x00007ffe675d1bc0 in ?? ()
#26 0x00007f4f93fe1c4e in ?? ()
#27 0x0000000000000000 in ?? ()
@eguiraud
Copy link
Member Author

Need a test for this

@eguiraud
Copy link
Member Author

Files required to reproduce (an f.root produced as in the main ticket's description and a little program that reads it with TTreeReader):

repro.zip

@eguiraud
Copy link
Member Author

diff --git a/tree/treeplayer/inc/TTreeReaderArray.h b/tree/treeplayer/inc/TTreeReaderArray.h
index 1fdab62afa..564ddcc8d9 100644
--- a/tree/treeplayer/inc/TTreeReaderArray.h
+++ b/tree/treeplayer/inc/TTreeReaderArray.h
@@ -28,9 +28,7 @@ Base class of TTreeReaderArray.

    class TTreeReaderArrayBase: public TTreeReaderValueBase {
    public:
-      TTreeReaderArrayBase(TTreeReader* reader, const char* branchname,
-                           TDictionary* dict):
-         TTreeReaderValueBase(reader, branchname, dict) {}
+      TTreeReaderArrayBase(TTreeReader *reader, const char *branchname, TDictionary *dict);

       std::size_t GetSize() const { return fImpl->GetSize(GetProxy()); }
       Bool_t IsEmpty() const { return !GetSize(); }
diff --git a/tree/treeplayer/src/TTreeReaderArray.cxx b/tree/treeplayer/src/TTreeReaderArray.cxx
index fdadfcc5f7..7f3b548d5f 100644
--- a/tree/treeplayer/src/TTreeReaderArray.cxx
+++ b/tree/treeplayer/src/TTreeReaderArray.cxx
@@ -21,6 +21,7 @@
 #include "TFriendElement.h"
 #include "TFriendProxy.h"
 #include "TLeaf.h"
+#include "TFile.h"
 #include "TList.h"
 #include "TROOT.h"
 #include "TStreamerInfo.h"
@@ -369,6 +370,17 @@ namespace {

 ClassImp(TTreeReaderArrayBase);

+ROOT::Internal::TTreeReaderArrayBase::TTreeReaderArrayBase(TTreeReader *reader, const char *branchname,
+                                                           TDictionary *dict)
+   : TTreeReaderValueBase(reader, branchname, dict)
+{
+   auto *f = reader->GetTree()->GetCurrentFile();
+   const auto v = f->GetVersion();
+   if (v < 62600 && std::string(dict->GetName()).find("RAdoptAllocator") != std::string::npos &&
+       std::string(dict->GetName()).rfind("std::vector<", 0) != 0)
+      R__ASSERT(0);
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 /// Create the proxy object for our branch.

@eguiraud eguiraud modified the milestone: 6.26/00 Jan 27, 2022
@eguiraud
Copy link
Member Author

We decided we must have better diagnostics for v6.26, an actual fix would be nice to have.

@pcanal
Copy link
Member

pcanal commented Jan 29, 2022

The solution turned out to be straight-forward: #9750

pcanal added a commit to pcanal/root that referenced this issue Jan 30, 2022
…erInfoActions

This fixes root-project#9136.

Without this commit, SelectLooper would select the 'GenericLooper'
in the case of an emulated proxy for STL collection with (in the
name) a custom allocator.  However the GenericLooper only usable
for collection with a compiled collection proxy.

In particular, GenericLooper is calling the 'Next' function which
is not defined for vector ... and emulated collection.  Using
it lead to an assert complaining (right fully so) that an
'undefined' function is being called.
pcanal added a commit to pcanal/root that referenced this issue Jan 31, 2022
…erInfoActions

This fixes root-project#9136.

Without this commit, SelectLooper would select the 'GenericLooper'
in the case of an emulated proxy for STL collection with (in the
name) a custom allocator.  However the GenericLooper only usable
for collection with a compiled collection proxy.

In particular, GenericLooper is calling the 'Next' function which
is not defined for vector ... and emulated collection.  Using
it lead to an assert complaining (right fully so) that an
'undefined' function is being called.
pcanal added a commit that referenced this issue Jan 31, 2022
…erInfoActions

This fixes #9136.

Without this commit, SelectLooper would select the 'GenericLooper'
in the case of an emulated proxy for STL collection with (in the
name) a custom allocator.  However the GenericLooper only usable
for collection with a compiled collection proxy.

In particular, GenericLooper is calling the 'Next' function which
is not defined for vector ... and emulated collection.  Using
it lead to an assert complaining (right fully so) that an
'undefined' function is being called.
pcanal added a commit that referenced this issue Jan 31, 2022
…erInfoActions

This fixes #9136.

Without this commit, SelectLooper would select the 'GenericLooper'
in the case of an emulated proxy for STL collection with (in the
name) a custom allocator.  However the GenericLooper only usable
for collection with a compiled collection proxy.

In particular, GenericLooper is calling the 'Next' function which
is not defined for vector ... and emulated collection.  Using
it lead to an assert complaining (right fully so) that an
'undefined' function is being called.
pcanal added a commit that referenced this issue Jan 31, 2022
…erInfoActions

This fixes #9136.

Without this commit, SelectLooper would select the 'GenericLooper'
in the case of an emulated proxy for STL collection with (in the
name) a custom allocator.  However the GenericLooper only usable
for collection with a compiled collection proxy.

In particular, GenericLooper is calling the 'Next' function which
is not defined for vector ... and emulated collection.  Using
it lead to an assert complaining (right fully so) that an
'undefined' function is being called.
pcanal added a commit that referenced this issue Feb 1, 2022
…erInfoActions

This fixes #9136.

Without this commit, SelectLooper would select the 'GenericLooper'
in the case of an emulated proxy for STL collection with (in the
name) a custom allocator.  However the GenericLooper only usable
for collection with a compiled collection proxy.

In particular, GenericLooper is calling the 'Next' function which
is not defined for vector ... and emulated collection.  Using
it lead to an assert complaining (right fully so) that an
'undefined' function is being called.
pcanal added a commit that referenced this issue Feb 1, 2022
…erInfoActions

This fixes #9136.

Without this commit, SelectLooper would select the 'GenericLooper'
in the case of an emulated proxy for STL collection with (in the
name) a custom allocator.  However the GenericLooper only usable
for collection with a compiled collection proxy.

In particular, GenericLooper is calling the 'Next' function which
is not defined for vector ... and emulated collection.  Using
it lead to an assert complaining (right fully so) that an
'undefined' function is being called.
@pcanal
Copy link
Member

pcanal commented Feb 1, 2022

Reopening until the test is ready.

@pcanal pcanal reopened this Feb 1, 2022
@eguiraud
Copy link
Member Author

eguiraud commented Feb 1, 2022

I shared the test code with an input file above.

If it is urgent that I open a PR for it, please let me know and specify where you would like that test (sources, roottest, which subdirectory..).

@Axel-Naumann Axel-Naumann removed this from the 6.26/00 milestone Feb 1, 2022
@pcanal
Copy link
Member

pcanal commented Feb 1, 2022

Can you please add it to roottest? Thanks.

@eguiraud
Copy link
Member Author

eguiraud commented Apr 8, 2022

We will be testing this case with root-project/roottest#866

@eguiraud eguiraud closed this as completed Apr 8, 2022
pcanal added a commit to pcanal/root that referenced this issue Oct 4, 2022
…erInfoActions

This fixes root-project#9136.

Without this commit, SelectLooper would select the 'GenericLooper'
in the case of an emulated proxy for STL collection with (in the
name) a custom allocator.  However the GenericLooper only usable
for collection with a compiled collection proxy.

In particular, GenericLooper is calling the 'Next' function which
is not defined for vector ... and emulated collection.  Using
it lead to an assert complaining (right fully so) that an
'undefined' function is being called.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants