Skip to content

Coding- Replace Standard_Mutex with std::mutex and migrate to RAII locks#766

Merged
dpasukhi merged 30 commits intoOpen-Cascade-SAS:IRfrom
dpasukhi:mutex_migration
Nov 3, 2025
Merged

Coding- Replace Standard_Mutex with std::mutex and migrate to RAII locks#766
dpasukhi merged 30 commits intoOpen-Cascade-SAS:IRfrom
dpasukhi:mutex_migration

Conversation

@dpasukhi
Copy link
Copy Markdown
Member

  • Replace legacy Standard_Mutex usage across many modules with std::mutex.
  • Include where needed and remove <Standard_Mutex.hxx> includes.
  • Replace Standard_Mutex::Sentry / explicit Lock/Unlock with std::lock_guard or std::unique_lock.
  • Convert optional/heap mutex holders to std::unique_ptrstd::mutex and adapt locking accordingly.
  • Simplify several singleton initializations (remove manual double-checked locking where safe).
  • Use thread_local for per-thread flags instead of ad-hoc mutex protection.
  • Fix BVH_BuildQueue Fetch logic to preserve thread counters and wasBusy handling.
  • Remove obsolete TopTools_MutexForShapeProvider sources and update FILES.cmake.

This modernizes mutex usage, reduces dependency on custom mutex types and improves clarity of locking patterns.

@dpasukhi dpasukhi added this to the Release 8.0 milestone Oct 26, 2025
@dpasukhi dpasukhi requested a review from Copilot October 26, 2025 15:36
@dpasukhi dpasukhi self-assigned this Oct 26, 2025
@dpasukhi dpasukhi added 2. Enhancement New feature or request 1. Coding Coding rules, trivial changes and misprints labels Oct 26, 2025
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR modernizes mutex usage across the OCCT codebase by replacing legacy Standard_Mutex with C++ standard library std::mutex. The changes eliminate custom mutex implementations in favor of RAII-based locking patterns, simplify singleton initialization patterns, and convert thread-local flags to use thread_local keyword.

Key changes:

  • Replace Standard_Mutex::Sentry with std::lock_guard<std::mutex> and std::unique_lock<std::mutex>
  • Convert Handle(Standard_HMutex) to std::unique_ptr<std::mutex>
  • Use thread_local for per-thread variables instead of mutex-protected static variables
  • Remove obsolete TopTools_MutexForShapeProvider class and update build files

Reviewed Changes

Copilot reviewed 53 out of 53 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
Geom_BSplineSurface_1.cxx Remove unused Standard_Mutex.hxx include
TopTools_MutexForShapeProvider.* Remove obsolete mutex provider class
TopTools/FILES.cmake Update build configuration to remove deleted files
BRepExtrema_DistShapeShape.cxx Convert Handle(Standard_HMutex) to unique_ptr
BRepCheck_*.cxx Replace mutex sentry with unique_lock pattern
BRepCheck_Result.* Convert member mutex to unique_ptr and update accessors
BOPTools_Parallel.hxx Replace Standard_Mutex with std::mutex member
Standard_Type.cxx Convert static mutex to std::mutex with lock_guard
Standard_StackTrace.cxx Update mutex accessor and usage patterns
Standard_MMgrOpt.* Replace member mutexes with std::mutex
Standard_ErrorHandler.cxx Convert global mutex accessor and usage
OSD_signal.cxx Replace global mutex with std::mutex
OSD_Parallel_Threads.cxx Update mutex member and usage
OSD_Environment.cxx Replace static mutex with std::mutex
NCollection_IncAllocator.* Convert mutex pointer to unique_ptr
NCollection_HeapAllocator.cxx Simplify singleton initialization
Message_. Replace member mutexes with std::mutex
BVH_BuildQueue.* Update mutex member and fix scope
XSControl_WorkSession.cxx Convert global mutex to function accessor
Interface_Category.cxx Update mutex accessor pattern
RWMesh_TriangulationReader.* Replace member mutex type
VrmlData_Scene.* Update member mutex and remove manual unlocks
StepSelect_StepType.cxx Replace static mutex with std::mutex
StepFile_Read.cxx Convert global mutex to function accessor
STEPSelections_*.cxx Replace static variables with thread_local
STEPEdit.cxx Update static mutex usage
STEPControl_Controller.cxx Replace mutex in initialization
STEPCAFControl_Controller.cxx Update mutex in Init method
RWGltf_CafReader.cxx Replace member mutex type
DE_Wrapper.* Update global mutex accessor signature
DE_PluginHolder.hxx Replace Sentry with unique_lock
TDF_DerivedAttribute.cxx Convert mutex accessor and usage
QABugs_20.cxx Remove unused include

Comment thread src/FoundationClasses/TKernel/Standard/Standard_ErrorHandler.cxx Outdated
Comment thread src/DataExchange/TKDEVRML/VrmlData/VrmlData_Scene.cxx
Comment thread src/FoundationClasses/TKMath/BVH/BVH_BuildQueue.cxx
Comment thread src/FoundationClasses/TKernel/OSD/OSD_Environment.cxx Outdated
Comment thread src/FoundationClasses/TKernel/Standard/Standard_ErrorHandler.cxx Outdated
@dpasukhi dpasukhi requested a review from Copilot October 28, 2025 10:28
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 95 out of 95 changed files in this pull request and generated 6 comments.

Comment thread src/Visualization/TKService/Media/Media_PlayerContext.cxx
Comment thread src/Visualization/TKService/Graphic3d/Graphic3d_MediaTextureSet.cxx Outdated
Comment thread src/Visualization/TKService/Graphic3d/Graphic3d_MediaTexture.hxx Outdated
Comment thread src/FoundationClasses/TKernel/Standard/Standard_ErrorHandler.cxx
Comment thread src/DataExchange/TKDEVRML/VrmlData/VrmlData_Scene.cxx
Comment thread src/DataExchange/TKDEVRML/VrmlData/VrmlData_Scene.cxx
@dpasukhi dpasukhi requested review from AtheneNoctuaPt and removed request for AtheneNoctuaPt October 28, 2025 10:32
@dpasukhi dpasukhi marked this pull request as ready for review October 29, 2025 20:03
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 95 out of 95 changed files in this pull request and generated 4 comments.

Comment thread src/Visualization/TKV3d/StdPrs/StdPrs_BRepTextBuilder.cxx
Comment thread src/Visualization/TKV3d/AIS/AIS_ViewController.cxx
Comment thread src/FoundationClasses/TKernel/OSD/OSD_signal.cxx
IBlock* myOrderedBlocks = nullptr; //!< Ordered list for store growing size blocks
unsigned int myBlockSize; //!< Block size to incremental allocations
unsigned int myBlockCount = 0; //!< Count of created blocks
std::unique_ptr<std::mutex> myMutex = nullptr; //!< Thread-safety mutex
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can potentially use std::optional to avoid dynamic allocation entirely.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, migration of some optimal pointers to std::optional planned.
But for now it is postponed. I will keep that comment for the future update

Comment thread src/FoundationClasses/TKernel/Standard/Standard_MMgrOpt.cxx Outdated
Standard_Boolean myBlind;
BRepCheck_DataMapOfShapeListOfStatus myMap;
mutable Handle(Standard_HMutex) myMutex;
mutable std::unique_ptr<std::mutex> myMutex;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can also potentially be turned into std::optional

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, migration of some optimal pointers to std::optional planned.
But for now it is postponed. I will keep that comment for the future update

Comment thread src/Visualization/TKService/Graphic3d/Graphic3d_MediaTextureSet.cxx Outdated
- Replace legacy Standard_Mutex usage across many modules with std::mutex.
- Include <mutex> where needed and remove <Standard_Mutex.hxx> includes.
- Replace Standard_Mutex::Sentry / explicit Lock/Unlock with std::lock_guard or std::unique_lock.
- Convert optional/heap mutex holders to std::unique_ptr<std::mutex> and adapt locking accordingly.
- Simplify several singleton initializations (remove manual double-checked locking where safe).
- Use thread_local for per-thread flags instead of ad-hoc mutex protection.
- Fix BVH_BuildQueue Fetch logic to preserve thread counters and wasBusy handling.
- Remove obsolete TopTools_MutexForShapeProvider sources and update FILES.cmake.

This modernizes mutex usage, reduces dependency on custom mutex types and improves clarity of locking patterns.
…which could cause double-destruction/undefined behavior.
…its to ensure proper error handling visibility.

- Migrate BVH_BuildQueue to std::mutex and std::lock_guard (RAII) and simplify Fetch/Enqueue implementations.
- Fix unique_lock construction in BRepCheck_Analyzer to use aFaceEdgeRes->GetMutex().
- Initialize TreatmentFunctor::Mutex properly (default construct instead of NULL).
- Add #include <unistd.h> on POSIX for mmap usage in Standard_MMgrOpt.
- Minor header fixes (Message_ProgressIndicator and others) and additional Standard_ErrorHandler includes in various modules.
Ensure signal symbols are available when compiling non-Windows sources by including <signal.h> earlier in OSD_signal.cxx.
…ders

- Make Standard_ErrorHandler::GetMutex() return a heap-allocated static std::mutex
  to avoid destruction-order issues during static deinitialization.
- Qualify GetMutex() definition in the .cxx and add its declaration & explanatory comment in the .hxx.
- Clean up Callback method declarations formatting in Standard_ErrorHandler.hxx.
- Add missing NCollection_Array1/NCollection_Shared includes to Select3D_SensitivePrimitiveArray.hxx.
CallHandler already acquires THE_SIGNAL_MUTEX, so remove the extra std::lock_guard
from TranslateSE to avoid double-locking and potential deadlock/reentrancy during
structured-exception translation.
… adopt shared locking

- Standard_ErrorHandler: remove GetMutex() and the old mutex pointer; introduce a namespace-local std::shared_mutex (THE_GLOBAL_MUTEX).
  Use std::shared_lock for read-only FindHandler path and std::unique_lock for modifying paths (constructor, Unlink, FindHandler with unlink). Add comments/TODO about possible thread_local optimization.

- Aspect_VKeySet (hxx/cxx): replace Standard_Mutex with std::shared_mutex; use std::shared_lock in const readers and std::lock_guard/std::unique_lock for writers.
  Add KeyDown_Unlocked / KeyUp_Unlocked helpers and thin locked wrappers to avoid double-locking.

- Graphic3d_MediaTexture / Graphic3d_MediaTextureSet (hxx/cxx): replace Handle(Standard_HMutex)/Standard_HMutex with std::mutex / std::mutex&; initialize mutex as std::mutex(); switch to std::unique_lock<std::mutex> for critical sections; tidy headers (include <mutex>, <Standard_MemoryUtils.hxx> as needed).

- AIS_ViewController: update lock to std::unique_lock<std::shared_mutex> when accessing myKeys.Mutex().

General: migrate away from custom mutex types to standard C++ mutexes, enable concurrent read access where beneficial, and simplify locking patterns.
- Replace Standard_Mutex member with std::mutex and adjust includes.
- Replace Standard_Mutex::Sentry usages with std::lock_guard<std::mutex>.
- Remove redundant lock in pushPlayEvent and add comment that caller must hold the mutex.
dpasukhi and others added 18 commits November 3, 2025 13:21
… adopt std::lock_guard

- Replace legacy Standard_Mutex members/types with std::mutex (header & implementation).
- Use std::lock_guard<std::mutex> for list/thread locking instead of manual Lock/Unlock or Standard_Mutex::Sentry.
- Update BVHThread::BVHMutex() to return std::mutex& and adjust lock()/unlock() calls.
- Add <mutex> include to the header and include Standard_ErrorHandler.hxx where needed.
- Remove unused <Standard_Mutex.hxx> include from AIS_ViewController.hxx.
…Prs modules

Replace legacy Standard_Mutex usage with std::mutex across:
- StdPrs_BRepFont (header/member/Mutex() accessor and RenderGlyph lock)
- StdPrs_BRepTextBuilder (use std::lock_guard for font mutex)
- StdPrs_WFShape (functor mutex type and locking)

Also update includes to <mutex> and mark mutex mutable where required.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Pasukhin Dmitry <pasuhinsvzn@gmail.com>
This reverts commit 1899df7.
- Add -D CMAKE_CXX_STANDARD=17 and -D BUILD_CPP_STANDARD="C++17" to both
  Windows and Linux configure steps in .github/actions/build-tinspector/action.yml.
- Remove unnecessary mutex locks in AIS_ViewController::handleViewOrientationKeys
  and StdPrs_BRepTextBuilder::Perform to avoid redundant locking / potential
  dead-locks.
Replace std::unique_lock with std::lock_guard in several places to simplify locking:
- DE_PluginHolder.hxx: ctor/dtor
- Graphic3d_MediaTexture.cxx: GetImage
- Graphic3d_MediaTextureSet.cxx: LockFrame, ReleaseFrame, SwapFrames

Also replace manual myMutex.lock()/unlock() in Standard_MMgrOpt::Free with a scoped std::lock_guard and a narrowed block scope to improve exception safety and avoid potential deadlocks.
- Replace volatile boolean flags with std::atomic<bool> in BRepExtrema_DistShapeShape,
  use atomic load/store (with memory_order) inside TreatmentFunctor and set up local
  atomic copies for the parallel functor, copying results back after the parallel loop.
- Add missing <mutex> include to XSControl_WorkSession for proper use of std::lock_guard.
- Remove unused <Standard_MemoryUtils.hxx> include from Graphic3d_MediaTextureSet.
Move Pause() and Resume() implementations to the source file, add Standard_EXPORT to their declarations and protect pushPlayEvent with std::lock_guard<std::mutex> to ensure proper synchronization.
Add explicit <mutex> includes for headers that use std::mutex to avoid
relying on transitive includes and ensure correct compilation with
stricter include ordering.
@dpasukhi dpasukhi merged commit 787bee3 into Open-Cascade-SAS:IR Nov 3, 2025
23 checks passed
@github-project-automation github-project-automation bot moved this from Todo to Done in Maintenance Nov 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1. Coding Coding rules, trivial changes and misprints 2. Enhancement New feature or request

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants