Uploaded image for project: 'ROOT'
  1. ROOT
  2. ROOT-9668

IMT code segfaults when branches are changed

    XMLWordPrintable

    Details

    • Type: Improvement
    • Status: Closed (View Workflow)
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: 6.16/00, 6.14/04, 6.14/06
    • Fix Version/s: 6.16/00, 6.14/08
    • Component/s: I/O
    • Labels:
      None

      Description

      CMS encountered a case where, when a branch is added to a Tree in the middle of processing, the IMT code for TTree::Flush triggers a segfault.  See https://its.cern.ch/jira/browse/CMSCOMPPR-3348

      From Chris Jones:

      The problem appears to be the TTree has an inconsistent list of branches. I'm running the job in the debugger with a debug version of ROOT. The relevant place for the crash is here:

      https://github.com/root-project/root/blob/v6-12-00-patches/tree/tree/src/TTree.cxx#L4998

       

      The branch being worked on is at entry `j` which is 909. However, if we get the size of `fSortedBranches` we get

           (gdb) print fSortedBranches.size()

           $9 = 893

       So clearly we are getting a value outside the range of the number of branches in the sorted list.

      I believe the problem is here:

      https://github.com/root-project/root/blob/v6-12-00-patches/tree/tree/src/TTree.cxx#L4971

      the fSortedBranch list is only guaranteed to be the same size as the internal list of branchs in the TTree

      https://github.com/root-project/root/blob/v6-12-00-patches/tree/tree/src/TTree.cxx#L4966

      the first time this routine is called. Since Andrea said they add more branches later, which can happen after a `TTree::FlushBaskets` is called, then `fSortedBranches` is no longer the correct size and we read off the end of the container.

      Seems that the fSortedBranch cache ought to be invalidated each time a branch is added or removed.  Looking quickly at the code, I expect it is possible to trigger the segfaults on reads too.

        Attachments

          Activity

            People

            • Assignee:
              dpiparo Danilo Piparo
              Reporter:
              bbockelm Brian Paul Bockelman
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:
                PlannedEnd:
                PlannedStart:
                Actual Start:
                Actual End: