1

Do I have to use concurrent types in MethodTwo.

if I don't, can my list (anotherList) is completely saved with true values?

public void MethodOne(List<int> idList)
{
    foreach (int id in IdList)
    {
         var Item = GetById(id);
         System.Threading.Tasks.Task.Run(() => MethodTwo(item.PropOne));
    }
}

public void MethodTwo(int prop)
{
    List<string> anotherList = new List<string>();
    anotherList.Add("AAA");
        
    if (prop>20)
        anotherList.Add("BBB");
    else
        anotherList.Add("CCC");

    //some other operations

    SaveList(anotherList);
}

There is a bug but I can not catch it. I am suspicious about this method. I tried load test on this method and I could not get any information.

Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
eray
  • 13
  • 5
  • 3
    I only see `anotherList` being accessed by a single thread in the code you have shown. – ProgrammingLlama Nov 11 '22 at 10:09
  • 2
    That's not your actual code. I can tell, because it contains an error which will stop it from compiling. Please share your actual code -- we don't know what changes you've made, which may have removed your problem entirely – canton7 Nov 11 '22 at 10:12
  • 1
    The code seems to be thread-safe at first glance. But what does `SaveList` do? Maybe there is the problem. – SomeBody Nov 11 '22 at 10:21
  • As a side note the `Task.Run(() => MethodTwo(item.PropOne));` call is what is known as fire-and-forget. This practice is [disapproved](https://stackoverflow.com/questions/61316504/proper-way-to-start-and-async-fire-and-forget-call/61320933#61320933) by the experts. Please keep track of your tasks, and don't let them become fire-and-forget! – Theodor Zoulias Nov 11 '22 at 15:38
  • 1
    Thank you @TheodorZoulias. I read the reference you mentioned. I do not want to forget but I have timeout restriction. – eray Nov 11 '22 at 17:58
  • I suspect `SaveList` because a) you have a bug, and b) I don't see thread safety issues in the code shown. That tells me it's in the code not shown. – Scott Hannen Nov 11 '22 at 18:27

1 Answers1

1

No

Thread safety is a concern whenever multiple threads may write to the same data, or reading and writing, at the same time.

If your list is only accessed by a single thread it is perfectly safe to use a regular list.

This is often used when writing asynchronous code, a typical pattern might look something like:

int[] data = new int[]{1, 2, 3};
var result = await Task.Run(MyMethod);
// Use result
...

int[] MyMethod(int[] data ){
    // Do some processing
    return new int[]{4, 5, 6};
}

While the data-array is shared, it is perfectly safe as long as it is not modified.

If we change the example to do concurrent reading and writing we would be in trouble:

int[] data = new int[]{1, 2, 3};
var task = Task.Run(MyMethod);
data[0] = 42; // Writing shared data concurrently with reading. Unsafe!
var result = await task;
// Use result
...

int[] MyMethod(int[] data ){    
    var d = data[0]; // Reading shared data concurrently with writing. Unsafe!
    // Do some processing
    return new int[]{d, 5, 6};
}

Since we now have both reading and writing that may be done at the same time, the code is unsafe. This could be fixed by a lock, but it is typically a better idea to try to avoid shared mutable state as much as possible in the first place.

JonasH
  • 28,608
  • 2
  • 10
  • 23