Ok, finally after hours of research and coding, I think I have a thread safe code. Maybe?

Here, Am I doing anything stupid here?

namespace Engine.AI.Pathfinding
{
    public static class AstarManager
    {
        static Thread UpdateThread = new Thread(new ThreadStart(Update));

        static ConcurrentQueue<Astar> AstarQueue = new ConcurrentQueue<Astar>();
        public static Queue<Astar> AstarResults = new Queue<Astar>();

        public static void Initialize()
        {
            UpdateThread.IsBackground = true;
            UpdateThread.Priority = ThreadPriority.Highest;

            if(!UpdateThread.IsAlive)
                UpdateThread.Start();
        }

        public static void AddNewPath(Node StartingNode, Node TargetNode, Map map, bool DisableDiagonalPathfinding)
        {
            AstarQueue.Enqueue(new Astar(StartingNode, TargetNode, map, DisableDiagonalPathfinding));
        }

        static void Update()
        {
            while (true)
            {
                for (int i = 0; i < AstarQueue.Count; i++)
                {
                    Astar TempAstar;

                    AstarQueue.ToList()[i].FindPath();
                    AstarQueue.TryDequeue(out TempAstar);
                    AstarResults.Enqueue(TempAstar);
                    
                    Thread.Sleep(5);
                }
            }
        }
    }
}

I'm not a C# programmer, and even to me that code is clearly not thread safe.

Mistakes I've noticed in the Update() function:

- You're using a for loop to go through the queue. Since the count can change at any time due to other threads this is just wrong. Don't use .Count at all.

- Calling ToList() is going to be bad, for the same reason. The only way you should be examining the list contents is via TryDequeue().

- You ignore the return value from TryDequeue(). This is bad because if it's false you've not got a valid object to work with - the list is empty.

- The code only sleeps when there's an item in the list. If it's empty the thread eats 100% of one CPU core for no reason. You want the exact opposite of that.

- Looping calling Thread.Sleep() is a bad way to wait for work anyway. You want to use some sort of blocking wait function, which may require a different container.

There's also no sign of any test code. Even if you think you've got threaded code right, it's generally a good idea to do some stress testing, just in case you missed something.

My recommendation would be to avoid using threads until you have more experience. They are difficult to use correctly, even for experienced programmers, and modern CPUs run code fast enough on a single core that you can easily get away without them.

I'm not a C# programmer, and even to me that code is clearly not thread safe.

Mistakes I've noticed in the Update() function:
- You're using a for loop to go through the queue. Since the count can change at any time due to other threads this is just wrong. Don't use .Count at all.
- Calling ToList() is going to be bad, for the same reason. The only way you should be examining the list contents is via TryDequeue().
- You ignore the return value from TryDequeue(). This is bad because if it's false you've not got a valid object to work with - the list is empty.
- The code only sleeps when there's an item in the list. If it's empty the thread eats 100% of one CPU core for no reason. You want the exact opposite of that.
- Looping calling Thread.Sleep() is a bad way to wait for work anyway. You want to use some sort of blocking wait function, which may require a different container.

There's also no sign of any test code. Even if you think you've got threaded code right, it's generally a good idea to do some stress testing, just in case you missed something.

My recommendation would be to avoid using threads until you have more experience. They are difficult to use correctly, even for experienced programmers, and modern CPUs run code fast enough on a single core that you can easily get away without them.

Thank you for the tips, that was really helpfully. Here is an update version of my code. I think I covered all your points from my understanding. not sure though.

- Looping calling Thread.Sleep() is a bad way to wait for work anyway. You want to use some sort of blocking wait function, which may require a different container.

Any ideas on how to do that?

I do apologize for my lack of knowledge. I feel like I'm just spamming code for other people to look at. again I apologize.

static void Update()
{
    while (true)
    {
        if (AstarQueue.Count > 0)
        {
            Astar TempAstar;

            AstarQueue.TryDequeue(out TempAstar);
            TempAstar.FindPath();
            AstarResults.Enqueue(TempAstar);
        }

        Thread.Sleep(5);
    }
}

Your results queue is not a ConcurrentQueue, and yet you are expecting different threads to operate on it (one thread writes to it, another thread reads from it).

Thread.Sleep() is totally the wrong way to avoid throttling. You can end up in a situation where the thread is finishing a job right as another comes in, but it sleeps for 5 seconds before getting on to it. You can use something like an Event (there's an auto-reset and manual-reset variant that have very similar but different use cases) to replace both the Thread.Sleep and replace the need to ever query AstarQueue.Count (you should never ever need to do that; just use TryDeque and check the return value!). You can also use a Semaphore instead of an event.

Your thread loop would look something like:

run:
  wait on event
  reset event (if manual reset)
  while try deque job:
    run job

And your submission thread would simply be:

add job:
  enque job
  signal event

Some concurrent queues are fixed size and you might need to use a TryEnque and handle the failure case. I don't know .NET's library that well and don't know how their concurrent queue is implemented.

Thank you, That really helped

Here is the last version of my code. I'm really hoping I fixed everything :)

namespace Engine.AI.Pathfinding
{
    public static class AstarManager
    {
        static Thread UpdateThread = new Thread(new ThreadStart(Update));
        static AutoResetEvent autoResetEvent = new AutoResetEvent(false);

        static ConcurrentQueue<Astar> AstarQueue = new ConcurrentQueue<Astar>();
        public static ConcurrentQueue<Astar> AstarResults = new ConcurrentQueue<Astar>();

        public static void Initialize()
        {
            UpdateThread.IsBackground = true;
            UpdateThread.Priority = ThreadPriority.Highest;

            if(!UpdateThread.IsAlive)
                UpdateThread.Start();
        }

        public static void AddNewPath(Node StartingNode, Node TargetNode, Map map, bool DisableDiagonalPathfinding)
        {
            AstarQueue.Enqueue(new Astar(StartingNode, TargetNode, map, DisableDiagonalPathfinding));
            autoResetEvent.Set();
        }

        static void Update()
        {
            while (true)
            {
                autoResetEvent.WaitOne();

                Astar TempAstar;

                AstarQueue.TryDequeue(out TempAstar);
                TempAstar.FindPath();
                AstarResults.Enqueue(TempAstar);
            }
        }
    }
}