Locking on List Produces Deadlock

Question

I'm working on a C# WPF game that uses multi-threading, and I encountered a bug, where the game would just suddenly stop, when I'm adding a new entity to the playground.

I have 2 Tasks running each tick that makes the game work. The first is the one doing all the calculations and running the simulation. The second is used to update the renderer in every tick.

This code is run when the game starts:

Task.Run(() =>
{
    while (state != GameLevelState.Ended)
    {
        Thread.Sleep(1000 / 60);
        if (state == GameLevelState.Paused)
            continue;
        Task.Run(RunSimulation);
        Task.Run(UpdateRenderer);
    }
});

The problem start with the List of entities in which they're stored in, as I need to access it with both of my Task, and also I need to create a new entity when a button is clicked. The deadlock appears when the UI thread wants to check, if there is already another entity of the same type in the entity List.

Here's the code that get the entities:

public List<GameEntity> GetGameEntities(Func<GameEntity, bool> predicate = null)
{
    if (predicate == null)
        predicate = (GameEntity e) => true;

    List<GameEntity> selected = new List<GameEntity>();

    // this is where the execution stops
    lock (Entities)
    {
        foreach (var entityList in Entities)
            selected.AddRange(entityList.Value.Where(predicate));
        return selected;
    }
}

So when I try to get the entities of the same type when adding an entity, execution stops on the lock. My theory is that the UI thread can't find an opening, because the 2 other threads/tasks are constantly locking the Entities List. (they only lock the list when they're iterating through it)

(I should also point out that the 2 tasks/threads run perfectly essentially forever if left alone. It only stops when trying to create/add an entity from the UI thread)

Here's the relevant code that the Tasks perform:

private void RunSimulation()

/* ... */
lock (Entities)
{
    foreach (var entityList in Entities.ToArray())
        foreach (GameEntity entity in entityList.Value.ToArray())
            if (!entity.Tick())
            {
                entityList.Value.Remove(entity);
                Application.Current.Dispatcher.Invoke(() => renderer.RemoveEntity(entity));
            }
}
/* ... */

private void UpdateRenderer()

/* ... */
lock (Entities)
{
    foreach (var entityList in Entities)
        foreach (GameEntity entity in entityList.Value.ToArray())
            Application.Current.Dispatcher.Invoke(() => renderer.DrawEntity(entity));
}
/* ... */

My question is: How can I fix this issue?

Should I make a copy of the List and iterate through it, so the List is only locked for the time of copying? or Should I delegate the entity-adding-process to the thread doing all the calculations?

Any help is greatly appreciated!

Answer 1

Not sure if this will fix your problem, but it will certainly help in keeping your spawned tasks under control. A new loop will not be started before the completion of the currently running tasks. It uses await and Task.Delay instead of Thread.Sleep.

Task.Run(async () =>
{
    while (state != GameLevelState.Ended)
    {
        var delayTask = Task.Delay(1000 / 60);
        if (state != GameLevelState.Paused)
        {
            var task1 = Task.Run(RunSimulation);
            var task2 = Task.Run(UpdateRenderer);
            await Task.WhenAll(task1, task2);
        }
        await delayTask;
    }
});