22b4470f2a
Update all links to C++ Core Guidelines to `e49158a`.
Refresh done using the following script and some manual edits:
db76e34e74/personal/fred-tingaud/rspec/refresh-cppcoreguidelines.py
When re-using this script, be mindful that:
- it does not cover `shared_content`
- it does not properly escape inline code in links (e.g., "[=]" or "`mutex`es")
- it does not change `C++` to `{cpp}` in link titles.
Co-authored-by: Marco Borgeaud <marco.borgeaud@sonarsource.com>
113 lines
3.2 KiB
Plaintext
113 lines
3.2 KiB
Plaintext
== Why is this an issue?
|
|
|
|
_Mutexes_ are synchronization primitives that allow managing concurrency using a mechanism of ``++lock++``/``++unlock++``.
|
|
|
|
While explicitly locking or unlocking a _mutex_ is possible, it is error-prone. This is particularly true in complex code paths (or with exceptions) where it is easy to have a mismatch between ``++lock++``s and ``++unlock++``s.
|
|
|
|
As a result, _mutexes_ should not be locked or unlocked manually.
|
|
|
|
|
|
Adopting the {cpp} RAII (_Resource Acquisition Is Initialization_) idiom solves this problem by creating an object that will lock the _mutex_ on creation and unlock it on destruction. Furthermore, using this idiom can also greatly improve the readability of the code.
|
|
|
|
|
|
Several classes are available as RAII wrappers:
|
|
|
|
* ``++std::scoped_lock++`` is the default, most efficient wrapper for simple cases (only available since {cpp}17)
|
|
* ``++std::lock_guard++`` is similar to ``++std::scoped_lock++``, but with fewer features. It should only be used if you don't have access to ``++std::scoped_lock++``.
|
|
* ``++std::unique_lock++`` allows more manual unlocking/locking again and should only be used when these features are needed, for instance, with condition variables.
|
|
|
|
|
|
=== Noncompliant code example
|
|
|
|
[source,cpp,diff-id=1,diff-type=noncompliant]
|
|
----
|
|
#include <mutex>
|
|
|
|
class DataItem;
|
|
|
|
class DataStore {
|
|
public:
|
|
bool store(const DataItem &dataItem);
|
|
bool has(const DataItem &dataItem);
|
|
};
|
|
|
|
DataStore sharedDataStore;
|
|
std::mutex sharedDataStoreMutex;
|
|
|
|
bool storeIfRelevantInSharedContext(const DataItem &dataItem) {
|
|
sharedDataStoreMutex.lock(); // Noncompliant
|
|
if (sharedDataStore.has(dataItem)) {
|
|
sharedDataStoreMutex.unlock(); // Noncompliant
|
|
return false;
|
|
}
|
|
bool result = sharedDataStore.store(dataItem);
|
|
sharedDataStoreMutex.unlock(); // Noncompliant
|
|
return result;
|
|
}
|
|
----
|
|
|
|
|
|
=== Compliant solution
|
|
|
|
[source,cpp,diff-id=1,diff-type=compliant]
|
|
----
|
|
#include <mutex>
|
|
|
|
class DataItem;
|
|
|
|
class DataStore {
|
|
public:
|
|
bool store(const DataItem &dataItem);
|
|
bool has(const DataItem &dataItem);
|
|
};
|
|
|
|
DataStore sharedDataStore;
|
|
std::mutex sharedDataStoreMutex;
|
|
|
|
bool storeIfRelevantInSharedContext(const DataItem &dataItem) {
|
|
std::scoped_lock<std::mutex> lock(sharedDataStoreMutex);
|
|
if (sharedDataStore.has(dataItem)) {
|
|
return false;
|
|
}
|
|
return sharedDataStore.store(dataItem);
|
|
}
|
|
----
|
|
|
|
|
|
== Resources
|
|
|
|
=== Documentation
|
|
|
|
* {cpp} reference - https://en.cppreference.com/w/cpp/language/raii[RAII]
|
|
|
|
=== External coding guidelines
|
|
|
|
* {cpp} Core Guidelines - https://github.com/isocpp/CppCoreGuidelines/blob/e49158a/CppCoreGuidelines.md#cp20-use-raii-never-plain-lockunlock[CP.20: Use RAII, never plain `lock()`/`unlock()`]
|
|
|
|
|
|
ifdef::env-github,rspecator-view[]
|
|
|
|
'''
|
|
== Implementation Specification
|
|
(visible only on this page)
|
|
|
|
=== Message
|
|
|
|
Use the RAII idiom instead of calling try_lock()/lock()/unlock() explicitly.
|
|
|
|
|
|
'''
|
|
== Comments And Links
|
|
(visible only on this page)
|
|
|
|
=== relates to: S5184
|
|
|
|
=== is related to: S5524
|
|
|
|
=== on 5 Nov 2019, 20:08:42 Loïc Joly wrote:
|
|
\[~geoffray.adde] Can you review my changes?
|
|
|
|
In particular, I removed one of your examples because I did not see the value it really brought... Do you agree?
|
|
|
|
endif::env-github,rspecator-view[]
|