C# Thread safe code

Question

Is this code thread safe?

DoStuff is called in a new thread using Task.

Task.Factory.StartNew(() => DoStuff());

private void DoStuff()
{
    List<SalesRecord> salesRecords = new List<SalesRecord>();
    SalesRecord salesRecord1 = new SalesRecord {Amount = 10.0, Sales = 1};
    SalesRecord salesRecord2 = new SalesRecord {Amount = 15.0, Sales = 1};
    SalesRecord salesRecord3 = new SalesRecord {Amount = 1.0, Sales = 2};
    salesRecords.Add(salesRecord1);
    salesRecords.Add(salesRecord2);
    salesRecords.Add(salesRecord3);
    SalesRecord result = Util.SumSales(salesRecords);
}

A struct just to store data:

public struct SalesRecord
{
    public uint Sales;
    public double Amount;
}

stuff

public static class Util
{
    public static SalesRecord SumSales(List<SalesRecord> records)
    {
        SalesRecord result = new SalesRecord();

        result.Amount = records.FindAll(record => (record.Sales == 1)).Sum(record => record.Amount);
        result.Sales = 1;
        return result;
    }
}

I'm not actually seeing any multithreading happening, so I would guess from the code you provided that yes, it is thread-safe. Unless you're accessing things between different threads, this code shouldn't be a problem. — Josh L., Apr 24 '15 at 14:32
I see nowhere you actually call DoStuff - would be nice to see where it's used to make an educated decision. — Der Kommissar, Apr 24 '15 at 14:33
@user1766169 it doesn't matter as long as everything is done inside the same thread. This is single-threaded code. — Panagiotis Kanavos, Apr 24 '15 at 14:33
I don't see `static` properties and you operate with only local variable, there is no possibility to have multithreading problem outside `DoStuff`. This makes `DoStuff` thread-safe. — Sinatr, Apr 24 '15 at 14:35
@PeterSchneider All modifications and creations are done inside a single function so this *is* thread-safe, even by accident. If the List was returned from `DoStuff` and multiple threads modified the list subsequently, no it wouldn't be — Panagiotis Kanavos, Apr 24 '15 at 14:37
@PeterSchneider I'm not sure I see why it's not. Thread-local data manipulation means there's not any reason any thread affects any other threads. — Josh L., Apr 24 '15 at 14:37
@JoshL. @Pangiotis You are right; I didn't see that DoStuff uses only local variables (which is somewhat unusual -- there is no observable effect of calling it unless `SalesRecord`'s ctor has a side effect, e.g. creates a DB entry (which is never commited though). But my comment was wrong, no doubt. — Peter - Reinstate Monica, Apr 24 '15 at 14:42

score 9 · Accepted Answer · answered Apr 24 '15 at 14:36

9

Is this code thread safe?

Yes, it is. This code doesn't use any shared state. By definition code which doesn't uses any shared state is thread-safe.

You can call DoStuff concurrently in any number of threads without any problem. That said, DoStuff method isn't very useful though.

answered Apr 24 '15 at 14:36

Sriram Sakthivel

72,067
7
111
189

Would 'DoStuff' still be thread safe if it returned 'salesRecords'? @SriramSakthivel – user1766169 Apr 25 '15 at 14:14
@user1766169 Yes, it will be. But that isn't still useful. Thread safety is a concern only when you access same instance/static member from two threads. But your code doesn't and thus it is thread-safe. – Sriram Sakthivel Apr 25 '15 at 18:28

score 4 · Answer 2 · edited May 23 '17 at 11:59

4

Your code looks fine. Even if DoStuff is being launched via Task, I don't see anywhere in your code where you're dealing with class variables, they're all local variables.

Reference: C# : What if a static method is called from multiple threads?

edited May 23 '17 at 11:59

Community

1
1

answered Apr 24 '15 at 14:36

Shar1er80

9,001
2
20
29

4

@PeterSchneider Only if it's a class/shared state variable, which it is not. It's a local variable to the thread. If the OP were to launch another DoStuff(), via task again, there would be another local List to that class. – Shar1er80 Apr 24 '15 at 14:40
4

@PeterSchneider wrong wrong. It is called from a single thread, on an instance that is local to the method it is defined in. No other thread can access this instance – Panagiotis Kanavos Apr 24 '15 at 14:40
As I said in another comment, I didn't read the method carefully enough. (But as an excuse I concur with @SiriamSa that `DoStuff()` doesn't do anything which is misleading to the reader. In fact, probably no code needs to be generated for it so that thread safety is easy to show.) – Peter - Reinstate Monica Apr 24 '15 at 14:46

score 0 · Answer 3 · answered Apr 24 '15 at 14:53

Posted for those trying to figure out why it's thread-safe even with a static method

In this particular case, access to Util.SumSales(salesRecords) is thread safe as it is a thread-local method (the thread that calls this method provides it's own copy of the data to the method and data access in that thread is exclusive to the thread that called it).

When you call the following code:

Task.Factory.StartNew(() => DoStuff());

You assign a new thread to handle DoStuff(). Everything that happens in DoStuff (unless you introduce variables that could be manipulated outside of it) is exclusive to that new thread.

I assume you'll do more with what you get from DoStuff(). In that case, you should store your results in Concurrent Collections.

C# Thread safe code

3 Answers3