Memory Leak in Function

[Wealthyturtle]

Description

When a function returns a variable, and the function is called but does not actually get stored in a variable (local or global), it causes a leak

Steps to Reproduce

function lore_add(i: item, s: string) :: item:
    set line (size of {_i}'s lore)+1 of {_i}'s lore to {_s}
    print_lore({_i})
    return {_i}

function print_lore(i: item):
    set {_lore::*} to {_i}'s lore
    set {_c} to 1
    loop {_lore::*}:
        broadcast "L%{_c}%: %loop-value%"
        increase {_c} by 1

command /test:
    trigger:
        lore_add(dirt, "This should only appear once")

If you run the /test command multiple times, this occurs:

As you can see, at the first call of /test there's only 1 line of lore, but then the next call of /test has 2, followed by 3, ...

Expected Behavior

Prior lore should not be stored, since it's technically local

Errors / Screenshots

Server Information

• Server version/platform: Spigot 1.14.4
• Skript version: Skript 2.4-beta5

Additional Context

Should probably test other vectors other than lore, to confirm if it's limited to lore or all expressions

Also, @bensku

[bensku]

Since when does this occur?

[Wealthyturtle]

Can test other versions of Skript, give me a moment

[Wealthyturtle]

Skript 2.4-alpha1 (Spigot 1.14.4): Exists
Skript 2.3.7 (Spigot 1.13.2): Exists
Skript 2.3.1 (Spigot 1.13.2): Exists
Skript 2.2-dev37c (Spigot 1.12.2): Exists

From here on out, running modified Skript (bug exists on Skript 2.4-beta5)

function lore_add(i: item, s: string) :: item:
    set {_lore::*} to {_i}'s lore
    set line (size of {_lore::*})+1 of {_i}'s lore to {_s}
    broadcast "%size of {_lore::*}%"
    return {_i}

command /test:
    trigger:
        lore_add(dirt, "This should only appear once")

Skript 2.2-dev36 (Spigot 1.11.2) Exists
Skript 2.2-dev34 (Spigot 1.11.2) Exists
Skript 2.2-dev33 (Spigot 1.11.2) Does not exist
Skript 2.2-dev32c (Spigot 1.11.2) Does not exist
Skript 2.2-dev31c (Spigot 1.11.2): Does not exist
Skript 2.2-dev29 (Spigot 1.11.2): Does not exist
Skript 2.2-dev13 (Spigot 1.9): Does not exist
Skript 2.1.2 (CraftBukkit 1.7.2): Functions no longer supported
Still testing other Skript versions (will update this message as I go on)

[Wealthyturtle]

@bensku Seems like Skript 2.2-dev34 broke it, try starting from there?

[bensku]

Thanks for investigating it! dev34 is so old that I'm probably not going to investigate there. Rewriting messy parts of the function system is likely faster, and produces other benefits (readable code) too.

Unfortunately this means that the fix will take a while.

[TheLimeGlass]

Potentially related #2279

[bensku]

Likely fixed in #2383. In the end, this was not terribly hard to fix.

[Wealthyturtle]

I just found out that this issue has been reintroduced in some recent Skript update, as this memory leak still occurs in Skript 2.5-alpha3.

[Wealthyturtle]

Just to note, it seems like the memory leak was never resolved by the function overhaul. See #2877 for some memory profiles taken that show the leak.

[TPGamesNL]

The lore part of this issue is caused by #3779 (see the comment for example with items)
The loop memory leak (#2877) is caused by Loop#current not being removed from:

(it might also be Loop#currentIter, but that field at least has remove called somewhere, so it's more likely to be Loop#current)

So this doesn't just affect functions