Orama-Interactive / Pixelorama

Unleash your creativity with Pixelorama, a powerful and accessible open-source pixel art multitool. Whether you want to create sprites, tiles, animations, or just express yourself in the language of pixel art, this software will realize your pixel-perfect dreams with a vast toolbox of features. Available on Windows, Linux, macOS and the Web!
https://orama-interactive.itch.io/pixelorama
MIT License
7.17k stars 383 forks source link

Leak caused by returning objects from classes #141

Closed qarmin closed 4 years ago

qarmin commented 4 years ago

Pixelorama latest 0.61

When I open and close editor I have this errors

ERROR: ~List: Condition "_first != __null" is true.
   At: ./core/self_list.h:112.
ERROR: ~List: Condition "_first != __null" is true.
   At: ./core/self_list.h:112.
WARNING: cleanup: ObjectDB Instances still exist!
   At: core/object.cpp:2071.
ERROR: clear: Resources Still in use at Exit!
   At: core/resource.cpp:476.

This is Godot issue - https://github.com/godotengine/godot/issues/30668 - but I think that create here an issue may accelerate the resolution of the problem.

The problem is with two files

Leaked instance: GDScript:5494 - Resource name:  Path: res://Scripts/Palette/Palette.gd
Leaked instance: GDScript:5495 - Resource name:  Path: res://Scripts/Palette/PaletteColor.gd

Each file is named class(with class_name) which in one of its function returns an object of its own type PaletteColor.gd

extends Reference
class_name PaletteColor

func fromDict(input_dict : Dictionary) -> PaletteColor:
    var result = get_script().new()

    result.data = input_dict.data
    result.name = input_dict.name

    return result

and Palette.gd

class_name Palette

func deserialize(input_string : String) -> Palette:
    var result = get_script().new()

    var result_json = JSON.parse(input_string)
OverloadedOrama commented 4 years ago

Hello, thanks for reporting the issue. I wasn't aware that this would cause leaks. Should we try and find a workaround for this issue or should we just wait until it's fixed in Godot?

qarmin commented 4 years ago

Workaround for now is to remove all info about types, if type of returned object or created variable is same as type of class e.g.

class rr
var qq : rr = null      # will cause leak
var qq2                 # no leak
func zz() ->  rr:       # leak
pass
func zz2():             # no leak
pass

For now this patch should remove leak

diff --git a/Scripts/Palette/Palette.gd b/Scripts/Palette/Palette.gd
index f8ece9a..60ca46d 100644
--- a/Scripts/Palette/Palette.gd
+++ b/Scripts/Palette/Palette.gd
@@ -72,8 +72,8 @@ func save_to_file(path : String) -> void:
        file.store_string(_serialize())
        file.close()

-func duplicate() -> Palette:
-       var copy : Palette = get_script().new()
+func duplicate():
+       var copy = get_script().new()
        copy.name = name
        copy.comments = comments
        copy.editable = editable
@@ -96,7 +96,7 @@ func _serialize() -> String:

        return result

-func deserialize(input_string : String) -> Palette:
+func deserialize(input_string : String):
        var result = get_script().new()

        var result_json = JSON.parse(input_string)
@@ -120,8 +120,8 @@ func deserialize(input_string : String) -> Palette:

        return result

-func load_from_file(path : String) -> Palette:
-       var result : Palette = null
+func load_from_file(path : String):
+       var result = null
        var file = File.new()

        if file.file_exists(path):
diff --git a/Scripts/Palette/PaletteColor.gd b/Scripts/Palette/PaletteColor.gd
index 806f7fd..ef74967 100644
--- a/Scripts/Palette/PaletteColor.gd
+++ b/Scripts/Palette/PaletteColor.gd
@@ -29,7 +29,7 @@ func toDict() -> Dictionary:
                }
        return result

-func fromDict(input_dict : Dictionary) -> PaletteColor:
+func fromDict(input_dict : Dictionary):
        var result = get_script().new()

        result.data = input_dict.data
@@ -37,8 +37,8 @@ func fromDict(input_dict : Dictionary) -> PaletteColor:

        return result

-func duplicate() -> PaletteColor:
-       var copy : PaletteColor = get_script().new()
+func duplicate():
+       var copy = get_script().new()
        copy.data = data
        copy.name = name
-       return copy
\ No newline at end of file
+       return copy