CivMC / Civ

Monorepo for development of and running a Civ Server.
MIT License
4 stars 11 forks source link

Potential CivModCore race condition fixes #388

Closed xFier closed 3 months ago

xFier commented 3 months ago

Addresses two possible race condition scenarios:

A first race condition with unloadChunkCoord in the following scenario:

  1. Thread A checks coord.isUnloaded() inside the synchronized block towards the end of the method and finds that it is unloaded.
  2. Thread B loads the chunk before Thread A removes the chunk from the metas map.
  3. Thread A removes the chunk from the metas map, even through it's now loaded.

Attempted to be fixed by preventing the chunk from being loaded while we're checking it and potentially removing it from metas.

The second race condition is to do with loadChunk, where:

  1. Thread A calls loadChunk
  2. Thread B unloads the chunk before Thread A update's the chunk's status
  3. Thread A updates the chunk's status to loaded, even though it's now unloaded.

Attempted to be fixed by checking inside the loadChunk method to ensure the chunk has not been unloaded after it was loaded. We're holding a lock on the metas map to prevent other threads from unloading the chunk during the check.

okx-code commented 3 months ago

I don't think either of your race conditions will happen because getChunkCoordalso has a synchronised (metas) before interacting with the metas map.