Skip to content

Conversation

@HolonProduction
Copy link
Member

@HolonProduction HolonProduction commented Jan 6, 2026

Alternative to #114801

Fixes #114420
Fixes #98141 (tested using the 4.3 MRP by myself. Not the original one.)

Supersedes #114623


Might be related to: #95428 -> No MRP, but my hypothesis is, that script members were cleared from a still referenced script.


When a GDScript is freed it searches for cycles refs amongst its dependencies and clears those as well. However the current implementation does not account for cases were certain scripts are still referenced but by something that is not GDScript. The only indication we have of such a reference is the refcount which the current algorithm does never even look at.

This PR introduces a new approach:

  1. Use Tarjan's Algorithm to find strongly connected components (SCC) (almost the same as cycles but we need to account for overlapping cycles) between the transitive dependencies of the script that gets cleaned up.
  2. topological sort those.
  3. For each SCC count the sum of refcounts, and the sum of internal (from the SCC to the same SCC) references that GDScript knows of. If the difference is 0 the SSC can be cleared as well.
    • If an SCC can be cleared substract it's known references to all other SCC so that those can be cleared as well if they only are alive because of a clearable SCC. -> This works because of the topological sorting.

  • Pass tests
  • Verify linked issues
  • Verify that cyclic refs get properly cleaned when it is safe -> tested using this toy project: cycle_cleanup.zip -> requires unreference to actually trigger, that's not an issue with this PR though, GDScriptCache holds references to all scripts so they only get destructed in rare cases
  • Cleanup code
    • Find transitive deps during Tarjans DFS instead of doing a second DFS before.

@adamscott
Copy link
Member

adamscott commented Jan 6, 2026

I would put your new code inside a class TarjansSCCAlgorithm defined in gdscript.cpp. It will make it easier to follow. Also, if possible, it would be nice if the algorithm code symbols to use the same standards as the rest of the code.

Here's a list of names I found:

  • strongconnect() parameters and variables:
    • v ➡️ p_visited_node_index
    • w ➡️ visited_node_successor_index
    • head ➡️ stack_head

Also, is this possible to an added boolean for "on the stack/not on the stack" instead of negative index meaning something other than an index?

@AThousandShips AThousandShips added this to the 4.6 milestone Jan 7, 2026
@HolonProduction HolonProduction force-pushed the gdscripts/cycles-was-easier-in-geometry-dash branch 6 times, most recently from 7cac3d9 to c02cb8c Compare January 9, 2026 16:42
@HolonProduction HolonProduction force-pushed the gdscripts/cycles-was-easier-in-geometry-dash branch from c02cb8c to 2774d34 Compare January 10, 2026 12:21
@HolonProduction HolonProduction marked this pull request as ready for review January 10, 2026 13:11
@HolonProduction HolonProduction requested a review from a team as a code owner January 10, 2026 13:11
@akien-mga akien-mga requested review from adamscott and vnen January 10, 2026 22:21
@akien-mga
Copy link
Member

Superseded by #114801.

@akien-mga akien-mga closed this Jan 12, 2026
@akien-mga akien-mga removed this from the 4.6 milestone Jan 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

4 participants