Fix race condition in SSmapping z-level creation (#59560) (#6207)

As described in issue #56733, there is a possibility of a race condition in SSmapping.add_new_zlevel - either during Z level incrementing, or the proc itself, as it only expands the amount of registered z-levels after being finished.

While the first hopefully shouldn't happen, it can still manifest because of the CHECK_TICK within. A practical result is that two concurrent calls to this proc will both create a new Z-level, but actually report having made the same first one.

This is problematic for map_templates that will then load blindly to the reported Z-level when using load_new_z, overwriting each other. We sometimes encounter that end result on CM13 codebase because of a map load that can be triggered by an user topic - if two people click at the same time, it's not unlikely for the first call to sleep in CHECK_TICK and let the second run, causing double-loading.

Co-authored-by: fira <loyauflorian@gmail.com>
This commit is contained in:
SkyratBot
2021-06-09 22:55:34 +02:00
committed by GitHub
parent 78992ac852
commit cd94063668
2 changed files with 5 additions and 0 deletions
@@ -17,6 +17,8 @@
z_list += S
/datum/controller/subsystem/mapping/proc/add_new_zlevel(name, traits = list(), z_type = /datum/space_level)
UNTIL(!adding_new_zlevel)
adding_new_zlevel = TRUE
SEND_GLOBAL_SIGNAL(COMSIG_GLOB_NEW_Z, args)
var/new_z = z_list.len + 1
if (world.maxz < new_z)
@@ -25,6 +27,7 @@
// TODO: sleep here if the Z level needs to be cleared
var/datum/space_level/S = new z_type(new_z, name, traits)
z_list += S
adding_new_zlevel = FALSE
return S
/datum/controller/subsystem/mapping/proc/get_level(z)