C
C#•3y ago
DeaDo

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
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
DeaDo
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
canton7•3y ago
Yeah, both AddSwapped and AddMutation mutate tracks And tracks isn't thread-safe
DeaDo
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
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)
DeaDo
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
Angius•3y ago
Also, you really shouldn't be using .ContinueWith() It's really just an older and worse version of what await provides

Did you find this page helpful?