Btukfyl

For an implementation of a games browser I need a collection of games that can be read and written by different threads. I want the elements of the collection to be as independent of each other as possible. I feel like changing one game should not disturb another game. Thats why I am hesitating to make use of the ReaderWriterLockSlim class when changing a game. As of now I am using a combination of a simple lock object within the game class and the ReaderWriterLockSlim class as follows:

• Consumer: Multiple threads. One for every client that requests the collection of games. They are using EnterReadLock.


• Producer (add): One thread for adding new games to the collection if required. After it finishes its work it will be called through a timer later again. It is using EnterUpgradeableReadLock once and EnterWriteLock for every required new game.


• Producer (modify): Multiple threads. Up to at least one for every game that exists in the collection. Every thread is always solely going to change one game. There may be multiple threads for one game. To access the right game in the collection they are making use of EnterReadLock. To prevent multiple threads from interfering when processing the same game, I am using an additional simple lock object on the game class right now.

As of this article Are IEnumerable Linq methods thread-safe? I know, that modifying the collection invalidates the enumerator and its behavior becomes undefined. But I am not sure, how to solve it properly.

So my Problem is: Modifying producers may change the elements of the collection while either

• adding producers may add new elements or


• consumers may iterate over the collection via linq expressions.

What do you suggest? Does a thread that wants to change a game really have to EnterWriteLock, thus preventing other games from being updated at the same time? Am I too concerned about performance issues?

Sample code:

public class Game

{

    private readonly IList<Player> players;



    public Game(int id, string name, GameMode mode)

    {

        ID = id;

        Name = name;

        Mode = mode;

        Status = GameStatus.WaitingForPlayers;

        players = new List<Player>();

    }



    public object Lock { get; private set; } = new object();

    public int ID { get; }

    public string Name { get; }

    public GameMode Mode { get; }

    public GameStatus Status { get; private set; }

    public int PlayerCount => players.Count;



    public void Join(Player player)

    {

        // Just an example...

        players.Add(player);

    }



    public void Leave(Player player)

    {

        players.Remove(player);

    }



    public void Start()

    {

        // Just an example...

        Status = GameStatus.Started;

    }



    public GameDto ToDto() => new GameDto(ID, Name, Mode, Status, PlayerCount);



    // Just an example. Depends on if it was started already and so on...

    public bool CanStart => PlayerCount > 3;

}



public class Server : IDisposable

{

    private readonly ReaderWriterLockSlim gamesLock = new ReaderWriterLockSlim();



    // This is the collection, that is giving me headaches.

    private ICollection<Game> games = new List<Game>();



    // ... Constructor and more ...



    // Will be called once in a while but not in parallel. Adds games, if available games are full...

    private void AddRequiredGames()

    {

        while (TryGetRequiredGame(GetGames(), out Game game))

        {

            gamesLock.EnterWriteLock();

            games.Add(game);

            gamesLock.ExitWriteLock();

        }

    }



    // Will be called once the last player left a game. There may be multiple threads at once for different games.

    private void RemoveIfObsolete(Game game)

    {

        gamesLock.EnterUpgradeableReadLock();

        lock (game.Lock)

        {

#warning Here I am eventually iterating over the collection, while its elements could be modified.

            if (game.IsEmpty && games.Count(item => item.Mode == game.Mode

              && item.Status == GameStatus.WaitingForPlayers) > 3)

            {

                // ToDo - Mark game as obsolete to prevent incoming join attempts.

                gamesLock.EnterWriteLock();

                games.Remove(game);

                gamesLock.ExitWriteLock();

            }

        }

        gamesLock.ExitUpgradeableReadLock();

    }



    private ICollection<GameDto> GetGames()

    {

        gamesLock.EnterReadLock();

#warning Here I am iterating over the collection, while its elements could be modified.

        IList<GameDto> result = games.Select(item => item.ToDto()).ToList();

        gamesLock.ExitReadLock();

        return result;

    }



    // Will be called asynchronously be every client that requests to join a game.

    private void HandlePlayerJoin(Player player, Game game)

    {

        lock (game.Lock)

        {

#warning Here I am modifying an element of the collection without locking the collection.

            game.Join(player);

        }

        // Just an example. Wouldn't try to start the game, if the player could not join etc...

        TryStartGame(game);

    }



    private void TryStartGame(Game game)

    {

        lock (game.Lock)

        {

            if (game.CanStart)

#warning Here I am modifying an element of the collection without locking the collection.

                game.Start();

        }

    }



    // Will be called asynchronously be every client that requests the list of games.

    private void HandleGetGames(Client client)

    {

        client.Send(new GetGamesResponse(GetGames()));

    }

}