Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use lock_guard<mutex> in itkPlatformMultiThreaderWindows, ParallelSparseFieldLevelSetImageFilter, and PDEDeformable Registration #4180

Conversation

N-Dekker
Copy link
Contributor

Following C++ Core Guidelines, April 13, 2023, Use RAII, never plain lock()/unlock()

Follow-up to pull request #4173 commit c4c3710 "STYLE: Use lock_guard<mutex> in PlatformMultiThreader classes"

Follow-up to pull request InsightSoftwareConsortium#4173
commit c4c3710
"STYLE: Use `lock_guard<mutex>` in `PlatformMultiThreader` classes"
Following C++ Core Guidelines, April 13, 2023, "Use RAII, never plain `lock()`/`unlock()`"
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cp20-use-raii-never-plain-lockunlock

Also replaced raw `GlobalDataStruct` pointers by `std::unique_ptr` and removed
manual `delete globalData` statements from `ReleaseGlobalDataPointer` member
functions.
@github-actions github-actions bot added area:Core Issues affecting the Core module area:Registration Issues affecting the Registration module area:Segmentation Issues affecting the Segmentation module labels Aug 31, 2023
@N-Dekker
Copy link
Contributor Author

@dzenanz @blowekamp Thanks for your approval and your 👍 FYI, I found one more possible lock_guard<mutex> use case, in FFTWGlobalConfiguration::GetInstance():

m_PimplGlobals->m_CreationLock.lock();
// Need to make sure that during gaining access
// to the lock that some other thread did not
// initialize the singleton.
if (!m_PimplGlobals->m_Instance)
{
m_PimplGlobals->m_Instance = Self::New();
if (!m_PimplGlobals->m_Instance)
{
std::ostringstream message;
message << "itk::ERROR: "
<< "FFTWGlobalConfiguration"
<< " Valid FFTWGlobalConfiguration instance not created";
itk::ExceptionObject e_(__FILE__, __LINE__, message.str().c_str(), ITK_LOCATION);
throw e_; /* Explicit naming to work around Intel compiler bug. */
}
}
m_PimplGlobals->m_CreationLock.unlock();

This one is somewhat more interesting than the ones addressed so far by this PR, because it throws an exception between the lock() and the unlock() call. I think that's a bug. (It might mean that the mutex would never be unlocked anymore! 🙀) Anyway, I can make that a separate PR.

@N-Dekker N-Dekker marked this pull request as ready for review August 31, 2023 18:40
@N-Dekker
Copy link
Contributor Author

N-Dekker commented Sep 1, 2023

FYI, I found one more possible lock_guard<mutex> use case, in FFTWGlobalConfiguration::GetInstance()

...

Anyway, I can make that a separate PR.

Here it is, please have a look:

Both PRs can be processed independently.

@dzenanz dzenanz merged commit 873a724 into InsightSoftwareConsortium:master Sep 1, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:Core Issues affecting the Core module area:Registration Issues affecting the Registration module area:Segmentation Issues affecting the Segmentation module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants