From cd94063668eee1952affd72ecb7ffda99dc0965e Mon Sep 17 00:00:00 2001 From: SkyratBot <59378654+SkyratBot@users.noreply.github.com> Date: Wed, 9 Jun 2021 22:55:34 +0200 Subject: [PATCH] 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 --- code/controllers/subsystem/mapping.dm | 2 ++ code/modules/mapping/space_management/zlevel_manager.dm | 3 +++ 2 files changed, 5 insertions(+) diff --git a/code/controllers/subsystem/mapping.dm b/code/controllers/subsystem/mapping.dm index 76910e342dc..fe8d6e19bee 100644 --- a/code/controllers/subsystem/mapping.dm +++ b/code/controllers/subsystem/mapping.dm @@ -49,6 +49,8 @@ SUBSYSTEM_DEF(mapping) var/datum/space_level/transit var/datum/space_level/empty_space var/num_of_res_levels = 1 + /// True when in the process of adding a new Z-level, global locking + var/adding_new_zlevel = FALSE /datum/controller/subsystem/mapping/New() ..() diff --git a/code/modules/mapping/space_management/zlevel_manager.dm b/code/modules/mapping/space_management/zlevel_manager.dm index 6129c5fd2b6..35235579a44 100644 --- a/code/modules/mapping/space_management/zlevel_manager.dm +++ b/code/modules/mapping/space_management/zlevel_manager.dm @@ -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)