From 91065149ca9c06ff676b2d203138a65e9e681560 Mon Sep 17 00:00:00 2001 From: Chris Jones Date: Wed, 4 Sep 2013 08:40:30 -0500 Subject: [PATCH 1/3] Removed unused variable and use std::atomic when communicating with unix signal handler The variable my_sig_num_ was the only place where getSigNum was ever called and the value of the variable was never used after that. Both the variable and the function have been removed. The unix signal handler code no longer uses a mutex and instead uses a std::atomic to denote if an external user has requested that the framework stop processing events. --- FWCore/Framework/interface/EventProcessor.h | 1 - FWCore/Framework/src/EventProcessor.cc | 13 +++---------- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/FWCore/Framework/interface/EventProcessor.h b/FWCore/Framework/interface/EventProcessor.h index 38f1033a1b0bd..7fabd8dbada79 100644 --- a/FWCore/Framework/interface/EventProcessor.h +++ b/FWCore/Framework/interface/EventProcessor.h @@ -259,7 +259,6 @@ namespace edm { std::auto_ptr subProcess_; std::unique_ptr historyAppender_; - int my_sig_num_; std::unique_ptr fb_; boost::shared_ptr looper_; diff --git a/FWCore/Framework/src/EventProcessor.cc b/FWCore/Framework/src/EventProcessor.cc index 0207ef25ca2f0..ce159d19cba14 100644 --- a/FWCore/Framework/src/EventProcessor.cc +++ b/FWCore/Framework/src/EventProcessor.cc @@ -214,7 +214,6 @@ namespace edm { schedule_(), subProcess_(), historyAppender_(new HistoryAppender), - my_sig_num_(getSigNum()), fb_(), looper_(), principalCache_(), @@ -255,7 +254,6 @@ namespace edm { schedule_(), subProcess_(), historyAppender_(new HistoryAppender), - my_sig_num_(getSigNum()), fb_(), looper_(), principalCache_(), @@ -299,7 +297,6 @@ namespace edm { schedule_(), subProcess_(), historyAppender_(new HistoryAppender), - my_sig_num_(getSigNum()), fb_(), looper_(), principalCache_(), @@ -339,7 +336,6 @@ namespace edm { schedule_(), subProcess_(), historyAppender_(new HistoryAppender), - my_sig_num_(getSigNum()), fb_(), looper_(), principalCache_(), @@ -1244,12 +1240,9 @@ namespace edm { bool returnValue = false; // Look for a shutdown signal - { - boost::mutex::scoped_lock sl(usr2_lock); - if(shutdown_flag) { - returnValue = true; - returnCode = epSignal; - } + if(shutdown_flag) { + returnValue = true; + returnCode = epSignal; } return returnValue; } From 426ab05621ade0ceb7ac56e93ca082504d39836c Mon Sep 17 00:00:00 2001 From: Chris Jones Date: Wed, 4 Sep 2013 08:44:33 -0500 Subject: [PATCH 2/3] Switch from using a mutex to using a std::atomic when signaling shutdown request To be properly thread safe under C++11 we switched from using a volatile bool to a volatile std::atomic as the way to communicate an external shutdown request. The mutex usr2_lock was also removed since it actually was never used correctly anyway and the std::atomic does the work the mutex should have been used for. The function getSigNum() and its related global variables were removed since the function is no longer used anywhere. --- .../Utilities/interface/UnixSignalHandlers.h | 5 ++-- FWCore/Utilities/src/UnixSignalHandlers.cc | 29 ++----------------- 2 files changed, 5 insertions(+), 29 deletions(-) diff --git a/FWCore/Utilities/interface/UnixSignalHandlers.h b/FWCore/Utilities/interface/UnixSignalHandlers.h index 0a7e046e6a0c0..e4fd55076bde4 100644 --- a/FWCore/Utilities/interface/UnixSignalHandlers.h +++ b/FWCore/Utilities/interface/UnixSignalHandlers.h @@ -9,19 +9,18 @@ and manipulate Unix-style signal handling. ----------------------------------------------------------------------*/ #include +#include #include "boost/thread/thread.hpp" namespace edm { - extern boost::mutex usr2_lock; - extern volatile bool shutdown_flag; + extern volatile std::atomic shutdown_flag; extern "C" { void ep_sigusr2(int,siginfo_t*,void*); typedef void(*CFUNC)(int,siginfo_t*,void*); } - int getSigNum(); void disableAllSigs(sigset_t* oldset); void disableRTSigs(); void enableSignal(sigset_t* newset, int signum); diff --git a/FWCore/Utilities/src/UnixSignalHandlers.cc b/FWCore/Utilities/src/UnixSignalHandlers.cc index d4d425eece4dc..fe1b6138cf715 100644 --- a/FWCore/Utilities/src/UnixSignalHandlers.cc +++ b/FWCore/Utilities/src/UnixSignalHandlers.cc @@ -17,42 +17,19 @@ namespace edm { - boost::mutex usr2_lock; - //-------------------------------------------------------------- - volatile bool shutdown_flag = false; + volatile std::atomic shutdown_flag{false}; extern "C" { void ep_sigusr2(int,siginfo_t*,void*) { FDEBUG(1) << "in sigusr2 handler\n"; - shutdown_flag = true; + shutdown_flag.store(true); } } - -//-------------------------------------------------------------- - - boost::mutex signum_lock; - volatile int signum_value = -#if defined(__linux__) - SIGRTMIN; -#else - 0; -#endif - -//-------------------------------------------------------------- - - int getSigNum() - { - boost::mutex::scoped_lock sl(signum_lock); - int rc = signum_value; - ++signum_value; - return rc; - } - #define MUST_BE_ZERO(fun) if((fun) != 0) \ - { perror("UnixSignalHandlers::setupSignal: sig function failed"); abort(); } +{ perror("UnixSignalHandlers::setupSignal: sig function failed"); abort(); } //-------------------------------------------------------------- From 87c1583428a65b64178a14ac0008c0252978009f Mon Sep 17 00:00:00 2001 From: Chris Jones Date: Wed, 4 Sep 2013 08:53:20 -0500 Subject: [PATCH 3/3] Use a faster form of synchronization when checking for shutdown request Since the variable shutdown_flag is not being used to synchronize other variables we can use the memory_order_relaxed which requires less synchronization between CPUs. --- FWCore/Framework/src/EventProcessor.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/FWCore/Framework/src/EventProcessor.cc b/FWCore/Framework/src/EventProcessor.cc index ce159d19cba14..93c52245704ac 100644 --- a/FWCore/Framework/src/EventProcessor.cc +++ b/FWCore/Framework/src/EventProcessor.cc @@ -1240,7 +1240,7 @@ namespace edm { bool returnValue = false; // Look for a shutdown signal - if(shutdown_flag) { + if(shutdown_flag.load(std::memory_order_relaxed)) { returnValue = true; returnCode = epSignal; }