Having problem making my code thread-safe

Question

CreateNewRound() method is accessed by multiple threads at runtime. Let's say the _game.CurrentRound = 99 and two threads access the method at the same time and they both initialize the currentRoundId as 100 and both threads add two entities with the same roundId. But that is wrong and I don't want that to happen since rounds should be unique and different. How can I fix this so that thread one adds an entity with round 100 and the other with round 101.

 public void CreateNewRound() { var game = _cache.GetGameById(_session.gameId); var currentRoundId = game.CurrentRound + 1; var response = SomeAPI.SomeCall(); if (response.responseCode == (int)responseCodes.Success) { _dbContext.GameState.Add(new GameState() { RoundId = CurrentRoundId }); _dbContext.SaveChanges(); } }

learn.microsoft.com/en-gb/dotnet/csharp/language-reference/… — Franz Gleichmann
– Franz Gleichmann, Commented Aug 21, 2020 at 12:59
@Artavazd Yes but you have only one resource, in that case you must use monitor (or binary semaphore); aniway you must prevent other thread to do something while other threads are already using same resource, so you don't care if this is slower it is important to run corectly. P.S. monitor in C# is implemented as lock. — Lazar Đorđević
– Lazar Đorđević, Commented Aug 21, 2020 at 13:07
If multiple threads can call this in the same round, you clearly cannot use the round number as a unique ID. You will have to introduce another (or an additional) ID to disambiguate. — Matthew Watson
– Matthew Watson, Commented Aug 21, 2020 at 13:07
@Artavazd "doesn't applying a lock make my code slower?" - very likely. But is that a problem? - I'd suggest implementing the easiest solution that fixes the problem ( i.e. fewest changes ) and benchmark it. If you don't like the benchmarks' results: find the next more complex solution that shifts focus from "fewest changes" to "speed". Then measure again ... — Fildor
– Fildor, Commented Aug 21, 2020 at 13:07
yes. a lock makes your code slower. the exacly same way a traffic light makes cars go slower - so they don't crash into each other. basically it's your choice - either live with the fact that your code runs with errors, or live with the fact that your code waits for other pieces of code to finish. you can't have both. (or find a solution that doesn't rely on threads accessing the same resources, thus eliminating the need for synchronisation) — Franz Gleichmann
– Franz Gleichmann, Commented Aug 21, 2020 at 13:15

JonasH · Accepted Answer · 2020-08-21 15:07:07Z

If (and only if) all called methods are pure, i.e. where the result only depends on the input parameters, you can simply used interlocked.Increment to ensure the currentRound will be unique for each call:

 private int currentRound = 0; public void CreateNewRound() { var thisRound = Interlocked.Increment(ref currentRound); var gamestate = CreateGameState(thisRound) // process game state }

In most games, the next round will depend on the game-state of the previous round. And in that case you must run each round sequentially. The typical solution would be to use a lock for this:

 private int currentRound = 0; private object myLock = new object(); private MyGameState gameState; public void CreateNewRound() { lock (myLock) { currentRound++; gameState = ComputeNextGameState(gameState, currentRound); // process game state } }

There are alternatives, like assigning a specific thread to do all game-state updates, and make CreateNewRound merely ask the update thread to do an update.

Will there be any problem if that "process game state" is a call to an API that might take some time? that will affect the performance of my code. Am I correct?
@artavazd that would depend on what the API call does. If the API call is pure it will not matter if multiple threads call it concurrently, so it would be safe to run it on a separate thread if needed.

Collectives™ on Stack Overflow

Having problem making my code thread-safe

1 Answer 1

2 Comments

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

2 Comments

Related