cmssw: MeasurementTrackerEvent is not thread safe

While looking through a helgrind log I found the following problem

TkStripMeasurementDet::empty(MeasurementTrackerEvent const&) const calls MeasurementTrackerEvent::stripData which returns a StMeasurementDetSet. The function empty then calls StMeasurementDetSet::detSet. The code for that call attempts to do lazy caching

const StripDetset & detSet(int i) const { if (ready_[i]) const_cast<StMeasurementDetSet*>(this)->getDetSet(i);     return detSet_[i]; }

The getDetSet call is

       void getDetSet(int i) {
         if(detIndex_[i]>=0) {
           detSet_[i].set(*handle_,handle_->item(detIndex_[i]));
           empty_[i]=false; // better be false already
           incAct();
         }  else { // we should not be here
           detSet_[i] = StripDetset();
           empty_[i]=true;  
         }
         ready_[i]=false;
         incSet();
       }

The call to set is

   template<typename T>
   inline void DetSet<T>::set(DetSetVector<T> const & icont,
 			     typename Container::Item const & item, bool update) {
     // if an item is being updated we wait
     if (update) icont.update(item);
     while(item.initializing()) nanosleep(0,0);
     m_data=&icont.data();
     m_id=item.id; 
     m_offset = item.offset; 
     m_size=item.size;
   }

The call to icont.update(item) is done in a correct way such that the modification to item is handled in a thread safe manner so that reading the member data of item after the update call will be correct. Unfortunately, the same can not be said for the data members of DetSet<T> nor StMeasurementDetSet::ready_ where these are pure race conditions and are undefined behavior. The compiler is allowed to set ready_ to false before setting the value of any of DetSet<T> member data (since they are all inlined together).

A remedy would be to change StMeasurementDetSet::ready_ to be of type std::vector<std::atomic<bool>> ready_. Then ready_ would properly protect the call into StMeasurementDetSet::detSet and synchronize the memory at the end of the call to StMeasurementDetSet::getDetSet. It would not, however, stop multiple calls into StMeasurementDetSet::getDetSet from all calling DetSet<T>::set.

About this issue

  • Original URL
  • State: open
  • Created 7 years ago
  • Comments: 17 (17 by maintainers)

Most upvoted comments

type tracking

@VinInn @Dr15Jones @makortel was this issue resolved?

is it still a “no” ?

@mmusich @vmariani @mtosi