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-590 Create rule S7133: Locks should be released within the same method #4449

Merged
merged 9 commits into from
Nov 15, 2024
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.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.readerwriterlock.acquirewriterlock[ReaderWriterLock.AcquireWriterLock]
* 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.enterupgradeablereadlock[ReaderWriterLockSlim.EnterUpgradeableReadLock]
* 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[]
Loading