Thread safe singleton #5018
Thread safe singleton #5018
Conversation
ianna
commented
Aug 21, 2014
ianna
commented
- Thread safe Singleton
- DDAxis map initialization at construction
|
@Dr15Jones and @ktf - Please, review the changes to Singleton class. Comments are welcome. It seems to me some of the singletons (as DDAxes) can be simplified to a static const map, for example. DDRoot friendship will be reviewed next. Thanks, Yana |
| static std::unique_ptr<value_type> m_instance; | ||
| static std::once_flag m_onceFlag; | ||
| Singleton(void); | ||
| Singleton(const Singleton&); |
There was a problem hiding this comment.
To stop the compiler from creating this, you can now say
Singleton(const Singleton&) = delete;
|
@ianna Your use of |
|
A new Pull Request was created by @ianna for CMSSW_7_2_X. Thread safe singleton It involves the following packages: DetectorDescription/Base @civanch, @Dr15Jones, @ianna, @mdhildreth, @cmsbuild, @nclopezo, @Degano, @ktf can you please review it and eventually sign? Thanks. |
| static std::unique_ptr<value_type> m_instance; | ||
| static std::once_flag m_onceFlag; | ||
| Singleton(void); | ||
| Singleton(const Singleton&) = delete; | ||
| Singleton& operator=(const Singleton &); |
There was a problem hiding this comment.
I assume you also want to get rid of the operator=
|
@Dr15Jones - Thanks, I've fixed the copy constructor and can remove std::call_once - I was not sure if all the compilers we use are fully C++11 compliant. |
|
Pull request #5018 was updated. @civanch, @Dr15Jones, @ianna, @mdhildreth, @cmsbuild, @nclopezo, @Degano, @ktf can you please check and sign again. |
|
Pull request #5018 was updated. @civanch, @Dr15Jones, @ianna, @mdhildreth, @cmsbuild, @nclopezo, @Degano, @ktf can you please check and sign again. |
| template <class I> | ||
| I& Singleton<I>::instance() { | ||
| static I value; | ||
| return value; | ||
| std::call_once(m_onceFlag, |
There was a problem hiding this comment.
Since this is not needed, please revert back to the previous implementation.
|
Pull request #5018 was updated. @civanch, @Dr15Jones, @ianna, @mdhildreth, @cmsbuild, @nclopezo, @Degano, @ktf can you please check and sign again. |
|
@Dr15Jones - Ok, done. BTW, why does static analyzer complain about it? https://cmssdt.cern.ch/SDT/jenkins-artifacts/ib-static-analysis/CMSSW_7_2_X_2014-08-21-1400/slc6_amd64_gcc481/llvm-analysis/report-7d22ee.html#EndPath |
|
+1 |
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_7_2_X IBs unless changes (tests are also fine). |
|
@ianna The static analyzer is complaining that the static is non const. That means multiple threads can simultaneously modify the statics memory which leads to a race condition. Only if the class 'I' is specially written such that all its member functions and member data are thread safe would that static be thread safe. So it isn't the initialization that the static analyzer is complaining about, it is the subsequent use of the object instance. |
