Skip to content

Conversation

@yomnes0
Copy link
Collaborator

@yomnes0 yomnes0 commented Jul 11, 2024

Add a shared mutex class

@yomnes0 yomnes0 marked this pull request as ready for review July 12, 2024 06:36
@maxsharabayko maxsharabayko added Type: Enhancement Indicates new feature requests [core] Area: Changes in SRT library core labels Jul 12, 2024
@maxsharabayko maxsharabayko added this to the v1.5.4 milestone Jul 12, 2024
@maxsharabayko
Copy link
Collaborator

maxsharabayko commented Jul 12, 2024

@yomnes0

  • Please review the style and indention formatting used in this PR. Those should follow the project style.
  • Please add unit tests for the class.


Mutex m_Mutex;
int m_iCountRead;
bool m_bWriterLocked;
Copy link
Collaborator

Choose a reason for hiding this comment

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

These two fields can be combined into one field. Please see /usr/include/c++/v1/shared_mutex.

Would be also nice that you use exactly the same public API as the standard one so that a prospective drop-in replacement with the standard one from C++17 can be done later.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I thought about merging those two fields. I however felt that it would hurt readibility for a negligible memory and performance gain.

API has been changed to match the standard one

Improved syntax
Changed lock/unlock functions name to match reference implementation
Add unit tests for SharedMutex
@yomnes0 yomnes0 marked this pull request as draft July 15, 2024 13:03
@yomnes0 yomnes0 marked this pull request as ready for review July 29, 2024 12:43
@maxsharabayko
Copy link
Collaborator

@yomnes0
The PR's status has been changed to "Ready for review", but only formatting comments have been addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[core] Area: Changes in SRT library core Type: Enhancement Indicates new feature requests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants