Method giving back wrong result during Task

Question

I have a loop creating three tasks:

List<Task> tasks = new List<Task>();
foreach (DSDevice device in validdevices)
{
    var task = Task.Run(() =>
    {
         var conf = PrepareModasConfig(device, alternativconfig));
         //CHECK-Point1
         string config = ModasDicToConfig(conf);
         //CHECK-Point2
         if (config != null)
         {
             //Do Stuff
         }
         else
         {
             //Do other Stuff
         }
   });
  tasks.Add(task);
 }
 Task.WaitAll(tasks.ToArray());

it calls this method, where some data of a dictionary of a default-config gets overwritten:

private Dictionary<string, Dictionary<string, string>> PrepareModasConfig(DSDevice device, string alternativeconfig)
{
    try
    {
        Dictionary<string, Dictionary<string, string>> config = new Dictionary<string, Dictionary<string, string>>(Project.project.ModasConfig.Config);
        if (config.ContainsKey("[Main]"))
        {
            if (config["[Main]"].ContainsKey("DevName"))
            {
                config["[Main]"]["DevName"] = device.ID;
            }
        }
        return config;
    }
    catch
    {
        return null;
    }
}

and after that, it gets converted into a string with this method:

private string ModasDicToConfig(Dictionary<string, Dictionary<string, string>> dic)
{
    string s = string.Empty;
    try
    {
        foreach (string key in dic.Keys)
        {
            s = s + key + "\n";
            foreach (string k in dic[key].Keys)
            {
                s = s + k + "=" + dic[key][k] + "\n";
            }
            s = s + "\n";
        }
        return s;
    }
    catch
    {
        return null;
    }
}

But every Tasks gets the exact same string back.

On //CHECK-Point1 I check the Dic for the changed value: Correct Value for each Task

On //CHECK-Point2 I check the String: Same String on all 3 Tasks (Should be of course different)

Default-Dictionary looks like this: (shortened)

{
  {"[Main]",
     {"DevName", "Default"},
     ...
  },
  ...
}

The resulting string look like that:

[Main]
DevName=003     <--This should be different (from Device.ID)
...

[...]

EDIT: I moved the methods to execute outside the Task. Now I get the correct Results. So I guess it has something to do with the Task?

List<Task> tasks = new List<Task>();
foreach (DSDevice device in validdevices)
{
    var conf = PrepareModasConfig(device, alternativconfig));
    //CHECK-Point1
    string config = ModasDicToConfig(conf);
    //CHECK-Point2
    var task = Task.Run(() =>
    {
         
         if (config != null)
         {
             //Do Stuff
         }
         else
         {
             //Do other Stuff
         }
   });
  tasks.Add(task);
 }
 Task.WaitAll(tasks.ToArray());

The code inside `Task.Run` doesn't return anything. The other methods can be simplified *drastically*. Right now it's hard to understand what this code does. The fact there's a `catch{}` inside `ModasDicToConfig` is a very strong indication that the code doesn't work in the first place, since the method is trying to iterate over the dictionary items, in a rather complicated way. — Panagiotis Kanavos, Nov 23 '22 at 14:01
`Same String on all 3 Tasks (Should be of course different)` what does that mean? What do you get? Please post enough code to reproduce the problem. Right now one can only guess there are error that are hidden by `catch [return null;}`. — Panagiotis Kanavos, Nov 23 '22 at 14:04
For example, what if one of the nested dictionaries is missing? In that case `dic[key].Keys` would throw a null reference exception and the iteration would stop. If the first 2 of 10 dictionaries are the same for all devices and the second is `null`, you'd end up with the same string simply because the iteration would fail for all devices — Panagiotis Kanavos, Nov 23 '22 at 14:06
I updated the Post to make it clear, what the Default-Config is, and what the Result is. The Mothode works fine. It returns a valid string, not null! But the Value at "DevName" is the same on every Tasks, even when the Dictionaries are different right before ModasDicToConfig(conf) — Toby_Stoe, Nov 23 '22 at 14:12
You're still describing the code and data instead of posting them. Tasks aren't broken. If they were every single .NET developer would have noticed 10 years ago. The code posted here has problems that can easily result in bad results though - are you *sure* your code isn't failing? Those `catch{ return null;}` will hide all errors. — Panagiotis Kanavos, Nov 23 '22 at 14:20
This is the exact data. Just shortend (don't make sense to post a 3000line-file). The Brackets are already there in the sting ("[Main]") if you look at the default-Dic. — Toby_Stoe, Nov 23 '22 at 14:24
It's not. `Default-Dictionary looks like this: (shortend)` what follows is formatted text, not a dictionary. I repeat. Post something others can copy and execute. You're asking people to guess what's wrong with data and code that's missing. — Panagiotis Kanavos, Nov 23 '22 at 14:26
And the Methode does not run into the catch! 1) i have a Log-Function in the catch{}, so i would notice 2) on the main-code it is checking if the result is null (its not) 3) The string i check on -"Chek-Point2" is 3000lines and NOT NULL — Toby_Stoe, Nov 23 '22 at 14:27
What happens if there are trailing spaces in the template dictionary? Or instead of `DevName` there's `Devname`? Or the section name is `Main` instead of `[Main]`? Why would a config section include square brackets? There a *lot* of reasons this code can result in identical strings. If all it changes is the DeviceID, and that is still the default, the explanation is that the code fails to find those keys. — Panagiotis Kanavos, Nov 23 '22 at 14:32
`So I guess it has something to do with the Task?` no it's the code. Which is still missing, still can't be executed to reproduce the problem because whatever is in `Project.project.ModasConfig.Config` is missing — Panagiotis Kanavos, Nov 23 '22 at 14:36
You can clearly see, that it is checking if the Key (Main] and DevName exists, before it is overwriting them. So no, this does not Result into a catch. This Methode works, and this Post is about a different Problem i see — Toby_Stoe, Nov 23 '22 at 14:36

score 0 · Accepted Answer · answered Nov 23 '22 at 15:08

The problem isn't caused by tasks. The lambda passed to Task.Run captures the loop variable device so when the tasks are executed, all will use the contents of that variable. The same problem would occur even without tasks as this SO question shows. The following code would print 10 times:

List<Action> actions = new List<Action>();

for (int i = 0; i < 10; ++i )
    actions.Add(()=>Console.WriteLine(i));

foreach (Action a in actions)
    a();
------
10
10
10
10
10
10
10
10
10
10

If the question's code used an Action without Task.Run it would still result in bad results.

One way to fix this is to copy the loop variable into a local variable and use only that in the lambda :

for (int i = 0; i < 10; ++i )
{
    var ii=i;
    actions.Add(()=>Console.WriteLine(ii));
}

The question's code can be fixed by copying the device loop variable into the loop:

foreach (DSDevice dev in validdevices)
{
    var device=dev;
    var task = Task.Run(() =>
    {
         var conf = PrepareModasConfig(device, alternativconfig));

Another way is to use Parallel.ForEach to process all items in parallel, using all available cores, without creating tasks explicitly:

Parallel.ForEach(validdevices,device=>{
    var conf = PrepareModasConfig(device, alternativconfig));
    string config = ModasDicToConfig(conf);
    ...
});

Parallel.ForEach allows limiting the number of worker tasks through the MaxDegreeOfParallelism option. It's a blocking call because it uses the current thread to process data along with any worker tasks.

Method giving back wrong result during Task

1 Answers1