diff --git a/code/controllers/subsystem/garbage.dm b/code/controllers/subsystem/garbage.dm index 4f11a3ee577..706b4e51d69 100644 --- a/code/controllers/subsystem/garbage.dm +++ b/code/controllers/subsystem/garbage.dm @@ -162,6 +162,9 @@ SUBSYSTEM_DEF(garbage) lastlevel = level +// 1 from the hard reference in the queue, and 1 from the variable used before this +#define REFS_WE_EXPECT 2 + //We do this rather then for(var/list/ref_info in queue) because that sort of for loop copies the whole list. //Normally this isn't expensive, but the gc queue can grow to 40k items, and that gets costly/causes overrun. for (var/i in 1 to length(queue)) @@ -179,9 +182,8 @@ SUBSYSTEM_DEF(garbage) var/datum/D = L[GC_QUEUE_ITEM_REF] - // 1 from the hard reference in the queue, and 1 from the variable used before this // If that's all we've got, send er off - if (refcount(D) == 2) + if (refcount(D) == REFS_WE_EXPECT) ++gcedlasttick ++totalgcs pass_counts[level]++ @@ -202,12 +204,15 @@ SUBSYSTEM_DEF(garbage) switch (level) if (GC_QUEUE_CHECK) #ifdef REFERENCE_TRACKING + // Decides how many refs to look for (potentially) + // Based off the remaining and the ones we can account for + var/remaining_refs = refcount(D) - REFS_WE_EXPECT if(reference_find_on_fail[text_ref(D)]) - INVOKE_ASYNC(D, TYPE_PROC_REF(/datum,find_references)) + INVOKE_ASYNC(D, TYPE_PROC_REF(/datum,find_references), remaining_refs) ref_searching = TRUE #ifdef GC_FAILURE_HARD_LOOKUP else - INVOKE_ASYNC(D, TYPE_PROC_REF(/datum,find_references)) + INVOKE_ASYNC(D, TYPE_PROC_REF(/datum,find_references), remaining_refs) ref_searching = TRUE #endif reference_find_on_fail -= text_ref(D) @@ -257,6 +262,8 @@ SUBSYSTEM_DEF(garbage) queue.Cut(1,count+1) count = 0 +#undef REFS_WE_EXPECT + /datum/controller/subsystem/garbage/proc/Queue(datum/D, level = GC_QUEUE_FILTER) if (isnull(D)) return diff --git a/code/datums/datum.dm b/code/datums/datum.dm index 07fad7092a3..e98f0941d64 100644 --- a/code/datums/datum.dm +++ b/code/datums/datum.dm @@ -60,8 +60,12 @@ var/list/filter_data #ifdef REFERENCE_TRACKING - var/running_find_references + /// When was this datum last touched by a reftracker? + /// If this value doesn't match with the start of the search + /// We know this datum has never been seen before, and we should check it var/last_find_references = 0 + /// How many references we're trying to find when searching + var/references_to_clear = 0 #ifdef REFERENCE_TRACKING_DEBUG ///Stores info about where refs are found, used for sanity checks and testing var/list/found_refs diff --git a/code/game/machinery/camera/camera.dm b/code/game/machinery/camera/camera.dm index f3008ca4ebc..213c43b5a11 100644 --- a/code/game/machinery/camera/camera.dm +++ b/code/game/machinery/camera/camera.dm @@ -115,6 +115,11 @@ MAPPING_DIRECTIONAL_HELPERS(/obj/machinery/camera/xray, 0) /obj/machinery/camera/proc/create_prox_monitor() if(!proximity_monitor) proximity_monitor = new(src, 1) + RegisterSignal(proximity_monitor, COMSIG_QDELETING, PROC_REF(proximity_deleted)) + +/obj/machinery/camera/proc/proximity_deleted() + SIGNAL_HANDLER + proximity_monitor = null /obj/machinery/camera/proc/set_area_motion(area/A) area_motion = A diff --git a/code/game/machinery/gulag_item_reclaimer.dm b/code/game/machinery/gulag_item_reclaimer.dm index 9c649b2c8ee..e1c2c8c73d6 100644 --- a/code/game/machinery/gulag_item_reclaimer.dm +++ b/code/game/machinery/gulag_item_reclaimer.dm @@ -30,6 +30,7 @@ I.forceMove(get_turf(src)) if(linked_teleporter) linked_teleporter.linked_reclaimer = null + linked_teleporter = null return ..() /obj/machinery/gulag_item_reclaimer/emag_act(mob/user, obj/item/card/emag/emag_card) diff --git a/code/game/machinery/gulag_teleporter.dm b/code/game/machinery/gulag_teleporter.dm index ce46a5d2fce..2fffb98d56c 100644 --- a/code/game/machinery/gulag_teleporter.dm +++ b/code/game/machinery/gulag_teleporter.dm @@ -43,6 +43,7 @@ The console is located at computer/gulag_teleporter.dm /obj/machinery/gulag_teleporter/Destroy() if(linked_reclaimer) linked_reclaimer.linked_teleporter = null + linked_reclaimer = null return ..() /obj/machinery/gulag_teleporter/interact(mob/user) diff --git a/code/modules/admin/view_variables/reference_tracking.dm b/code/modules/admin/view_variables/reference_tracking.dm index 645467bf0fe..b9fd0e6d2ad 100644 --- a/code/modules/admin/view_variables/reference_tracking.dm +++ b/code/modules/admin/view_variables/reference_tracking.dm @@ -1,36 +1,29 @@ #ifdef REFERENCE_TRACKING +#define REFSEARCH_RECURSE_LIMIT 64 -/datum/proc/find_references(skip_alert) - running_find_references = type +/datum/proc/find_references(references_to_clear = INFINITY) if(usr?.client) - if(usr.client.running_find_references) - log_reftracker("CANCELLED search for references to a [usr.client.running_find_references].") - usr.client.running_find_references = null - running_find_references = null - //restart the garbage collector - SSgarbage.can_fire = TRUE - SSgarbage.update_nextfire(reset_time = TRUE) - return - - if(!skip_alert && tgui_alert(usr,"Running this will lock everything up for about 5 minutes. Would you like to begin the search?", "Find References", list("Yes", "No")) != "Yes") - running_find_references = null + if(tgui_alert(usr,"Running this will lock everything up for about 5 minutes. Would you like to begin the search?", "Find References", list("Yes", "No")) != "Yes") return + src.references_to_clear = references_to_clear //this keeps the garbage collector from failing to collect objects being searched for in here SSgarbage.can_fire = FALSE - if(usr?.client) - usr.client.running_find_references = type + _search_references() + //restart the garbage collector + SSgarbage.can_fire = TRUE + SSgarbage.update_nextfire(reset_time = TRUE) - log_reftracker("Beginning search for references to a [type].") +/datum/proc/_search_references() + log_reftracker("Beginning search for references to a [type], looking for [references_to_clear] refs.") var/starting_time = world.time - - log_reftracker("Refcount for [type]: [refcount(src)]") - //Time to search the whole game for our ref - DoSearchVar(GLOB, "GLOB", search_time = starting_time) //globals + DoSearchVar(GLOB, "GLOB", starting_time) //globals log_reftracker("Finished searching globals") + if(src.references_to_clear == 0) + return //Yes we do actually need to do this. The searcher refuses to read weird lists //And global.vars is a really weird list @@ -38,45 +31,46 @@ for(var/key in global.vars) global_vars[key] = global.vars[key] - DoSearchVar(global_vars, "Native Global", search_time = starting_time) + DoSearchVar(global_vars, "Native Global", starting_time) log_reftracker("Finished searching native globals") + if(src.references_to_clear == 0) + return for(var/datum/thing in world) //atoms (don't beleive its lies) - DoSearchVar(thing, "World -> [thing.type]", search_time = starting_time) + DoSearchVar(thing, "World -> [thing.type]", starting_time) + if(src.references_to_clear == 0) + break log_reftracker("Finished searching atoms") + if(src.references_to_clear == 0) + return for(var/datum/thing) //datums - DoSearchVar(thing, "Datums -> [thing.type]", search_time = starting_time) + DoSearchVar(thing, "Datums -> [thing.type]", starting_time) + if(src.references_to_clear == 0) + break log_reftracker("Finished searching datums") + if(src.references_to_clear == 0) + return //Warning, attempting to search clients like this will cause crashes if done on live. Watch yourself #ifndef REFERENCE_DOING_IT_LIVE for(var/client/thing) //clients - DoSearchVar(thing, "Clients -> [thing.type]", search_time = starting_time) + DoSearchVar(thing, "Clients -> [thing.type]", starting_time) + if(src.references_to_clear == 0) + break log_reftracker("Finished searching clients") + if(src.references_to_clear == 0) + return #endif log_reftracker("Completed search for references to a [type].") - if(usr?.client) - usr.client.running_find_references = null - running_find_references = null - - //restart the garbage collector - SSgarbage.can_fire = TRUE - SSgarbage.update_nextfire(reset_time = TRUE) - -/datum/proc/DoSearchVar(potential_container, container_name, recursive_limit = 64, search_time = world.time) - #ifdef REFERENCE_TRACKING_DEBUG - if(SSgarbage.should_save_refs && !found_refs) - found_refs = list() - #endif - - if(usr?.client && !usr.client.running_find_references) +/datum/proc/DoSearchVar(potential_container, container_name, search_time, recursion_count, is_special_list) + if(recursion_count >= REFSEARCH_RECURSE_LIMIT) + log_reftracker("Recursion limit reached. [container_name]") return - if(!recursive_limit) - log_reftracker("Recursion limit reached. [container_name]") + if(references_to_clear == 0) return //Check each time you go down a layer. This makes it a bit slow, but it won't effect the rest of the game at all @@ -90,73 +84,116 @@ return datum_container.last_find_references = search_time - var/container_print = datum_container.ref_search_details() var/list/vars_list = datum_container.vars + var/is_atom = FALSE + var/is_area = FALSE + if(isatom(datum_container)) + is_atom = TRUE + if(isarea(datum_container)) + is_area = TRUE for(var/varname in vars_list) - #ifndef FIND_REF_NO_CHECK_TICK - CHECK_TICK - #endif - if (varname == "vars" || varname == "vis_locs") //Fun fact, vis_locs don't count for references - continue var/variable = vars_list[varname] - - if(variable == src) + if(islist(variable)) + //Fun fact, vis_locs don't count for references + if(varname == "vars" || (is_atom && (varname == "vis_locs" || varname == "overlays" || varname == "underlays" || varname == "filters" || varname == "verbs" || (is_area && varname == "contents")))) + continue + // We do this after the varname check to avoid area contents (reading it incures a world loop's worth of cost) + if(!length(variable)) + continue + DoSearchVar(variable,\ + "[container_name] [datum_container.ref_search_details()] -> [varname] (list)",\ + search_time,\ + recursion_count + 1,\ + /*is_special_list = */ is_atom && (varname == "contents" || varname == "vis_contents" || varname == "locs")) + else if(variable == src) #ifdef REFERENCE_TRACKING_DEBUG if(SSgarbage.should_save_refs) + if(!found_refs) + found_refs = list() found_refs[varname] = TRUE continue //End early, don't want these logging + else + log_reftracker("Found [type] [text_ref(src)] in [datum_container.type]'s [datum_container.ref_search_details()] [varname] var. [container_name]") + #else + log_reftracker("Found [type] [text_ref(src)] in [datum_container.type]'s [datum_container.ref_search_details()] [varname] var. [container_name]") #endif - log_reftracker("Found [type] [text_ref(src)] in [datum_container.type]'s [container_print] [varname] var. [container_name]") + references_to_clear -= 1 + if(references_to_clear == 0) + log_reftracker("All references to [type] [text_ref(src)] found, exiting.") + return continue - if(islist(variable)) - DoSearchVar(variable, "[container_name] [container_print] -> [varname] (list)", recursive_limit - 1, search_time) - else if(islist(potential_container)) - var/normal = IS_NORMAL_LIST(potential_container) var/list/potential_cache = potential_container for(var/element_in_list in potential_cache) - #ifndef FIND_REF_NO_CHECK_TICK - CHECK_TICK - #endif - //Check normal entrys - if(element_in_list == src) - #ifdef REFERENCE_TRACKING_DEBUG - if(SSgarbage.should_save_refs) - found_refs[potential_cache] = TRUE - continue //End early, don't want these logging - #endif - log_reftracker("Found [type] [text_ref(src)] in list [container_name].") - continue - - var/assoc_val = null - if(!isnum(element_in_list) && normal) - assoc_val = potential_cache[element_in_list] - //Check assoc entrys - if(assoc_val == src) - #ifdef REFERENCE_TRACKING_DEBUG - if(SSgarbage.should_save_refs) - found_refs[potential_cache] = TRUE - continue //End early, don't want these logging - #endif - log_reftracker("Found [type] [text_ref(src)] in list [container_name]\[[element_in_list]\]") - continue - //We need to run both of these checks, since our object could be hiding in either of them //Check normal sublists if(islist(element_in_list)) - DoSearchVar(element_in_list, "[container_name] -> [element_in_list] (list)", recursive_limit - 1, search_time) - //Check assoc sublists - if(islist(assoc_val)) - DoSearchVar(potential_container[element_in_list], "[container_name]\[[element_in_list]\] -> [assoc_val] (list)", recursive_limit - 1, search_time) + if(length(element_in_list)) + DoSearchVar(element_in_list, "[container_name] -> [element_in_list] (list)", search_time, recursion_count + 1) + //Check normal entrys + else if(element_in_list == src) + #ifdef REFERENCE_TRACKING_DEBUG + if(SSgarbage.should_save_refs) + if(!found_refs) + found_refs = list() + found_refs[potential_cache] = TRUE + continue + else + log_reftracker("Found [type] [text_ref(src)] in list [container_name].") + #else + log_reftracker("Found [type] [text_ref(src)] in list [container_name].") + #endif -/proc/qdel_and_find_ref_if_fail(datum/thing_to_del, force = FALSE) - thing_to_del.qdel_and_find_ref_if_fail(force) + // This is dumb as hell I'm sorry + // I don't want the garbage subsystem to count as a ref for the purposes of this number + // If we find all other refs before it I want to early exit, and if we don't I want to keep searching past it + var/ignore_ref = FALSE + var/list/queues = SSgarbage.queues + for(var/list/queue in queues) + if(potential_cache in queue) + ignore_ref = TRUE + break + if(ignore_ref) + log_reftracker("[container_name] does not count as a ref for our count") + else + references_to_clear -= 1 + if(references_to_clear == 0) + log_reftracker("All references to [type] [text_ref(src)] found, exiting.") + return -/datum/proc/qdel_and_find_ref_if_fail(force = FALSE) - SSgarbage.reference_find_on_fail[text_ref(src)] = TRUE - qdel(src, force) + if(!isnum(element_in_list) && !is_special_list) + // This exists to catch an error that throws when we access a special list + // is_special_list is a hint, it can be wrong + try + var/assoc_val = potential_cache[element_in_list] + //Check assoc sublists + if(islist(assoc_val)) + if(length(assoc_val)) + DoSearchVar(potential_container[element_in_list], "[container_name]\[[element_in_list]\] -> [assoc_val] (list)", search_time, recursion_count + 1) + //Check assoc entry + else if(assoc_val == src) + #ifdef REFERENCE_TRACKING_DEBUG + if(SSgarbage.should_save_refs) + if(!found_refs) + found_refs = list() + found_refs[potential_cache] = TRUE + continue + else + log_reftracker("Found [type] [text_ref(src)] in list [container_name]\[[element_in_list]\]") + #else + log_reftracker("Found [type] [text_ref(src)] in list [container_name]\[[element_in_list]\]") + #endif + references_to_clear -= 1 + if(references_to_clear == 0) + log_reftracker("All references to [type] [text_ref(src)] found, exiting.") + return + catch + // So if it goes wrong we kill it + is_special_list = TRUE + log_reftracker("Curiosity: [container_name] lead to an error when acessing [element_in_list], what is it?") +#undef REFSEARCH_RECURSE_LIMIT #endif // Kept outside the ifdef so overrides are easy to implement diff --git a/code/modules/atmospherics/machinery/components/unary_devices/machine_connector.dm b/code/modules/atmospherics/machinery/components/unary_devices/machine_connector.dm index b1e2567f82f..dc1c2d6282f 100644 --- a/code/modules/atmospherics/machinery/components/unary_devices/machine_connector.dm +++ b/code/modules/atmospherics/machinery/components/unary_devices/machine_connector.dm @@ -21,12 +21,19 @@ gas_connector.set_init_directions() gas_connector.atmos_init() SSair.add_to_rebuild_queue(gas_connector) + RegisterSignal(gas_connector, COMSIG_QDELETING, PROC_REF(connector_deleted)) /datum/gas_machine_connector/Destroy() connected_machine = null QDEL_NULL(gas_connector) return ..() +/datum/gas_machine_connector/proc/connector_deleted() + SIGNAL_HANDLER + gas_connector = null + if(!QDELETED(connected_machine)) + qdel(connected_machine) + /** * Register various signals that are required for the proper work of the connector */ diff --git a/code/modules/unit_tests/find_reference_sanity.dm b/code/modules/unit_tests/find_reference_sanity.dm index 0dda1559819..4bc6445f024 100644 --- a/code/modules/unit_tests/find_reference_sanity.dm +++ b/code/modules/unit_tests/find_reference_sanity.dm @@ -15,6 +15,8 @@ return ..() /atom/movable/ref_test + // Gotta make sure we do a full check + references_to_clear = INFINITY var/atom/movable/ref_test/self_ref /atom/movable/ref_test/Destroy(force) @@ -29,9 +31,9 @@ //Sanity check var/refcount = refcount(victim) TEST_ASSERT_EQUAL(refcount, 3, "Should be: test references: 0 + baseline references: 3 (victim var,loc,allocated list)") - victim.DoSearchVar(testbed, "Sanity Check", search_time = 1) //We increment search time to get around an optimization + victim.DoSearchVar(testbed, "Sanity Check") //We increment search time to get around an optimization - TEST_ASSERT(!victim.found_refs.len, "The ref-tracking tool found a ref where none existed") + TEST_ASSERT(!LAZYLEN(victim.found_refs), "The ref-tracking tool found a ref where none existed") SSgarbage.should_save_refs = FALSE /datum/unit_test/find_reference_baseline/Run() @@ -46,11 +48,11 @@ var/refcount = refcount(victim) TEST_ASSERT_EQUAL(refcount, 6, "Should be: test references: 3 + baseline references: 3 (victim var,loc,allocated list)") - victim.DoSearchVar(testbed, "First Run", search_time = 2) + victim.DoSearchVar(testbed, "First Run") - TEST_ASSERT(victim.found_refs["test"], "The ref-tracking tool failed to find a regular value") - TEST_ASSERT(victim.found_refs[testbed.test_list], "The ref-tracking tool failed to find a list entry") - TEST_ASSERT(victim.found_refs[testbed.test_assoc_list], "The ref-tracking tool failed to find an assoc list value") + TEST_ASSERT(LAZYACCESS(victim.found_refs, "test"), "The ref-tracking tool failed to find a regular value") + TEST_ASSERT(LAZYACCESS(victim.found_refs, testbed.test_list), "The ref-tracking tool failed to find a list entry") + TEST_ASSERT(LAZYACCESS(victim.found_refs, testbed.test_assoc_list), "The ref-tracking tool failed to find an assoc list value") SSgarbage.should_save_refs = FALSE /datum/unit_test/find_reference_exotic/Run() @@ -65,12 +67,12 @@ var/refcount = refcount(victim) TEST_ASSERT_EQUAL(refcount, 6, "Should be: test references: 3 + baseline references: 3 (victim var,loc,allocated list)") - victim.DoSearchVar(testbed, "Second Run", search_time = 3) + victim.DoSearchVar(testbed, "Second Run") //This is another sanity check - TEST_ASSERT(!victim.found_refs[testbed.overlays], "The ref-tracking tool found an overlays entry? That shouldn't be possible") - TEST_ASSERT(victim.found_refs[testbed.vis_contents], "The ref-tracking tool failed to find a vis_contents entry") - TEST_ASSERT(victim.found_refs[testbed.test_assoc_list], "The ref-tracking tool failed to find an assoc list key") + TEST_ASSERT(!LAZYACCESS(victim.found_refs, testbed.overlays), "The ref-tracking tool found an overlays entry? That shouldn't be possible") + TEST_ASSERT(LAZYACCESS(victim.found_refs, testbed.vis_contents), "The ref-tracking tool failed to find a vis_contents entry") + TEST_ASSERT(LAZYACCESS(victim.found_refs, testbed.test_assoc_list), "The ref-tracking tool failed to find an assoc list key") SSgarbage.should_save_refs = FALSE /datum/unit_test/find_reference_esoteric/Run() @@ -87,12 +89,12 @@ var/refcount = refcount(victim) TEST_ASSERT_EQUAL(refcount, 6, "Should be: test references: 3 + baseline references: 3 (victim var,loc,allocated list)") - victim.DoSearchVar(victim, "Third Run Self", search_time = 4) - victim.DoSearchVar(testbed, "Third Run Testbed", search_time = 4) + victim.DoSearchVar(victim, "Third Run Self") + victim.DoSearchVar(testbed, "Third Run Testbed") - TEST_ASSERT(victim.found_refs["self_ref"], "The ref-tracking tool failed to find a self reference") - TEST_ASSERT(victim.found_refs[to_find], "The ref-tracking tool failed to find a nested list entry") - TEST_ASSERT(victim.found_refs[to_find_assoc], "The ref-tracking tool failed to find a nested assoc list entry") + TEST_ASSERT(LAZYACCESS(victim.found_refs, "self_ref"), "The ref-tracking tool failed to find a self reference") + TEST_ASSERT(LAZYACCESS(victim.found_refs, to_find), "The ref-tracking tool failed to find a nested list entry") + TEST_ASSERT(LAZYACCESS(victim.found_refs, to_find_assoc), "The ref-tracking tool failed to find a nested assoc list entry") SSgarbage.should_save_refs = FALSE /datum/unit_test/find_reference_null_key_entry/Run() @@ -104,9 +106,9 @@ testbed.test_assoc_list = list(null = victim) var/refcount = refcount(victim) TEST_ASSERT_EQUAL(refcount, 4, "Should be: test references: 1 + baseline references: 3 (victim var,loc,allocated list)") - victim.DoSearchVar(testbed, "Fourth Run", search_time = 5) + victim.DoSearchVar(testbed, "Fourth Run") - TEST_ASSERT(testbed.test_assoc_list, "The ref-tracking tool failed to find a null key'd assoc list entry") + TEST_ASSERT(LAZYACCESS(victim.found_refs, testbed.test_assoc_list), "The ref-tracking tool failed to find a null key'd assoc list entry") /datum/unit_test/find_reference_assoc_investigation/Run() var/atom/movable/ref_test/victim = allocate(/atom/movable/ref_test) @@ -121,10 +123,10 @@ var/refcount = refcount(victim) TEST_ASSERT_EQUAL(refcount, 5, "Should be: test references: 2 + baseline references: 3 (victim var,loc,allocated list)") - victim.DoSearchVar(testbed, "Fifth Run", search_time = 6) + victim.DoSearchVar(testbed, "Fifth Run") - TEST_ASSERT(victim.found_refs[to_find_in_key], "The ref-tracking tool failed to find a nested assoc list key") - TEST_ASSERT(victim.found_refs[to_find_null_assoc_nested], "The ref-tracking tool failed to find a null key'd nested assoc list entry") + TEST_ASSERT(LAZYACCESS(victim.found_refs, to_find_in_key), "The ref-tracking tool failed to find a nested assoc list key") + TEST_ASSERT(LAZYACCESS(victim.found_refs, to_find_null_assoc_nested), "The ref-tracking tool failed to find a null key'd nested assoc list entry") SSgarbage.should_save_refs = FALSE /datum/unit_test/find_reference_static_investigation/Run() @@ -144,7 +146,7 @@ var/refcount = refcount(victim) TEST_ASSERT_EQUAL(refcount, 5, "Should be: test references: 2 + baseline references: 3 (victim var,loc,allocated list)") - victim.DoSearchVar(global_vars, "Sixth Run", search_time = 7) + victim.DoSearchVar(global_vars, "Sixth Run") - TEST_ASSERT(victim.found_refs[global_vars], "The ref-tracking tool failed to find a natively global variable") + TEST_ASSERT(LAZYACCESS(victim.found_refs, global_vars), "The ref-tracking tool failed to find a natively global variable") SSgarbage.should_save_refs = FALSE