From eba6cc5306ae11db0878d6427c612fa2a2b16091 Mon Sep 17 00:00:00 2001 From: Kyle Spier-Swenson Date: Fri, 15 Jan 2021 17:43:12 -0800 Subject: [PATCH] Stack overflow detection for the Master Controller. (#56008) * Stack overflow detection for the Master Controller. Using a weakref, we can detect if the mc's stack was ended by byond due to a stack overflow, and restart it without waiting the entire defcon countdown in the failsafe controller. I built a system around this concept under /datum/stack_end_detector and deployed it to the MC's main loop with checks in the failsafe controller. --- code/controllers/failsafe.dm | 16 ++++++++- code/controllers/master.dm | 34 ++++++++++++------- .../helper_datums/stack_end_detector.dm | 32 +++++++++++++++++ tgstation.dme | 1 + 4 files changed, 69 insertions(+), 14 deletions(-) create mode 100644 code/datums/helper_datums/stack_end_detector.dm diff --git a/code/controllers/failsafe.dm b/code/controllers/failsafe.dm index a15056e4427..0a6b4baebac 100644 --- a/code/controllers/failsafe.dm +++ b/code/controllers/failsafe.dm @@ -50,19 +50,32 @@ GLOBAL_REAL(Failsafe, /datum/controller/failsafe) // Only poke it if overrides are not in effect. if(processing_interval > 0) if(Master.processing && Master.iteration) + if (defcon > 1 && (!Master.stack_end_detector || !Master.stack_end_detector.check())) + + to_chat(GLOB.admins, "ERROR: The Master Controller code stack has exited unexpectedly, Restarting...") + defcon = 0 + var/rtn = Recreate_MC() + if(rtn > 0) + master_iteration = 0 + to_chat(GLOB.admins, "MC restarted successfully") + else if(rtn < 0) + log_game("FailSafe: Could not restart MC, runtime encountered. Entering defcon 0") + to_chat(GLOB.admins, "ERROR: DEFCON [defcon_pretty()]. Could not restart MC, runtime encountered. I will silently keep retrying.") // Check if processing is done yet. if(Master.iteration == master_iteration) switch(defcon) if(4,5) --defcon + if(3) message_admins("Notice: DEFCON [defcon_pretty()]. The Master Controller has not fired in the last [(5-defcon) * processing_interval] ticks.") --defcon + if(2) to_chat(GLOB.admins, "Warning: DEFCON [defcon_pretty()]. The Master Controller has not fired in the last [(5-defcon) * processing_interval] ticks. Automatic restart in [processing_interval] ticks.") --defcon + if(1) - to_chat(GLOB.admins, "Warning: DEFCON [defcon_pretty()]. The Master Controller has still not fired within the last [(5-defcon) * processing_interval] ticks. Killing and restarting...") --defcon var/rtn = Recreate_MC() @@ -75,6 +88,7 @@ GLOBAL_REAL(Failsafe, /datum/controller/failsafe) to_chat(GLOB.admins, "ERROR: DEFCON [defcon_pretty()]. Could not restart MC, runtime encountered. I will silently keep retrying.") //if the return number was 0, it just means the mc was restarted too recently, and it just needs some time before we try again //no need to handle that specially when defcon 0 can handle it + if(0) //DEFCON 0! (mc failed to restart) var/rtn = Recreate_MC() if(rtn > 0) diff --git a/code/controllers/master.dm b/code/controllers/master.dm index 59ac68960cb..de016fd8486 100644 --- a/code/controllers/master.dm +++ b/code/controllers/master.dm @@ -18,15 +18,17 @@ GLOBAL_REAL(Master, /datum/controller/master) = new /datum/controller/master name = "Master" - // Are we processing (higher values increase the processing delay by n ticks) + /// Are we processing (higher values increase the processing delay by n ticks) var/processing = TRUE - // How many times have we ran + /// How many times have we ran var/iteration = 0 + /// Stack end detector to detect stack overflows that kill the mc's main loop + var/datum/stack_end_detector/stack_end_detector - // world.time of last fire, for tracking lag outside of the mc + /// world.time of last fire, for tracking lag outside of the mc var/last_run - // List of subsystems to process(). + /// List of subsystems to process(). var/list/subsystems // Vars for keeping track of tick drift. @@ -34,25 +36,27 @@ GLOBAL_REAL(Master, /datum/controller/master) = new var/init_time var/tickdrift = 0 + /// How long is the MC sleeping between runs, read only (set by Loop() based off of anti-tick-contention heuristics) var/sleep_delta = 1 - ///Only run ticker subsystems for the next n ticks. + /// Only run ticker subsystems for the next n ticks. var/skip_ticks = 0 + /// makes the mc main loop runtime var/make_runtime = FALSE var/initializations_finished_with_no_players_logged_in //I wonder what this could be? - // The type of the last subsystem to be process()'d. + /// The type of the last subsystem to be fire()'d. var/last_type_processed - var/datum/controller/subsystem/queue_head //Start of queue linked list - var/datum/controller/subsystem/queue_tail //End of queue linked list (used for appending to the list) + var/datum/controller/subsystem/queue_head //!Start of queue linked list + var/datum/controller/subsystem/queue_tail //!End of queue linked list (used for appending to the list) var/queue_priority_count = 0 //Running total so that we don't have to loop thru the queue each run to split up the tick var/queue_priority_count_bg = 0 //Same, but for background subsystems - var/map_loading = FALSE //Are we loading in a new map? + var/map_loading = FALSE //!Are we loading in a new map? - var/current_runlevel //for scheduling different subsystems for different stages of the round + var/current_runlevel //!for scheduling different subsystems for different stages of the round var/sleep_offline_after_initializations = TRUE var/static/restart_clear = 0 @@ -61,8 +65,8 @@ GLOBAL_REAL(Master, /datum/controller/master) = new var/static/random_seed - //current tick limit, assigned before running a subsystem. - //used by CHECK_TICK as well so that the procs subsystems call can obey that SS's tick limits + ///current tick limit, assigned before running a subsystem. + ///used by CHECK_TICK as well so that the procs subsystems call can obey that SS's tick limits var/static/current_ticklimit = TICK_LIMIT_RUNNING /datum/controller/master/New() @@ -305,8 +309,12 @@ GLOBAL_REAL(Master, /datum/controller/master) = new var/error_level = 0 var/sleep_delta = 1 var/list/subsystems_to_check + + //setup the stack overflow detector + stack_end_detector = new() + var/datum/stack_canary/canary = stack_end_detector.prime_canary() + canary.use_variable() //the actual loop. - while (1) tickdrift = max(0, MC_AVERAGE_FAST(tickdrift, (((REALTIMEOFDAY - init_timeofday) - (world.time - init_time)) / world.tick_lag))) var/starting_tick_usage = TICK_USAGE diff --git a/code/datums/helper_datums/stack_end_detector.dm b/code/datums/helper_datums/stack_end_detector.dm new file mode 100644 index 00000000000..0d621f51b36 --- /dev/null +++ b/code/datums/helper_datums/stack_end_detector.dm @@ -0,0 +1,32 @@ +/** + Stack End Detector. + Can detect if a given code stack has exited, used by the mc for stack overflow detection. + + **/ +/datum/stack_end_detector + var/datum/weakref/_WF + var/datum/stack_canary/_canary + +/datum/stack_end_detector/New() + _canary = new() + _WF = WEAKREF(_canary) + +/** Prime the stack overflow detector. + Store the return value of this proc call in a proc level var. + Can only be called once. +**/ +/datum/stack_end_detector/proc/prime_canary() + if (!_canary) + CRASH("Prime_canary called twice") + . = _canary + _canary = null + +/// Returns true if the stack is still going. Calling before the canary has been primed also returns true +/datum/stack_end_detector/proc/check() + return !!_WF.resolve() + +/// Stack canary. Will go away if the stack it was primed by is ended by byond for return or stack overflow reasons. +/datum/stack_canary + +/// empty proc to avoid warnings about unused variables. Call this proc on your canary in the stack it's watching. +/datum/stack_canary/proc/use_variable() diff --git a/tgstation.dme b/tgstation.dme index 2770bec6afe..2a56e19513a 100644 --- a/tgstation.dme +++ b/tgstation.dme @@ -625,6 +625,7 @@ #include "code\datums\helper_datums\events.dm" #include "code\datums\helper_datums\getrev.dm" #include "code\datums\helper_datums\icon_snapshot.dm" +#include "code\datums\helper_datums\stack_end_detector.dm" #include "code\datums\helper_datums\teleport.dm" #include "code\datums\keybinding\_defines.dm" #include "code\datums\keybinding\_keybindings.dm"