From 25a97a0f085beb3c7e93a36297862b64f7d0a38f Mon Sep 17 00:00:00 2001 From: oranges Date: Wed, 14 Dec 2016 07:25:11 +0000 Subject: [PATCH 1/2] Add a way to detect recursive calls to qdel This is usually caused by logic in destroys that lead to the object being qdeleted again. This can cause nasty recursive loops, so an exception (with a stack trace) will prevent recursive stack crashes and also let devs chase the runtimes --- code/__DEFINES/qdel.dm | 3 ++- code/controllers/subsystem/garbage.dm | 15 +++++++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/code/__DEFINES/qdel.dm b/code/__DEFINES/qdel.dm index 78c0b532bd7..8358df737e0 100644 --- a/code/__DEFINES/qdel.dm +++ b/code/__DEFINES/qdel.dm @@ -12,4 +12,5 @@ //defines for the gc_destroyed var #define GC_QUEUED_FOR_QUEUING -1 -#define GC_QUEUED_FOR_HARD_DEL -2 \ No newline at end of file +#define GC_QUEUED_FOR_HARD_DEL -2 +#define GC_CURRENTLY_BEING_QDELETED -3 diff --git a/code/controllers/subsystem/garbage.dm b/code/controllers/subsystem/garbage.dm index 0f439d15c8e..a55b352cd41 100644 --- a/code/controllers/subsystem/garbage.dm +++ b/code/controllers/subsystem/garbage.dm @@ -121,7 +121,7 @@ var/datum/subsystem/garbage_collector/SSgarbage ++totalgcs /datum/subsystem/garbage_collector/proc/QueueForQueuing(datum/A) - if (istype(A) && isnull(A.gc_destroyed)) + if (istype(A) && A.gc_destroyed == GC_CURRENTLY_BEING_QDELETED) tobequeued += A A.gc_destroyed = GC_QUEUED_FOR_QUEUING @@ -142,7 +142,7 @@ var/datum/subsystem/garbage_collector/SSgarbage queue[refid] = gctime /datum/subsystem/garbage_collector/proc/HardQueue(datum/A) - if (istype(A) && isnull(A.gc_destroyed)) + if (istype(A) && A.gc_destroyed == GC_CURRENTLY_BEING_QDELETED) tobequeued += A A.gc_destroyed = GC_QUEUED_FOR_HARD_DEL @@ -163,6 +163,7 @@ var/datum/subsystem/garbage_collector/SSgarbage if(!istype(D)) del(D) else if(isnull(D.gc_destroyed)) + D.gc_destroyed = GC_CURRENTLY_BEING_QDELETED var/hint = D.Destroy(force) // Let our friend know they're about to get fucked up. if(!D) return @@ -198,6 +199,8 @@ var/datum/subsystem/garbage_collector/SSgarbage SSgarbage.noqdelhint["[D.type]"] = "[D.type]" testing("WARNING: [D.type] is not returning a qdel hint. It is being placed in the queue. Further instances of this type will also be queued.") SSgarbage.QueueForQueuing(D) + else if(D.gc_destroyed == GC_CURRENTLY_BEING_QDELETED) + throw EXCEPTION("[D.type] destroy proc was called multiple times, likely due to a qdel loop in the Destroy logic") // Returns 1 if the object has been queued for deletion. /proc/qdeleted(datum/D) @@ -207,6 +210,14 @@ var/datum/subsystem/garbage_collector/SSgarbage return TRUE return FALSE +// Returns true if the object's destroy has been called (set just before it is called) +/proc/qdestroying(datum/D) + if(!istype(D)) + return FALSE + if(D.gc_destroyed == GC_CURRENTLY_BEING_QDELETED) + return TRUE + return FALSE + // Default implementation of clean-up code. // This should be overridden to remove all references pointing to the object being destroyed. // Return the appropriate QDEL_HINT; in most cases this is QDEL_HINT_QUEUE. From 29298dfee9fdfd0cdc6531972a2d024734e18fe3 Mon Sep 17 00:00:00 2001 From: oranges Date: Wed, 14 Dec 2016 07:25:27 +0000 Subject: [PATCH 2/2] Reorder spacevine change turf qdel spacevines lead to them calling changeturf, lead to them being qdeleted lead to them calling changeturf etc etc This way the turf is changed before the vines are deleted, so the changeturf calls should not be hit. Further protection is added in the form of checking if that vine is actually in the process of being qdeleted, which limits the recursion to the maximum of how many vines are on the tile --- code/game/turfs/simulated/floor/misc_floor.dm | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/code/game/turfs/simulated/floor/misc_floor.dm b/code/game/turfs/simulated/floor/misc_floor.dm index 203f46e07ba..6189e068261 100644 --- a/code/game/turfs/simulated/floor/misc_floor.dm +++ b/code/game/turfs/simulated/floor/misc_floor.dm @@ -211,7 +211,9 @@ ChangeTurf(src.baseturf) /turf/open/floor/vines/ChangeTurf(turf/open/floor/T) - for(var/obj/structure/spacevine/SV in src) - qdel(SV) . = ..() + //Do this *after* the turf has changed as qdel in spacevines will call changeturf again if it hasn't + for(var/obj/structure/spacevine/SV in src) + if(!qdestroying(SV))//Helps avoid recursive loops + qdel(SV) UpdateAffectingLights()