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

NET-589 Create rule S7131: You should not release a write lock when a read lock has been acquired and vice versa #4433

Merged
merged 14 commits into from
Nov 14, 2024
Merged
23 changes: 23 additions & 0 deletions rules/S7131/csharp/metadata.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
"title": "You should not release a write lock when a read lock has been acquired",
pavel-mikula-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
pavel-mikula-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
"type": "BUG",
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "1h"
},
"tags": [
],
"defaultSeverity": "Major",
"ruleSpecification": "RSPEC-7131",
"sqKey": "S7131",
"scope": "All",
"defaultQualityProfiles": ["Sonar way"],
"quickfix": "infeasible",
"code": {
"impacts": {
"RELIABILITY": "HIGH"
},
"attribute": "LOGICAL"
}
}
102 changes: 102 additions & 0 deletions rules/S7131/csharp/rule.adoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@

When using https://learn.microsoft.com/en-us/dotnet/api/system.threading.readerwriterlock[ReaderWriterLock] and https://learn.microsoft.com/en-us/dotnet/api/system.threading.readerwriterlockslim[ReaderWriterLockSlim] for managing read and write locks, you should not release a read lock while holding a write lock and vice versa, otherwise you might have unexpected behavior.
The locks should be always correctly paired so that the shared resource is accessed safely.

This rule raises if:

* you acquire https://learn.microsoft.com/en-us/dotnet/api/system.threading.readerwriterlock.acquirewriterlock[ReaderWriterLock.AcquireWriterLock] or https://learn.microsoft.com/en-us/dotnet/api/system.threading.readerwriterlock.upgradetowriterlock[ReaderWriterLock.UpgradeToWriterLock] and then use https://learn.microsoft.com/en-us/dotnet/api/system.threading.readerwriterlock.releasereaderlock[ReaderWriterLock.ReleaseReaderLock] before releasing the writerlock
pavel-mikula-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
pavel-mikula-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
pavel-mikula-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
* you acquire https://learn.microsoft.com/en-us/dotnet/api/system.threading.readerwriterlockslim.enterwritelock[ReaderWriterLockSlim.EnterWriteLock] or https://learn.microsoft.com/en-us/dotnet/api/system.threading.readerwriterlockslim.tryenterwritelock[ReaderWriterLockSlim.TryEnterWriteLock] and then use https://learn.microsoft.com/en-us/dotnet/api/system.threading.readerwriterlockslim.exitreadlock[ReaderWriterLockSlim.ExitReadLock] before you release it
* you acquire https://learn.microsoft.com/en-us/dotnet/api/system.threading.readerwriterlock.acquirereaderlock[ReaderWriterLock.AcquireReaderLock] or https://learn.microsoft.com/en-us/dotnet/api/system.threading.readerwriterlock.downgradefromwriterlock[ReaderWriterLock.DowngradeFromWriterLock] and then use https://learn.microsoft.com/en-us/dotnet/api/system.threading.readerwriterlock.releasewriterlock[ReaderWriterLock.ReleaseWriterLock] before releasing the reader lock
* or you acquire https://learn.microsoft.com/en-us/dotnet/api/system.threading.readerwriterlockslim.enterreadlock[ReaderWriterLockSlim.EnterReadLock], https://learn.microsoft.com/en-us/dotnet/api/system.threading.readerwriterlockslim.tryenterreadlock[ReaderWriterLockSlim.TryEnterReadLock], https://learn.microsoft.com/en-us/dotnet/api/system.threading.readerwriterlockslim.enterupgradeablereadlock[ReaderWriterLockSlim.EnterUpgradeableReadLock] or https://learn.microsoft.com/en-us/dotnet/api/system.threading.readerwriterlockslim.tryenterupgradeablereadlock[ReaderWriterLockSlim.TryEnterUpgradeableReadLock] and then use https://learn.microsoft.com/en-us/dotnet/api/system.threading.readerwriterlockslim.exitwritelock[ReaderWriterLockSlim.ExitWriteLock] before you release it.
pavel-mikula-sonarsource marked this conversation as resolved.
Show resolved Hide resolved


== Why is this an issue?

If you use the `ReaderWriterLockSlim` class, you will get a https://learn.microsoft.com/en-us/dotnet/api/system.threading.lockrecursionexception[lockrecursionexception].
pavel-mikula-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
In the case of `ReaderWriterLock`, you'll get a runtime exception for trying to release a lock that is not owned by the calling thread.


=== Code examples

==== Noncompliant code example

[source,csharp,diff-id=1,diff-type=noncompliant]
----
class Example
{
private static ReaderWriterLock rwLock = new ReaderWriterLock();
pavel-mikula-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
private static int sharedResource = 0;
pavel-mikula-sonarsource marked this conversation as resolved.
Show resolved Hide resolved

static void Writer()
pavel-mikula-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
{
try
{
rwLock.AcquireWriterLock(2000);
pavel-mikula-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
sharedResource++;
}
finally
{
rwLock.ReleaseReaderLock(); // Noncompliant, will throw runtime exception
rwLock.ReleaseWriterLock();
pavel-mikula-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
}
}

static void Reader()
{
try
{
rwLock.AcquireReaderLock(2000);
}
finally
{
rwLock.ReleaseWriterLock(); // Noncompliant, will throw runtime exception
rwLock.ReleaseReaderLock();
pavel-mikula-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
----

==== Compliant solution

[source,csharp,diff-id=1,diff-type=compliant]
----
class Example
{
private static ReaderWriterLock rwLock = new ReaderWriterLock();
private static int sharedResource = 0;
pavel-mikula-sonarsource marked this conversation as resolved.
Show resolved Hide resolved

static void Writer()
{
try
{
rwLock.AcquireWriterLock(2000);
sharedResource++;
}
finally
{
rwLock.ReleaseWriterLock();
}
}

static void Reader()
{
try
{
rwLock.AcquireReaderLock(2000);
}
finally
{
rwLock.ReleaseReaderLock();
}
}
}
----

== Resources

=== Documentation

* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/api/system.threading.readerwriterlock[ReaderWriterLock Class]
* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/api/system.threading.readerwriterlockslim[ReaderWriterLockSlim]
pavel-mikula-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
pavel-mikula-sonarsource marked this conversation as resolved.
Show resolved Hide resolved

include::../rspecator.adoc[]
4 changes: 4 additions & 0 deletions rules/S7131/message.adoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
=== Message

You should not release this xxx lock as a xxx lock has been acquired.
pavel-mikula-sonarsource marked this conversation as resolved.
Show resolved Hide resolved

2 changes: 2 additions & 0 deletions rules/S7131/metadata.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
{
}
9 changes: 9 additions & 0 deletions rules/S7131/rspecator.adoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
ifdef::env-github,rspecator-view[]

'''
== Implementation Specification
(visible only on this page)

include::message.adoc[]

endif::env-github,rspecator-view[]
Loading