From 46118c319c080a3da2a590b2bd80b8248560abae Mon Sep 17 00:00:00 2001 From: Chris Jones Date: Wed, 30 Jan 2019 13:31:13 -0600 Subject: [PATCH 1/2] Fix possible race in WaitingTaskHolder if reused If the same WaitingTaskHolder is reused indirectly by the task it was holding, it was possible to cause the task being held to be lost. --- FWCore/Concurrency/interface/WaitingTaskHolder.h | 11 ++++++++--- FWCore/Concurrency/src/WaitingTaskWithArenaHolder.cc | 11 ++++++++--- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/FWCore/Concurrency/interface/WaitingTaskHolder.h b/FWCore/Concurrency/interface/WaitingTaskHolder.h index 78cb9ea33a8ab..336fb0e7511da 100644 --- a/FWCore/Concurrency/interface/WaitingTaskHolder.h +++ b/FWCore/Concurrency/interface/WaitingTaskHolder.h @@ -81,10 +81,15 @@ namespace edm { if(iExcept) { m_task->dependentTaskFailed(iExcept); } - if(0==m_task->decrement_ref_count()){ - tbb::task::spawn(*m_task); - } + //spawn can run the task before we finish + // doneWaiting and some other thread might + // try to reuse this object. Resetting + // before spawn avoids problems + auto task = m_task; m_task = nullptr; + if(0==task->decrement_ref_count()){ + tbb::task::spawn(*task); + } } private: diff --git a/FWCore/Concurrency/src/WaitingTaskWithArenaHolder.cc b/FWCore/Concurrency/src/WaitingTaskWithArenaHolder.cc index 25d344cccb9a8..d890bfc316cf8 100644 --- a/FWCore/Concurrency/src/WaitingTaskWithArenaHolder.cc +++ b/FWCore/Concurrency/src/WaitingTaskWithArenaHolder.cc @@ -67,12 +67,17 @@ namespace edm { if (iExcept) { m_task->dependentTaskFailed(iExcept); } - if (0 == m_task->decrement_ref_count()) { + //enqueue can run the task before we finish + // doneWaiting and some other thread might + // try to reuse this object. Resetting + // before enqueue avoids problems + auto task = m_task; + m_task = nullptr; + if (0 == task->decrement_ref_count()) { // The enqueue call will cause a worker thread to be created in // the arena if there is not one already. - m_arena->enqueue( [m_task = m_task](){ tbb::task::spawn(*m_task); }); + m_arena->enqueue( [task = task](){ tbb::task::spawn(*task); }); } - m_task = nullptr; } // This next function is useful if you know from the context that From 532c79977bd0069a5b76122b57f3cf45dc94c92a Mon Sep 17 00:00:00 2001 From: Christopher Jones Date: Wed, 30 Jan 2019 14:18:19 -0600 Subject: [PATCH 2/2] Removed unused lambda capture in CSCGeometryESModule This fixes a clang warning. --- Geometry/CSCGeometryBuilder/plugins/CSCGeometryESModule.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Geometry/CSCGeometryBuilder/plugins/CSCGeometryESModule.cc b/Geometry/CSCGeometryBuilder/plugins/CSCGeometryESModule.cc index 006a2ab62505a..fbf18288b2649 100644 --- a/Geometry/CSCGeometryBuilder/plugins/CSCGeometryESModule.cc +++ b/Geometry/CSCGeometryBuilder/plugins/CSCGeometryESModule.cc @@ -124,7 +124,7 @@ void CSCGeometryESModule::initCSCGeometry_( const MuonGeometryRecord& record, st if ( useDDD_ ) { host->ifRecordChanges(record, - [this, &host, &record](auto const& rec) { + [&host, &record](auto const& rec) { host->clear(); edm::ESTransientHandle cpv; edm::ESHandle mdc;