125 lines
4.0 KiB
Plaintext
125 lines
4.0 KiB
Plaintext
== Why is this an issue?
|
|
|
|
When you annotate a field with the https://learn.microsoft.com/en-us/dotnet/api/system.threadstaticattribute[`ThreadStatic` attribute], it is an indication that the value of this field is unique for each thread. But if you don't mark the field as `static`, then the `ThreadStatic` attribute is ignored.
|
|
|
|
The `ThreadStatic` attribute should either be removed or replaced with the use of https://learn.microsoft.com/en-us/dotnet/api/system.threading.threadlocal-1[`ThreadLocal<T>`] class, which gives a similar behavior for non-static fields.
|
|
|
|
== How to fix it
|
|
|
|
=== Code examples
|
|
|
|
==== Noncompliant code example
|
|
|
|
[source,csharp]
|
|
----
|
|
public class MyClass
|
|
{
|
|
[ThreadStatic] // Noncompliant
|
|
private int count = 0;
|
|
|
|
// ...
|
|
}
|
|
----
|
|
|
|
|
|
==== Compliant solution
|
|
|
|
[source,csharp]
|
|
----
|
|
public class MyClass
|
|
{
|
|
private int count = 0;
|
|
|
|
// ...
|
|
}
|
|
----
|
|
or
|
|
|
|
[source,csharp]
|
|
----
|
|
public class MyClass
|
|
{
|
|
private readonly ThreadLocal<int> count = new ThreadLocal<int>();
|
|
public int Count
|
|
{
|
|
get { return count.Value; }
|
|
set { count.Value = value; }
|
|
}
|
|
// ...
|
|
}
|
|
----
|
|
|
|
== Resources
|
|
|
|
=== Documentation
|
|
|
|
* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/api/system.threadstaticattribute[ThreadStaticAttribute Class]
|
|
* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/api/system.threading.threadlocal-1[ThreadLocal<T> Class]
|
|
|
|
ifdef::env-github,rspecator-view[]
|
|
|
|
'''
|
|
== Implementation Specification
|
|
(visible only on this page)
|
|
|
|
=== Message
|
|
|
|
Remove the "ThreadStatic" attribute from this definition.
|
|
|
|
|
|
'''
|
|
== Comments And Links
|
|
(visible only on this page)
|
|
|
|
=== on 8 Jun 2015, 08:43:39 Tamas Vajk wrote:
|
|
LGTM
|
|
|
|
=== on 15 Jun 2015, 12:39:35 Tamas Vajk wrote:
|
|
\[~ann.campbell.2] Based on [~dinesh.bolkensteyn]'s comments I've changed the description a bit. Also, with this wording it is more like a bug than a maintainability issue. So I've modified the severity as well. I didn't change the SQALE characteristic, do you see any better option?
|
|
|
|
=== on 15 Jun 2015, 15:03:19 Ann Campbell wrote:
|
|
\[~tamas.vajk] as written, 'Critical' is not currently appropriate for this rule. If we're going to increase the severity, then the description needs to show why it's 'Critical'. What mistakes will this misunderstanding have lead the developer to make?
|
|
|
|
=== on 16 Jun 2015, 09:15:33 Tamas Vajk wrote:
|
|
\[~ann.campbell.2] I've updated the description to be more bug-oriented.
|
|
|
|
=== on 16 Jun 2015, 11:17:19 Ann Campbell wrote:
|
|
\[~tamas.vajk] my 5 minutes with Google did not reveal the significance of ``++ThreadLocal++``. How is it relevant here?
|
|
|
|
=== on 17 Jun 2015, 07:16:29 Tamas Vajk wrote:
|
|
\[~ann.campbell.2] I can understand that you couldn't find a lot of info on ``++ThreadLocal++``. It is only part of .Net 4, and it is probably rarely used.
|
|
|
|
|
|
If you have a ``++ThreadStatic++`` non-``++static++`` field, that behaves as a normal non-``++static++`` field. So the attribute is useless on it. You should remove it (first compliant solution). But what if you want a non-``++static++`` field that can store different values based on the thread we are using it from. Then you can use the ``++ThreadLocal++`` class (second complaint solution).
|
|
|
|
|
|
Check out the below code:
|
|
|
|
----
|
|
var m1 = new MyClass();
|
|
var m2 = new MyClass();
|
|
m1.Count = 5;
|
|
m2.Count = 7;
|
|
|
|
Task.Factory.StartNew(() =>
|
|
{
|
|
m1.Count = 6;
|
|
m2.Count = 8;
|
|
Console.WriteLine(m1.Count);
|
|
Console.WriteLine(m2.Count);
|
|
}).Wait();
|
|
|
|
Console.WriteLine(m1.Count);
|
|
Console.WriteLine(m2.Count);
|
|
----
|
|
|
|
It writes to the console ``++6,8,5,7++``. We have two instances of ``++MyClass++``, we set the ``++Count++`` to different values (the field is not static). Then start a new thread, and set the ``++Count++`` again to different values. In the new thread and in the main thread the ``++Count++``s have different values even for the same objects.
|
|
|
|
=== on 17 Jun 2015, 12:50:37 Ann Campbell wrote:
|
|
Okay, your turn [~tamas.vajk]. :-)
|
|
|
|
=== on 17 Jun 2015, 13:13:58 Tamas Vajk wrote:
|
|
\[~ann.campbell.2] Thanks, it looks good, I'll run it through [~dinesh.bolkensteyn], and we'll see what he thinks.
|
|
|
|
endif::env-github,rspecator-view[]
|