### Author:  [@dzenanz](https://discourse.itk.org/u/dzenanz)
#### Post date: [August 6, 2018, 7:44pm UTC](https://discourse.itk.org/t/mutex-cleanup/1183/1 "2018-08-06T19:44:38Z")
Should we remove [MutexLock](https://itk.org/Doxygen/html/classitk_1_1MutexLock.html) and friends ([MutexLockHolder](https://itk.org/Doxygen/html/classitk_1_1MutexLockHolder.html), [ConditionVariable](https://itk.org/Doxygen/html/classitk_1_1ConditionVariable.html)), and replace them by C++11 equivalents ([std::mutex](https://en.cppreference.com/w/cpp/named_req/Mutex), [std::scoped\_lock](https://en.cppreference.com/w/cpp/thread/scoped_lock), [std::condition\_variable](https://en.cppreference.com/w/cpp/thread/condition_variable))?
A bump in major version number coupled with a switch to C++11 is an excellent opportunity to do so.
---
### Author:  [@blowekamp](https://discourse.itk.org/u/blowekamp)
#### Post date: [August 7, 2018, 12:50pm UTC](https://discourse.itk.org/t/mutex-cleanup/1183/2 "2018-08-07T12:50:10Z")
So as a philosophy, ITK has usually had a wrapper around system level things to enable more portable code and isolation ITK from changes and system differences. Therefore, perhaps we should just write a different back end to these ITK classes which is C++11 based.
TBB also has mutex’s and conditional variables, is it better to stick with one treading API then mix them?
---
### Author:  [@blowekamp](https://discourse.itk.org/u/blowekamp)
#### Post date: [August 7, 2018, 1:07pm UTC](https://discourse.itk.org/t/mutex-cleanup/1183/3 "2018-08-07T13:07:42Z")
I was looking at itk::AtomicInt vs std::atomic it. We are not very consistent in which one we are using. We should stabbing consistency convention for using a C++11 or system level or itk level wrapper for these types of things.
Here is a comparable change done to itk::AtomicInt:
>
But we have the fooling occurrences of `std::atomic`:
Documentation/ITK5MigrationGuide.md:you might use instead `std::atomic`.
Documentation/ITK5MigrationGuide.md:After, using `std::atomic`:
Documentation/ITK5MigrationGuide.md:std::atomic m\_NumVoxelsInsideMask;
Modules/Core/Common/include/itkMultiThreaderBase.h: std::atomic progress;
Modules/Core/Common/include/itkMultiThreaderBase.h: std::atomic pixelProgress;
Modules/Core/Common/src/itkTBBMultiThreader.cxx: std::atomic\< SizeValueType \> progress( 0 );
Modules/Core/Common/src/itkTBBMultiThreader.cxx: std::atomic pixelProgress = { 0 };
Particular of concert is the Migration Guide. Which do we recommend using? While there is the flexibility in remote modules, we should be consistent and clear especially in the Core group.
---
### Author:  [@dzenanz](https://discourse.itk.org/u/dzenanz)
#### Post date: [August 7, 2018, 1:22pm UTC](https://discourse.itk.org/t/mutex-cleanup/1183/4 "2018-08-07T13:22:38Z")
`itk::AtomicInt` and `itk::MutexLock` had a lot of sense before C++11. Now they are just a duplication of standard library’s functionality, and therefore make no sense. I intend to rewrite `itk::Mutex` and friends to be wrappers for `std::mutex` etc. But we should also deprecate them, along with `itk::AtomicInt` and any other system abstractions which were needed in ITK before C++11.
---
### Author:  [@matt.mccormick](https://discourse.itk.org/u/matt.mccormick)
#### Post date: [August 7, 2018, 1:49pm UTC](https://discourse.itk.org/t/mutex-cleanup/1183/5 "2018-08-07T13:49:46Z")
At one time, there was a greater need for wrapping standard library functionality since it was not implemented or buggy across compilers. I do not think we need wrappers if the standard library implementations are available across platforms and toolchains.
---
### Author:  [@blowekamp](https://discourse.itk.org/u/blowekamp)
#### Post date: [August 7, 2018, 2:02pm UTC](https://discourse.itk.org/t/mutex-cleanup/1183/6 "2018-08-07T14:02:20Z")
Do we have a list of things to be replaced by C++11 some place?
---
### Author:  [@matt.mccormick](https://discourse.itk.org/u/matt.mccormick)
#### Post date: [August 7, 2018, 2:06pm UTC](https://discourse.itk.org/t/mutex-cleanup/1183/7 "2018-08-07T14:06:31Z")
These should go in the ITK 5 migration guide.
---
### Author:  [@blowekamp](https://discourse.itk.org/u/blowekamp)
#### Post date: [August 7, 2018, 2:12pm UTC](https://discourse.itk.org/t/mutex-cleanup/1183/8 "2018-08-07T14:12:50Z")
I was referring more to a prioritized and approved TODO rather than a complete list.
---
### Author:  [@matt.mccormick](https://discourse.itk.org/u/matt.mccormick)
#### Post date: [August 7, 2018, 2:18pm UTC](https://discourse.itk.org/t/mutex-cleanup/1183/9 "2018-08-07T14:18:54Z")
There is no such list that I am aware of. If there items to be replaced by C++11, they can be discussed here, on Discourse.
---
### Author:  [@matt.mccormick](https://discourse.itk.org/u/matt.mccormick)
#### Post date: [August 7, 2018, 5:04pm UTC](https://discourse.itk.org/t/mutex-cleanup/1183/10 "2018-08-07T17:04:08Z")
We could mark `itk::AtomicInt` as deprecated and use `std::atomic` everwhere. At this point `std::atomic` has successfully proven itself as a cross-platform implementation for `itk::AtomicInt`.
---
### Author:  [@blowekamp](https://discourse.itk.org/u/blowekamp)
#### Post date: [August 7, 2018, 5:08pm UTC](https://discourse.itk.org/t/mutex-cleanup/1183/11 "2018-08-07T17:08:55Z")
There are a couple meta-programming items such as EnableIf, IsSame that have C++11 equivalents:
[https://en.cppreference.com/w/cpp/types/enable\_if](https://en.cppreference.com/w/cpp/types/enable_if)
[https://en.cppreference.com/w/cpp/types/is\_same](https://en.cppreference.com/w/cpp/types/is_same)
Then the who constraints mostly could be replaced with static\_assert and type traits. The old implementation is full of work arounds with old compilers and the compilation errors are not as clear.
---
### Author:  [@matt.mccormick](https://discourse.itk.org/u/matt.mccormick)
#### Post date: [August 7, 2018, 6:36pm UTC](https://discourse.itk.org/t/mutex-cleanup/1183/12 "2018-08-07T18:36:05Z")
👍 `std::enable_if` and `std::is_same` are good candidates, too.
---
### Author:  [@Simon](https://discourse.itk.org/u/Simon)
#### Post date: [August 10, 2018, 4:50pm UTC](https://discourse.itk.org/t/mutex-cleanup/1183/13 "2018-08-10T16:50:41Z")
Not chiming in with anything useful, but think migration to at c++11 and cleaning up consistency is a good goal!
---
### Author:  [@dzenanz](https://discourse.itk.org/u/dzenanz)
#### Post date: [August 31, 2018, 1:56pm UTC](https://discourse.itk.org/t/mutex-cleanup/1183/14 "2018-08-31T13:56:29Z")
Here is the [patch](http://review.source.kitware.com/#/c/23644/). I got stuck at a crash in `itkConditionVariable`. Can anyone see what I am doing wrong there?
---
### Author:  [@seanm](https://discourse.itk.org/u/seanm)
#### Post date: [August 31, 2018, 2:30pm UTC](https://discourse.itk.org/t/mutex-cleanup/1183/15 "2018-08-31T14:30:51Z")
If you are refactoring that code, I’d say it’s a must to try it all with Thread Sanitizer:
[https://clang.llvm.org/docs/ThreadSanitizer.html](https://clang.llvm.org/docs/ThreadSanitizer.html)
Sean
---
### Author:  [@dzenanz](https://discourse.itk.org/u/dzenanz)
#### Post date: [October 13, 2018, 5:14pm UTC](https://discourse.itk.org/t/mutex-cleanup/1183/16 "2018-10-13T17:14:37Z")
I finally got around to this again. The latest code is [here](https://github.com/dzenanz/ITK/commit/4d38dab80ce5a523f4759f9a3ef9490950eaf7ee).
It looks like ITK’s Mutex and friends have an incompatible API with respect to `std::mutex` and friends. `itk::MutexLock` is recursive, whereas `std::mutex` is not. There is `std::recursive_mutex`, but `std::condition_variable` works only with plain `std::mutex`. So I don’t think we can re-implement ITK mutex infrastructure to be simple wrappers for `std` stuff. The problem is this [public function](https://github.com/dzenanz/ITK/commit/4d38dab80ce5a523f4759f9a3ef9490950eaf7ee#diff-b734b9f98bec4713279dfd4c0ef3268cR103). Whatever it returns needs to be usable by `itk::ConditionVariable`.
Now I think it is best if we move all the classes which duplicate functionality in C++11 standard library into a compatibility module. Should we create `V4_Compatibility` module or something similarly named, or put it in [`Deprecated`](https://github.com/InsightSoftwareConsortium/ITK/tree/master/Modules/Compatibility/Deprecated)?
---
### Author:  [@dzenanz](https://discourse.itk.org/u/dzenanz)
#### Post date: [October 13, 2018, 5:17pm UTC](https://discourse.itk.org/t/mutex-cleanup/1183/17 "2018-10-13T17:17:24Z")
> [@seanm](#):
>
> Thread Sanitizer
Thanks for this suggestion. Here is some useful output for the latest code:
```plaintext
WARNING: ThreadSanitizer: lock-order-inversion (potential deadlock) (pid=20236)
Cycle in lock order graph: M931 (0x7ffc42ce0510) => M932 (0x7ffc42ce04e0) => M931
Mutex M932 acquired here while holding mutex M931 in thread T1:
#0 pthread_mutex_lock ??:? (ITKCommon1TestDriver+0x488727)
#1 __gthread_mutex_lock(pthread_mutex_t*) /usr/bin/../lib/gcc/x86_64-linux-gnu/5.4.0/../../../../include/x86_64-linux-gnu/c++/5.4.0/bits/gthr-default.h:748 (ITKCommon1TestDriver+0x91755c)
#2 std::mutex::lock() /usr/bin/../lib/gcc/x86_64-linux-gnu/5.4.0/../../../../include/c++/5.4.0/mutex:135 (ITKCommon1TestDriver+0x91755c)
#3 itk::SimpleMutexLock::Lock() /home/dzenan/ITK-git/Modules/Core/Common/include/itkMutexLock.h:66 (ITKCommon1TestDriver+0x91755c)
#4 ConditionVariableTestIncCount(void*) /home/dzenan/ITK-git/Modules/Core/Common/test/itkConditionVariableTest.cxx:50 (ITKCommon1TestDriver+0x91755c)
#5 ConditionVariableTestCallback(void*) /home/dzenan/ITK-git/Modules/Core/Common/test/itkConditionVariableTest.cxx:100 (ITKCommon1TestDriver+0x9177b2)
#6 itk::MultiThreaderBase::SingleMethodProxy(void*) /home/dzenan/ITK-git/Modules/Core/Common/src/itkMultiThreaderBase.cxx:440 (libITKCommon-5.0.so.1+0xc2d8f)
Hint: use TSAN_OPTIONS=second_deadlock_stack=1 to get more informative warning message
Mutex M931 acquired here while holding mutex M932 in thread T1:
#0 pthread_mutex_lock ??:? (ITKCommon1TestDriver+0x488727)
#1 __gthread_mutex_lock(pthread_mutex_t*) /usr/bin/../lib/gcc/x86_64-linux-gnu/5.4.0/../../../../include/x86_64-linux-gnu/c++/5.4.0/bits/gthr-default.h:748 (ITKCommon1TestDriver+0x620de2)
#2 std::mutex::lock() /usr/bin/../lib/gcc/x86_64-linux-gnu/5.4.0/../../../../include/c++/5.4.0/mutex:135 (ITKCommon1TestDriver+0x620de2)
#3 itk::SimpleMutexLock::Unlock() /home/dzenan/ITK-git/Modules/Core/Common/include/itkMutexLock.h:91 (ITKCommon1TestDriver+0x620de2)
#4 ConditionVariableTestIncCount(void*) /home/dzenan/ITK-git/Modules/Core/Common/test/itkConditionVariableTest.cxx:61 (ITKCommon1TestDriver+0x9175de)
#5 ConditionVariableTestCallback(void*) /home/dzenan/ITK-git/Modules/Core/Common/test/itkConditionVariableTest.cxx:100 (ITKCommon1TestDriver+0x9177b2)
#6 itk::MultiThreaderBase::SingleMethodProxy(void*) /home/dzenan/ITK-git/Modules/Core/Common/src/itkMultiThreaderBase.cxx:440 (libITKCommon-5.0.so.1+0xc2d8f)
```
---
### Author:  [@matt.mccormick](https://discourse.itk.org/u/matt.mccormick)
#### Post date: [October 16, 2018, 3:07pm UTC](https://discourse.itk.org/t/mutex-cleanup/1183/18 "2018-10-16T15:07:31Z")
> [@dzenanz](#):
>
> Now I think it is best if we move all the classes which duplicate functionality in C++11 standard library into a compatibility module. Should we create `V4_Compatibility` module or something similarly named, or put it in [`Deprecated`](https://github.com/InsightSoftwareConsortium/ITK/tree/master/Modules/Compatibility/Deprecated)?
Thanks for working on this @dzenanz.
I think these classes should be moved to `ITKDeprecated`. This will avoid confusion on their valid use and status. The transition is not difficult and is well-documented in the migration guide.
---
### Author:  [@Gordian](https://discourse.itk.org/u/Gordian)
#### Post date: [November 23, 2018, 10:32am UTC](https://discourse.itk.org/t/mutex-cleanup/1183/19 "2018-11-23T10:32:47Z")
Hi,
a follow up question and to wrap up the initial question:
Should I use [std::mutex](https://en.cppreference.com/w/cpp/named_req/Mutex) instead of [itk::SimpleFastMutexLock](https://itk.org/Doxygen/html/classitk_1_1SimpleFastMutexLock.html) in my ITK-4.13 code to make a (future) migration to ITKv5 easier?
---
### Author:  [@blowekamp](https://discourse.itk.org/u/blowekamp)
#### Post date: [November 23, 2018, 2:41pm UTC](https://discourse.itk.org/t/mutex-cleanup/1183/20 "2018-11-23T14:41:37Z")
It depends on the compatibility requirements of your project.
If your ITK 4.13 code/project already depends or required C++11 then I would embrace those features in your project.
If however you are writing an ITK remote module or want your code/project to be fully compatible with all ITKv4 compilers then you would need to still use ITKv4 C++03 style and headers.
[Next page](https://discourse.itk.org/t/mutex-cleanup/1183.md?page=2)