root-project / root

The official repository for ROOT: analyzing, storing and visualizing big data, scientifically
https://root.cern
Other
2.65k stars 1.26k forks source link

TTreeCacheUnzip thread safety with prefetching enabled #10095

Open dan131riley opened 2 years ago

dan131riley commented 2 years ago

Describe the bug

TTreeCacheUnzip::GetUnzipBuffer() does a binary search on fSeekSort and uses the return value in the loop that follows:

https://github.com/root-project/root/blob/412b2e8d829ee0a378b2aeb407505b2dcbb2595e/tree/tree/src/TTreeCacheUnzip.cxx#L694

This assumes that fSeekSort is sorted and stable. Neither assumption is safe in a multi-threaded program with prefetching enabled. This can lead to corruption of the fSeekSort array when prefetching calls the sort() method while GetUnzipBuffer() is searching for a block.

The call paths to the sort() method go through TTreeCacheUnzip::ReadBufferExt(), which locks fIOMutex. To work with prefetching enabled, TTreeCacheUnzip::UnzipBuffer() needs to lock fIOMutex in the sections where it needs fSeekSort to be stable, and reacquire the block location after any sections where the mutex isn't held. This looks to be non-trivial.

Expected behavior

TTreeCacheUnzip should work with prefetching without crashing.

To Reproduce

Non-trivial, but I can supply a CMSSW recipe if necessary.

Setup

  1. CMS ROOT6 master from CMSSW_12_3_ROOT6_X_2022-03-06-2300
  2. slc7_amd64_gcc10
  3. CMS CMSSW_12_3_ROOT6_X_2022-03-06-2300 in cvmfs

Additional context

We were hoping to use TTreeCacheUnzip as a way to increase parallelism in the CMS PoolInputSource, but this looks to be a blocker for that use.

pcanal commented 2 years ago

Can you supply a CMSSW recipe using local files? Thanks.