C#•3y ago

Why is my code not thread save?

await Task.WhenAll(tasks).ContinueWith(task => Selection());

await Task.WhenAll(tasks).ContinueWith(task => Selection());

is in a loop tasks is a list of tasks which add elements to a list; Selection is a method which edits the list (cuts off the worse results that were delivered when "tasks" were executed) I thought that all tasks are finished when starting Selection() and i start it like this afterwards. But i always get the following error after a few cycles:

System.ArgumentException: 'Destination array was not long enough. Check the destination index, length, and the array's lower bounds. Arg_ParamName_Name'

System.ArgumentException: 'Destination array was not long enough. Check the destination index, length, and the array's lower bounds. Arg_ParamName_Name'

7 Replies

canton7•3y ago

I don't think you've shared enough code for us to be able to help: we can't see what tasks or Selection are doing My best guess is that you're starting a bunch of tasks on different threads which all add to the same list, but without more info that's just a guess

DeaDoOP•3y ago

The program should help solving the "traveling-salesman-problem" It creates a bunch of tracks from city to city, checks the travel distance of each track and adds the track to a list. After creating 100 tracks, it takes the 10 shortest and adds mutated variants of those to the list until there are 100 again. Class Track serves as data-object for storing the track with its distance and the track it mutated from.

        private List<Track> tracks = new List<Track>();

        public async void CreateNextGen()
        {
            List<Task> tasks = new List<Task>();
            for (int i = 0; i < genSize - nextGenSize; i++)
            {
                if (rnd.Next(100) == 50)
                {
                    tasks.Add(Task.Run(() => AddSwapped()));
                }
                else
                {
                    tasks.Add(Task.Run(() => AddMutation()));
                }
            }
            await Task.WhenAll(tasks);
            //sleep here
            Selection();
        }

        private void AddMutation()
        {
            var track = tracks[rnd.Next(nextGenSize)];
            var mutation = CreateMutation(track.CityIndexes);
            var distance = GetTrackDistance(mutation);
            tracks.Add(new Track(mutation, distance, track));
        }
        private void AddSwapped()
        {
            var track = tracks[rnd.Next(nextGenSize)];
            var swapped = SectionSwap(track.CityIndexes);
            var distance = GetTrackDistance(swapped);
            tracks.Add(new Track(swapped, distance, track));
        }

        private void Selection()
        {
            tracks = tracks.OrderBy(t => t.Distance).Take(nextGenSize).ToList();
        }

        private List<Track> tracks = new List<Track>();

        public async void CreateNextGen()
        {
            List<Task> tasks = new List<Task>();
            for (int i = 0; i < genSize - nextGenSize; i++)
            {
                if (rnd.Next(100) == 50)
                {
                    tasks.Add(Task.Run(() => AddSwapped()));
                }
                else
                {
                    tasks.Add(Task.Run(() => AddMutation()));
                }
            }
            await Task.WhenAll(tasks);
            //sleep here
            Selection();
        }

        private void AddMutation()
        {
            var track = tracks[rnd.Next(nextGenSize)];
            var mutation = CreateMutation(track.CityIndexes);
            var distance = GetTrackDistance(mutation);
            tracks.Add(new Track(mutation, distance, track));
        }
        private void AddSwapped()
        {
            var track = tracks[rnd.Next(nextGenSize)];
            var swapped = SectionSwap(track.CityIndexes);
            var distance = GetTrackDistance(swapped);
            tracks.Add(new Track(swapped, distance, track));
        }

        private void Selection()
        {
            tracks = tracks.OrderBy(t => t.Distance).Take(nextGenSize).ToList();
        }

since i have >10000 cities there are factorial of 10000 different ways to create a track/route, so i cant just check every possible route. So i went for a ML-like approach

canton7•3y ago

Yeah, both AddSwapped and AddMutation mutate tracks And tracks isn't thread-safe

DeaDoOP•3y ago

i guess there is no easy solution for this? it was not intended for multithreading in the first place and i dont have experience with that as well does it change something if i make a copy of the track before AddMutation/AddSwapped change tthat one?

canton7•3y ago

Don't access tracks from those methods, but pass them a track as a parameter, and have them return the track Then the caller can sort all of that out (since CreateNextGen runs on a single thread)

DeaDoOP•3y ago

public async void CreateNextGen()
        {
            List<Task> tasks = new List<Task>();
            for (int i = 0; i < genSize - nextGenSize; i++)
            {
                if (rnd.Next(100) == 50)
                {
                    tasks.Add(Task.Run(() => AddSwapped(tracks[rnd.Next(nextGenSize)])));
                }
                else
                {
                    tasks.Add(Task.Run(() => AddMutation(tracks[rnd.Next(nextGenSize)])));
                }
            }
            await Task.WhenAll(tasks);
            //sleep here
            Selection();
        }

        private Track AddMutation(Track track)
        {
            var mutation = CreateMutation(track.CityIndexes);
            var distance = GetTrackDistance(mutation);
            return new Track(mutation, distance, track);
        }
        private Track AddSwapped(Track track)
        {
            var swapped = SectionSwap(track.CityIndexes);
            var distance = GetTrackDistance(swapped);
            return new Track(swapped, distance, track);
        }

public async void CreateNextGen()
        {
            List<Task> tasks = new List<Task>();
            for (int i = 0; i < genSize - nextGenSize; i++)
            {
                if (rnd.Next(100) == 50)
                {
                    tasks.Add(Task.Run(() => AddSwapped(tracks[rnd.Next(nextGenSize)])));
                }
                else
                {
                    tasks.Add(Task.Run(() => AddMutation(tracks[rnd.Next(nextGenSize)])));
                }
            }
            await Task.WhenAll(tasks);
            //sleep here
            Selection();
        }

        private Track AddMutation(Track track)
        {
            var mutation = CreateMutation(track.CityIndexes);
            var distance = GetTrackDistance(mutation);
            return new Track(mutation, distance, track);
        }
        private Track AddSwapped(Track track)
        {
            var swapped = SectionSwap(track.CityIndexes);
            var distance = GetTrackDistance(swapped);
            return new Track(swapped, distance, track);
        }

how do i collect the results in CreateNextGen? Ok, i can google that myself. Thanks a lot! 👍

Angius•3y ago

Also, you really shouldn't be using .ContinueWith() It's really just an older and worse version of what await provides

Gaming

Programming

Why is my code not thread save?