Corrupted uid in TRefArray::fUIDs[0] after adding object already referenced from another process

ROOT
1

ROOT Version: 6.22
Platform: x86 Linux v5.11
Compiler: gcc 10.2.0

Hi, rooters! Implementing idea to optimally organize experiment data, noticed wrong behaviour of TRefArray.

Task outline: to do some pre-analysis, namely, separately create tree with EntryTwo objs (additional info for each ev) based on bunch of root files (probably created in distinct root sessions) with tree containing at least two branches with EntryOne objs and link each EntryTwo obj with according EntryOne’s.
Restriction: treat files as read-only ones, that is there is no ability to change already collected root files during this processing.
Decision was to split setting-up referencing into two stages:

  1. in the course of generation of initial files assign each EntryOne obj used for filling tree to a dummy TRef, thus triggering referencing mechanism for each obj
  2. provide EntryTwo with TRefArray of EntryOne’s. Set up ref_arr properly while filling pre-analysis tree. Before writing tree on the disk sew old one to it as friend. After all, complete dataset will be organized in form of pre-analysis tree. To retrieve EntryOne’s fields in terms of new tree user should properly collect ‘raw’ files into the chain in addition to just reading pre-analysis tree.

Ok, let’s move! We gather root files into chain; declare EntryTwo obj for filling new tree and EntryOne pointers through which will deserialize initial entries; add these pointers (after getting first entry from chain) into EntryTwo’s ref_arr; now can iterate through all entries in chain and fill new tree on each step. Of course, we need to reset fPID of ref_arr every time switching from one actual tree in chain to another, cause pointers each time are setted to new EntryOne objs whose uids not necessarily contained in the same TProcessID as uids of previous ones. Simplest way I see is to reAdd pointers into ref_arr on each switching. According to code of method TRefArray::GetObjectUID(Int_t &uid, TObject *obj) ref_arr should manage this case checking own emptiness and reassign fPID to the TProcessID of newcomer. If to trace method’s control flow one could see that among other cases where kTRUE will be returned (consequent addition uid into TRefArray::fUIDs) there is another one when three conditions is fulfilled: obj has been referenced already; its TProcessID is not own one; current emptiness. Indeed, fPID is reassigned in this case, but &uid is LEFT UNCHANGED!!! As uid not assigned before method call, its value after the call is some garbage from stack section, that wil be entered into fUIDs[0] eventually.

Here is a macro to reproduce the problem:

class EntryOne : public TObject {
public:
    Int_t val;
ClassDef(EntryOne, 1)
};

class EntryTwo : public TObject {
public:
    TRefArray entriesOne;
    Int_t ev;
ClassDef(EntryTwo, 1)
};

// creates few root files with tree carrying 'raw' EntryOne objs:
//
// ones =====> ones_1 ~~~~> Int_t val
//      "====> ones_2 ~~~~> Int_t val
//
// Each time runs in separate session. 
void writeOnes(int n)
{
    TProcessID::GetPID()->Print();

    TFile out_f(Form("tmp%d.root", n), "recreate");
    TTree tr("ones", ""); 
    EntryOne eOne1, eOne2;
    TRef ref = &eOne1;
    ref = &eOne2;
    tr.Branch("ones_1", &eOne1);
    tr.Branch("ones_2", &eOne2);

    for (int ev=0; ev < 10; ++ev) {
        eOne1.val = n * 2 + ev % 2;
        eOne2.val = -eOne1.val;
        tr.Fill();
    }

    tr.Write();
    gROOT->CloseFiles();
    gSystem->Exit(0);
}

// construct pre-analysis tree on base of 'raw' files and save it in separate file.
// New tree comprises own branch for serializing EntryTwo objs and branches from old tree as a friend:
//
// twos ===> twos 
//      |     `~~~~> Int_t ev
//      |     `~~~~> TRefArray entriesOne
//      |                                                     
//      `----> ones_1   <**** entriesOne[0]
//      `----> ones_2   <**** entriesOne[1]
// 
//  Also runs in separate session
void writeTwos()
{
    TFile out_f("tmp.root", "recreate");
    TTree tr("twos", ""); 
    EntryTwo eTwo;
    tr.Branch("twos", &eTwo); 

    TChain *ones = new TChain("ones"); 
    ones->Add("tmp[0-9].root");
    tr.AddFriend(ones);
    EntryOne *eOne1 = 0, *eOne2 = 0;
    ones->SetBranchAddress("ones_1", &eOne1); 
    ones->SetBranchAddress("ones_2", &eOne2); 
    
    for (int ev=0; ev < ones->GetEntries(); ++ev) {
        eTwo.ev = ev;
        ones->GetEntry(ev);
        if (ones->GetTree()->GetReadEntry() == 0) {
            eTwo.entriesOne.Clear();
            eTwo.entriesOne.Add(eOne1);
            // eTwo.entriesOne.AddAt(eOne1, 0);
            eTwo.entriesOne.Add(eOne2);
        }
        tr.Fill();
    }

    out_f.cd();
    tr.Write();
    gROOT->CloseFiles();
    gSystem->Exit(0);
}


void readTwos()
{
    TTree *twos = (TTree*) TFile::Open("tmp.root")->Get("twos");
    TChain *ones = new TChain("ones"); 
    ones->Add("tmp[0-9].root");

    EntryTwo *two = 0;
    twos->SetBranchAddress("twos", &two);

    for (int ev=0; ev < twos->GetEntries(); ++ev) {
        twos->GetEntry(ev);
        cout << "two.ev: " << two->ev << '\t';
        
        TObject *one = two->entriesOne.At(0); 
        if (one) cout << "two.entriesOne[0].val: " << ((EntryOne*)one)->val << '\t';
        else cout << "Not derefed. uid=" << two->entriesOne.GetUID(0) << '\t';
        
        one = two->entriesOne.At(1); 
        if (one) cout << "two.entriesOne[1].val: " << ((EntryOne*)one)->val << endl;
        else cout << "Not derefed. uid=" << two->entriesOne.GetUID(1) << endl;
    }
    gSystem->Exit(0);
}

void test(int mode=0, int f_idx=0)
{
    switch (mode) {
        case 0: 
            for (int n=0; n<3; ++n) {
                gSystem->Exec(Form("root -l -b 'test.C(1, %d)'", n));
            }
            gSystem->Exec("root -l -b 'test.C(2)'");            
            gSystem->Exec("root -l -b 'test.C(3)'");            
            break;
        case 1: 
            writeOnes(f_idx);
            break;
        case 2:
            writeTwos();
            break;
        case 3:
            readTwos();
    }
    gSystem->Exit(0);
}

test.C (3.4 KB)

There is some workaround on commented line 74 that forces ref_arr to assign correct uid to fUIDs[0]. Two versions of output w/o and with execution of the line are listed below.

Without:

root [0] 
Processing test.C...
root [0] 
Processing test.C(1, 0)...
OBJ: TProcessID	ProcessID0	0640446c-84e2-11eb-88d0-0b00a8c0beef
root [0] 
Processing test.C(1, 1)...
OBJ: TProcessID	ProcessID0	0674fe14-84e2-11eb-be7e-0b00a8c0beef
root [0] 
Processing test.C(1, 2)...
OBJ: TProcessID	ProcessID0	06a93828-84e2-11eb-97f3-0b00a8c0beef
root [0] 
Processing test.C(2)...
root [0] 
Processing test.C(3)...
two.ev: 0	Not derefed. uid=32766	two.entriesOne[1].val: 0
two.ev: 1	Not derefed. uid=32766	two.entriesOne[1].val: -1
two.ev: 2	Not derefed. uid=32766	two.entriesOne[1].val: 0
two.ev: 3	Not derefed. uid=32766	two.entriesOne[1].val: -1
two.ev: 4	Not derefed. uid=32766	two.entriesOne[1].val: 0
two.ev: 5	Not derefed. uid=32766	two.entriesOne[1].val: -1
two.ev: 6	Not derefed. uid=32766	two.entriesOne[1].val: 0
two.ev: 7	Not derefed. uid=32766	two.entriesOne[1].val: -1
two.ev: 8	Not derefed. uid=32766	two.entriesOne[1].val: 0
two.ev: 9	Not derefed. uid=32766	two.entriesOne[1].val: -1
two.ev: 10	Not derefed. uid=32766	two.entriesOne[1].val: -2
two.ev: 11	Not derefed. uid=32766	two.entriesOne[1].val: -3
two.ev: 12	Not derefed. uid=32766	two.entriesOne[1].val: -2
two.ev: 13	Not derefed. uid=32766	two.entriesOne[1].val: -3
two.ev: 14	Not derefed. uid=32766	two.entriesOne[1].val: -2
two.ev: 15	Not derefed. uid=32766	two.entriesOne[1].val: -3
two.ev: 16	Not derefed. uid=32766	two.entriesOne[1].val: -2
two.ev: 17	Not derefed. uid=32766	two.entriesOne[1].val: -3
two.ev: 18	Not derefed. uid=32766	two.entriesOne[1].val: -2
two.ev: 19	Not derefed. uid=32766	two.entriesOne[1].val: -3
two.ev: 20	Not derefed. uid=32766	two.entriesOne[1].val: -4
two.ev: 21	Not derefed. uid=32766	two.entriesOne[1].val: -5
two.ev: 22	Not derefed. uid=32766	two.entriesOne[1].val: -4
two.ev: 23	Not derefed. uid=32766	two.entriesOne[1].val: -5
two.ev: 24	Not derefed. uid=32766	two.entriesOne[1].val: -4
two.ev: 25	Not derefed. uid=32766	two.entriesOne[1].val: -5
two.ev: 26	Not derefed. uid=32766	two.entriesOne[1].val: -4
two.ev: 27	Not derefed. uid=32766	two.entriesOne[1].val: -5
two.ev: 28	Not derefed. uid=32766	two.entriesOne[1].val: -4
two.ev: 29	Not derefed. uid=32766	two.entriesOne[1].val: -5

With:

root [0] 
Processing test.C...
root [0] 
Processing test.C(1, 0)...
OBJ: TProcessID	ProcessID0	38654784-84e3-11eb-9fbf-0b00a8c0beef
root [0] 
Processing test.C(1, 1)...
OBJ: TProcessID	ProcessID0	389b9352-84e3-11eb-b0a5-0b00a8c0beef
root [0] 
Processing test.C(1, 2)...
OBJ: TProcessID	ProcessID0	38d111ee-84e3-11eb-a6be-0b00a8c0beef
root [0] 
Processing test.C(2)...
root [0] 
Processing test.C(3)...
two.ev: 0	two.entriesOne[0].val: 0	two.entriesOne[1].val: 0
two.ev: 1	two.entriesOne[0].val: 1	two.entriesOne[1].val: -1
two.ev: 2	two.entriesOne[0].val: 0	two.entriesOne[1].val: 0
two.ev: 3	two.entriesOne[0].val: 1	two.entriesOne[1].val: -1
two.ev: 4	two.entriesOne[0].val: 0	two.entriesOne[1].val: 0
two.ev: 5	two.entriesOne[0].val: 1	two.entriesOne[1].val: -1
two.ev: 6	two.entriesOne[0].val: 0	two.entriesOne[1].val: 0
two.ev: 7	two.entriesOne[0].val: 1	two.entriesOne[1].val: -1
two.ev: 8	two.entriesOne[0].val: 0	two.entriesOne[1].val: 0
two.ev: 9	two.entriesOne[0].val: 1	two.entriesOne[1].val: -1
two.ev: 10	two.entriesOne[0].val: 2	two.entriesOne[1].val: -2
two.ev: 11	two.entriesOne[0].val: 3	two.entriesOne[1].val: -3
two.ev: 12	two.entriesOne[0].val: 2	two.entriesOne[1].val: -2
two.ev: 13	two.entriesOne[0].val: 3	two.entriesOne[1].val: -3
two.ev: 14	two.entriesOne[0].val: 2	two.entriesOne[1].val: -2
two.ev: 15	two.entriesOne[0].val: 3	two.entriesOne[1].val: -3
two.ev: 16	two.entriesOne[0].val: 2	two.entriesOne[1].val: -2
two.ev: 17	two.entriesOne[0].val: 3	two.entriesOne[1].val: -3
two.ev: 18	two.entriesOne[0].val: 2	two.entriesOne[1].val: -2
two.ev: 19	two.entriesOne[0].val: 3	two.entriesOne[1].val: -3
two.ev: 20	two.entriesOne[0].val: 4	two.entriesOne[1].val: -4
two.ev: 21	two.entriesOne[0].val: 5	two.entriesOne[1].val: -5
two.ev: 22	two.entriesOne[0].val: 4	two.entriesOne[1].val: -4
two.ev: 23	two.entriesOne[0].val: 5	two.entriesOne[1].val: -5
two.ev: 24	two.entriesOne[0].val: 4	two.entriesOne[1].val: -4
two.ev: 25	two.entriesOne[0].val: 5	two.entriesOne[1].val: -5
two.ev: 26	two.entriesOne[0].val: 4	two.entriesOne[1].val: -4
two.ev: 27	two.entriesOne[0].val: 5	two.entriesOne[1].val: -5
two.ev: 28	two.entriesOne[0].val: 4	two.entriesOne[1].val: -4
two.ev: 29	two.entriesOne[0].val: 5	two.entriesOne[1].val: -5

So, I interested in your opinion on my approach. Is it correct way to use referencing mechanism? Also, waiting for comments if I missed some subtle details. And finally, should I open new issue in Jira?

2

@pcanal Can you confirm that this is a bug?

3

Yes, this is a indeed a bug. Could you please submit an issue on github and then consider proposing a PR fixing this (and another one to roottest adding your test :slight_smile: )

Thanks,
Philippe.

4

This topic was automatically closed 14 days after the last reply. New replies are no longer allowed.