diff --git a/analyzers/its/expected/Automapper/AutoMapper--net461-S2445.json b/analyzers/its/expected/Automapper/AutoMapper--net461-S6507.json
similarity index 94%
rename from analyzers/its/expected/Automapper/AutoMapper--net461-S2445.json
rename to analyzers/its/expected/Automapper/AutoMapper--net461-S6507.json
index c9f27b7abe2..cd1d53d403f 100644
--- a/analyzers/its/expected/Automapper/AutoMapper--net461-S2445.json
+++ b/analyzers/its/expected/Automapper/AutoMapper--net461-S6507.json
@@ -1,7 +1,7 @@
{
"issues": [
{
-"id": "S2445",
+"id": "S6507",
"message": "Do not lock on local variable 'typeMap', use a readonly field instead.",
"location": {
"uri": "sources\Automapper\src\AutoMapper\Configuration\MapperConfiguration.cs",
diff --git a/analyzers/its/expected/Automapper/AutoMapper--netstandard2.0-S2445.json b/analyzers/its/expected/Automapper/AutoMapper--netstandard2.0-S6507.json
similarity index 94%
rename from analyzers/its/expected/Automapper/AutoMapper--netstandard2.0-S2445.json
rename to analyzers/its/expected/Automapper/AutoMapper--netstandard2.0-S6507.json
index c9f27b7abe2..cd1d53d403f 100644
--- a/analyzers/its/expected/Automapper/AutoMapper--netstandard2.0-S2445.json
+++ b/analyzers/its/expected/Automapper/AutoMapper--netstandard2.0-S6507.json
@@ -1,7 +1,7 @@
{
"issues": [
{
-"id": "S2445",
+"id": "S6507",
"message": "Do not lock on local variable 'typeMap', use a readonly field instead.",
"location": {
"uri": "sources\Automapper\src\AutoMapper\Configuration\MapperConfiguration.cs",
diff --git a/analyzers/rspec/cs/S2445_c#.html b/analyzers/rspec/cs/S2445_c#.html
index 21ef51420e3..36a28fa9354 100644
--- a/analyzers/rspec/cs/S2445_c#.html
+++ b/analyzers/rspec/cs/S2445_c#.html
@@ -3,8 +3,8 @@
- Locking on a non-
readonly
field makes it possible for the field’s value to change while a thread is in the code block locked on
the old value. This allows another thread to lock on the new value and access the same block concurrently.
- - Locking on a local variable or a new instance of an object can undermine synchronization because two different threads running the same method
- in parallel will potentially lock on different instances of the same object, allowing them to access the synchronized block at the same time.
+ - Locking on a new instance of an object undermines synchronization because two different threads running the same method in parallel will lock
+ on different instances of the same object, allowing them to access the synchronized block at the same time.
- Locking on a string literal is also dangerous since, depending on whether the string is interned or not, different threads may or may not
synchronize on the same object instance.
diff --git a/analyzers/rspec/cs/S6507_c#.html b/analyzers/rspec/cs/S6507_c#.html
new file mode 100644
index 00000000000..5b61f94376b
--- /dev/null
+++ b/analyzers/rspec/cs/S6507_c#.html
@@ -0,0 +1,35 @@
+Locking on a local variable can undermine synchronization because two different threads running the same method in parallel will potentially lock
+on different instances of the same object, allowing them to access the synchronized block at the same time.
+Noncompliant Code Example
+
+private void DoSomething()
+{
+ object local = new object();
+ // Code potentially modifying the local variable ...
+
+ lock (local) // Noncompliant
+ {
+ // ...
+ }
+}
+
+Compliant Solution
+
+private readonly object lockObj = new object();
+
+private void DoSomething()
+{
+ lock (lockObj)
+ {
+ //...
+ }
+}
+
+See
+
+
diff --git a/analyzers/rspec/cs/S6507_c#.json b/analyzers/rspec/cs/S6507_c#.json
new file mode 100644
index 00000000000..f01c758da85
--- /dev/null
+++ b/analyzers/rspec/cs/S6507_c#.json
@@ -0,0 +1,24 @@
+{
+ "title": "Blocks should not be synchronized on local variables",
+ "type": "BUG",
+ "status": "ready",
+ "remediation": {
+ "func": "Constant\/Issue",
+ "constantCost": "15min"
+ },
+ "tags": [
+ "cwe",
+ "multi-threading"
+ ],
+ "defaultSeverity": "Major",
+ "ruleSpecification": "RSPEC-6507",
+ "sqKey": "S6507",
+ "scope": "All",
+ "securityStandards": {
+ "CWE": [
+ 412,
+ 413
+ ]
+ },
+ "quickfix": "unknown"
+}
diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBeReadonly.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBeReadonly.cs
index 104b9592a54..524ac4eea34 100644
--- a/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBeReadonly.cs
+++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBeReadonly.cs
@@ -23,11 +23,14 @@ namespace SonarAnalyzer.Rules.CSharp;
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class LockedFieldShouldBeReadonly : SonarDiagnosticAnalyzer
{
- private const string DiagnosticId = "S2445";
+ private const string LockedFieldDiagnosticId = "S2445";
+ private const string LocalVariableDiagnosticId = "S6507";
+ private const string MessageFormat = "Do not lock on {0}, use a readonly field instead.";
- private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, "Do not lock on {0}, use a readonly field instead.");
+ private static readonly DiagnosticDescriptor LockedFieldRule = DescriptorFactory.Create(LockedFieldDiagnosticId, MessageFormat);
+ private static readonly DiagnosticDescriptor LocalVariableRule = DescriptorFactory.Create(LocalVariableDiagnosticId, MessageFormat);
- public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Rule);
+ public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(LockedFieldRule, LocalVariableRule);
protected override void Initialize(SonarAnalysisContext context) =>
context.RegisterNodeAction(CheckLockStatement, SyntaxKind.LockStatement);
@@ -37,27 +40,24 @@ private static void CheckLockStatement(SonarSyntaxNodeReportingContext context)
var expression = ((LockStatementSyntax)context.Node).Expression?.RemoveParentheses();
if (IsCreation(expression))
{
- ReportIssue("a new instance because is a no-op");
+ context.ReportIssue(Diagnostic.Create(LockedFieldRule, expression.GetLocation(), "a new instance because is a no-op"));
}
else
{
var lazySymbol = new Lazy(() => context.SemanticModel.GetSymbolInfo(expression).Symbol);
if (IsOfTypeString(expression, lazySymbol))
{
- ReportIssue("strings as they can be interned");
+ context.ReportIssue(Diagnostic.Create(LockedFieldRule, expression.GetLocation(), "strings as they can be interned"));
}
else if (expression is IdentifierNameSyntax && lazySymbol.Value is ILocalSymbol localSymbol)
{
- ReportIssue($"local variable '{localSymbol.Name}'");
+ context.ReportIssue(Diagnostic.Create(LocalVariableRule, expression.GetLocation(), $"local variable '{localSymbol.Name}'"));
}
else if (FieldWritable(expression, lazySymbol) is { } field)
{
- ReportIssue($"writable field '{field.Name}'");
+ context.ReportIssue(Diagnostic.Create(LockedFieldRule, expression.GetLocation(), $"writable field '{field.Name}'"));
}
}
-
- void ReportIssue(string message) =>
- context.ReportIssue(Diagnostic.Create(Rule, expression.GetLocation(), message));
}
private static bool IsCreation(ExpressionSyntax expression) =>
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/PackagingTests/RuleTypeMappingCS.cs b/analyzers/tests/SonarAnalyzer.UnitTest/PackagingTests/RuleTypeMappingCS.cs
index 919122b3e4a..a79296bd910 100644
--- a/analyzers/tests/SonarAnalyzer.UnitTest/PackagingTests/RuleTypeMappingCS.cs
+++ b/analyzers/tests/SonarAnalyzer.UnitTest/PackagingTests/RuleTypeMappingCS.cs
@@ -6431,7 +6431,7 @@ internal static class RuleTypeMappingCS
// ["S6504"],
// ["S6505"],
// ["S6506"],
- // ["S6507"],
+ ["S6507"] = "BUG",
// ["S6508"],
// ["S6509"],
// ["S6510"],