Skip to content

Commit

Permalink
NET-590 Create rule S7133: Locks should be released within the same m…
Browse files Browse the repository at this point in the history
…ethod (#4449)
  • Loading branch information
github-actions[bot] authored Nov 15, 2024
1 parent 470973e commit 21cc340
Show file tree
Hide file tree
Showing 5 changed files with 116 additions and 0 deletions.
23 changes: 23 additions & 0 deletions rules/S7133/csharp/metadata.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
"title": "Locks should be released within the same method",
"type": "BUG",
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "5min"
},
"tags": [
],
"defaultSeverity": "Major",
"ruleSpecification": "RSPEC-7133",
"sqKey": "S7133",
"scope": "All",
"defaultQualityProfiles": ["Sonar way"],
"quickfix": "targeted",
"code": {
"impacts": {
"RELIABILITY": "HIGH"
},
"attribute": "CONVENTIONAL"
}
}
78 changes: 78 additions & 0 deletions rules/S7133/csharp/rule.adoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
This rule raises if you acquire a lock with one of the following methods, and do not release it within the same method.

* https://learn.microsoft.com/en-us/dotnet/api/system.threading.readerwriterlock.acquirereaderlock[ReaderWriterLock.AcquireReaderLock]
* https://learn.microsoft.com/en-us/dotnet/api/system.threading.readerwriterlock.acquirewriterlock[ReaderWriterLock.AcquireWriterLock]
* 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.enterupgradeablereadlock[ReaderWriterLockSlim.EnterUpgradeableReadLock]
* 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.tryenterupgradeablereadlock[ReaderWriterLockSlim.TryEnterUpgradeableReadLock]
* https://learn.microsoft.com/en-us/dotnet/api/system.threading.readerwriterlockslim.enterwritelock[ReaderWriterLockSlim.EnterWriteLock]
* https://learn.microsoft.com/en-us/dotnet/api/system.threading.readerwriterlockslim.tryenterwritelock[ReaderWriterLockSlim.TryEnterWriteLock]
* https://learn.microsoft.com/en-us/dotnet/api/system.threading.spinlock.enter[SpinLock.Enter]
* https://learn.microsoft.com/en-us/dotnet/api/system.threading.spinlock.tryenter[SpinLock.TryEnter]
* https://learn.microsoft.com/en-us/dotnet/api/system.threading.monitor.enter[Monitor.Enter]
* https://learn.microsoft.com/en-us/dotnet/api/system.threading.monitor.tryenter[Monitor.TryEnter]
== Why is this an issue?

Not releasing a lock in the same method where you acquire it, and releasing in another one, makes the code less clear and harder to maintain. You are also introducing the risk of not releasing a lock at all which can lead to deadlocks or exceptions.


=== Code examples

==== Noncompliant code example

[source,csharp,diff-id=1,diff-type=noncompliant]
----
public class Example
{
private static ReaderWriterLock rwLock = new();
public void AcquireWriterLock() =>
rwLock.AcquireWriterLock(2000); // Noncompliant, as the lock release is on the callers responsibilty
public void DoSomething()
{
// ...
}
public void ReleaseWriterLock() =>
rwLock.ReleaseWriterLock();
}
----

==== Compliant solution

[source,csharp,diff-id=1,diff-type=compliant]
----
public class Example
{
private static ReaderWriterLock rwLock = new();
public void DoSomething()
{
rwLock.AcquireWriterLock(2000); // Compliant, locks are released in the same method
try
{
// ...
}
finally
{
rwLock.ReleaseWriterLock();
}
}
}
----

== 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 Classs]
* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/api/system.threading.spinlock[SpinLock Struct]
* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/api/system.threading.monitor[Monitor Classs]

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

You should release this lock in the same method.

2 changes: 2 additions & 0 deletions rules/S7133/metadata.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
{
}
9 changes: 9 additions & 0 deletions rules/S7133/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[]

0 comments on commit 21cc340

Please sign in to comment.