Skip to content

Commit

Permalink
Modify rule S2445: Add C# specifics, Move Java specifics
Browse files Browse the repository at this point in the history
  • Loading branch information
antonioaversa committed Feb 13, 2023
1 parent 8a5ec88 commit e812315
Show file tree
Hide file tree
Showing 7 changed files with 62 additions and 5 deletions.
10 changes: 10 additions & 0 deletions rules/S2445/csharp/message.adoc
@@ -0,0 +1,10 @@
=== Message

"xxx" is not "private readonly", and should not be used for locking.

"xxx" is a local variable, and should not be used for locking.

Locking on a new instance is a no-op.

Strings can be interned, and should not be used for locking.

2 changes: 1 addition & 1 deletion rules/S2445/csharp/metadata.json
@@ -1,3 +1,3 @@
{

"title": "Blocks should be synchronized on \"private readonly\" fields"
}
47 changes: 47 additions & 0 deletions rules/S2445/csharp/rule-except-see.adoc
@@ -0,0 +1,47 @@
Locking on a class field synchronizes not on the field itself, but on the object assigned to it. So locking on a non-``++readonly++`` field makes it possible for the field's value to change while a thread is in a block locked on the old value. That would allow a second thread, locked on the new value, to enter the block at the same time.


The story is very similar for locking on a local variable or on a new instance; two different threads running the method in parallel would lock on two different object instances, completely undermining the synchronization.


== Noncompliant Code Example

[source,csharp]
----
private string color = "red";
private void DoSomething()
{
lock (color) // Noncompliant; lock is actually on object instance "red" referred to by the color variable
{
//...
color = "green"; // other threads now allowed into this block
// ...
}
lock (new object()) // Noncompliant this is a no-op.
{
// ...
}
}
----


== Compliant Solution

[source,csharp]
----
private string color = "red";
private readonly object lockObj = new object();
private void DoSomething()
{
lock (lockObj)
{
//...
color = "green";
// ...
}
}
----


4 changes: 3 additions & 1 deletion rules/S2445/csharp/rule.adoc
@@ -1,4 +1,6 @@
include::../rule.adoc[]
include::rule-except-see.adoc[]

include::../see.adoc[]

ifdef::env-github,rspecator-view[]

Expand Down
File renamed without changes.
2 changes: 1 addition & 1 deletion rules/S2445/java/rule.adoc
@@ -1,4 +1,4 @@
include::../rule-except-see.adoc[]
include::rule-except-see.adoc[]

include::see.adoc[]

Expand Down
2 changes: 0 additions & 2 deletions rules/S2445/rule.adoc
@@ -1,3 +1 @@
include::rule-except-see.adoc[]

include::see.adoc[]

0 comments on commit e812315

Please sign in to comment.