View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0023286Open CASCADEOCCT:Foundation Classespublic2012-07-16 14:442013-04-29 15:21
Reporterkgv Assigned Toomy 
PrioritynormalSeverityminor 
Status closedResolutionfixed 
PlatformAOSL 
Product Version6.5.3 
Target Version6.6.0Fixed in Version6.6.0 
Summary0023286: Standard_Mutex behavior depends on platform
DescriptionStandard_Mutex provides system-independent API for thread locking functionality.
However it's behavior vary from system:
- On Windows critical sections used and allows recursive locks within current thread.
- POSIX threads mutex initialized with default options and non-recursive mutex created.

This is suggested to initialize recursive POSIX mutex instead to provide the similar behavior on different platforms:
> pthread_mutexattr_t anAttr;
> pthread_mutexattr_init(&anAttr);
> pthread_mutexattr_settype(&anAttr, PTHREAD_MUTEX_RECURSIVE);
> pthread_mutex_init(&&myMutex, &anAttr);
Steps To ReproduceStandard_Mutex aLock;
aLock.Lock();
aLock.Lock(); // recursive lock

aLock.Unlock();
aLock.Unlock();
TagsNo tags attached.
Test case numberNot needed

Activities

omy

2012-11-02 15:43

developer   ~0022082

Dear kgv,
Fix is integrated into branch СR23286.
Please, review.

kgv

2012-11-02 17:12

developer   ~0022086

Dear omy,

please consider removing of class Standard_Mutex::SentryNested.

It's feature to do not lock mutex (used only in BRepMesh_FastDiscretFace::RestoreStructureFromTriangulation)
could be implemented in Standard_Mutex::Sentry.

omy

2012-11-02 18:17

developer   ~0022090

Dear kgv,
The necessary class has been removed.
Please, review.

omy

2012-11-08 15:05

developer   ~0022149

Dear kgv,
I've added a check of NULL mutex to Sentry constructor.
Please, review.

kgv

2012-11-08 16:20

developer   ~0022151

Dear omy,

please do not commit incorrect uncompilable changes.

Please rename field nbLocked to more self-descriptional name and make it usage more obvious. Add description for doLock argument.

omy

2012-11-09 12:33

developer   ~0022171

Dear kgv,

I've fixed the errors and tested it on Windows.
See the commit message for details.
Please, review.

kgv

2012-11-09 12:52

developer   ~0022172

Dear omy,

>Standard_Mutex::Sentry aSentry(aMutex == NULL ? NULL : aMutex);
please remove this redundant if clause.
NULL mutex now handled in Standard_Mutex::Sentry constructor.

>static Standard_Mutex DummyMutex;
this variable is no in use anymore. Please remove it.

> //! Constructor - initializes the sentry object by reference to a
> //! mutex (which must be initialized) and locks the mutex if doLock is True
> // @param doLock defines whether mutex is able to be locked or not
> Sentry (Standard_Mutex* theMutex)
there no "doLock" parameter anymore! Please update documentation accordingly.

>- Standard_Mutex::SentryNested aSentry1 ( myMutexPools, myReentrant );
>+ Standard_Mutex::Sentry aSentry1 (myMutexPools);
Your modifications enforces mutex usage in Standard_MMgrOpt regardless from myReentrant flag.

omy

2012-11-09 14:31

developer   ~0022177

Dear kgv,
I've corrected the mistakes you mentioned.
Please, review

kgv

2012-11-09 14:36

developer   ~0022178

Dear omy,

>+ if(myReentrant)
>+ Standard_Mutex::Sentry aSentry (myMutexPools);
This is incorrect solution because sentry will be destroyed immediately at block end (single line after if statement in this case) according to C++ rules.

omy

2012-11-09 15:05

developer   ~0022185

Dear kgv,
I've corrected it.
Please, review.

kgv

2012-11-09 16:47

developer   ~0022192

Dear omy,

please replace this statement
> myReentrant == Standard_True ? &myMutexPools : NULL
with more readable
> myReentrant ? &myMutexPools : NULL

omy

2012-11-09 17:40

developer   ~0022193

Dear kgv,
I've fixed it.
Please, review.

kgv

2012-11-09 18:15

developer   ~0022194

Patch is ready for testing.

mkv

2012-11-13 12:47

tester   ~0022225

Last edited: 2012-11-21 16:52

 Dear BugMaster,
Branch CR23286 (and products from occt GIT master) was compiled on Linux and Windows platforms and tested.

Regressions:
Not detected

Improvements:
Not detected

Testing cases:
Not needed

abv

2012-11-21 11:55

manager   ~0022328

The branch has been rebased on last master under name CR23286_1; old branch CR23286 deleted

mkv

2012-11-22 11:35

tester   ~0022348

Dear BugMaster,
Branch CR23286_1 (and products from occt GIT master) was compiled on Linux and Windows platforms and tested.

Regressions:
Not detected

Improvements:
Not detected

Testing cases:
Not needed

Related Changesets

occt: master 49f38e37

2012-11-16 09:14:25

omy

Details Diff		 0023286: Standard_Mutex behavior depends on platform

Implemented recursive POSIX mutex instead of non-recursive,
Removed SentryNested class, implemented it's features into Sentry class
Added second constructor to Sentry class		 Affected Issues
0023286
mod - src/BRepMesh/BRepMesh_FastDiscretFace.cxx Diff File
mod - src/Standard/Standard_MMgrOpt.cxx Diff File
mod - src/Standard/Standard_Mutex.cxx Diff File
mod - src/Standard/Standard_Mutex.hxx Diff File

Issue History

Date Modified Username Field Change
2012-07-16 14:44 kgv New Issue
2012-07-16 14:44 kgv Assigned To => abv
2012-10-20 10:47 abv Assigned To abv => omy
2012-10-20 10:47 abv Status new => assigned
2012-10-20 10:47 abv Target Version 6.5.4 => 6.6.0
2012-11-02 15:43 omy Note Added: 0022082
2012-11-02 15:43 omy Assigned To omy => kgv
2012-11-02 15:43 omy Status assigned => resolved
2012-11-02 15:53 szv Summary Standard_Mutex behavior depends from platform => Standard_Mutex behavior depends on platform
2012-11-02 17:12 kgv Note Added: 0022086
2012-11-02 17:12 kgv Assigned To kgv => omy
2012-11-02 17:12 kgv Status resolved => assigned
2012-11-02 18:17 omy Note Added: 0022090
2012-11-02 18:17 omy Assigned To omy => kgv
2012-11-02 18:17 omy Status assigned => resolved
2012-11-08 15:05 omy Note Added: 0022149
2012-11-08 15:05 omy Status resolved => feedback
2012-11-08 16:20 kgv Note Added: 0022151
2012-11-08 16:20 kgv Status feedback => assigned
2012-11-08 16:20 kgv Assigned To kgv => omy
2012-11-09 12:33 omy Note Added: 0022171
2012-11-09 12:33 omy Assigned To omy => kgv
2012-11-09 12:33 omy Status assigned => resolved
2012-11-09 12:52 kgv Note Added: 0022172
2012-11-09 12:52 kgv Assigned To kgv => omy
2012-11-09 12:52 kgv Status resolved => assigned
2012-11-09 14:31 omy Note Added: 0022177
2012-11-09 14:31 omy Assigned To omy => kgv
2012-11-09 14:31 omy Status assigned => resolved
2012-11-09 14:36 kgv Note Added: 0022178
2012-11-09 14:36 kgv Assigned To kgv => omy
2012-11-09 14:36 kgv Status resolved => assigned
2012-11-09 15:05 omy Note Added: 0022185
2012-11-09 15:05 omy Assigned To omy => kgv
2012-11-09 15:05 omy Status assigned => resolved
2012-11-09 16:47 kgv Note Added: 0022192
2012-11-09 16:47 kgv Assigned To kgv => omy
2012-11-09 16:47 kgv Status resolved => assigned
2012-11-09 17:40 omy Note Added: 0022193
2012-11-09 17:40 omy Assigned To omy => kgv
2012-11-09 17:40 omy Status assigned => resolved
2012-11-09 18:15 kgv Note Added: 0022194
2012-11-09 18:15 kgv Assigned To kgv => bugmaster
2012-11-09 18:15 kgv Status resolved => reviewed
2012-11-12 17:28 mkv Assigned To bugmaster => mkv
2012-11-13 12:47 mkv Note Added: 0022225
2012-11-13 12:48 mkv Test case number => Not needed
2012-11-13 12:48 mkv Assigned To mkv => bugmaster
2012-11-13 12:48 mkv Status reviewed => tested
2012-11-21 11:55 abv Note Added: 0022328
2012-11-21 16:52 mkv Note Edited: 0022225
2012-11-22 11:35 mkv Note Added: 0022348
2012-11-26 15:45 omy Changeset attached => occt master 49f38e37
2012-11-26 15:45 omy Assigned To bugmaster => omy
2012-11-26 15:45 omy Status tested => verified
2012-11-26 15:45 omy Resolution open => fixed
2012-12-10 17:16 omy Changeset attached => occt master 49f38e37
2013-04-23 13:36 aiv Status verified => closed
2013-04-29 15:21 aiv Fixed in Version => 6.6.0