Godot: [2.1, 3.0, 3.1, 3.2, 3.2.1] Dangling variant issue

[xsellier]

Related PR:
https://github.com/godotengine/godot/pull/38119

Godot version:
2.1.*, 3.0, 3.1, 3.2, 3.2.1

Godot Version where is the bug is fixed:
3.2.2 (haven't tested the master branch)

OS/device including version:
N/A

Issue description:
Dangling pointer 101
I was talking with @RandomShaper about that issue.
Backporting the linked PR to those versions would fix the issue in debug version, but it would still happen in release version.
I backported it, and removed the debug flags from the PR, after having ran several benchmarks, I noticed that the performances are somewhat identical

Steps to reproduce:
Start a new project, add the following code to the _init function:

    var a = {
        control = Control.new()
    }

    print(a)
    # Print: Control[876]
    a.control.free()
    Control.new()
    print(a)
    # Print: Control[877] instead of [Deleted Object]

Minimal reproduction project:
N/A

[xsellier]

I made some more debugging, and I noticed the following behavior:

    var a = {
        control = Control.new()
    }

    print(a)
    # Print: Control[876]
    a.control.free()
    Node.new()
    print(a)
    # Print: [Deleted Object]

It seems to happens when you create the same type of object.

[jkb0o]

Backporting the linked PR to those versions would fix the issue in debug version, but it would still happen in release version.
I backported it, and removed the debug flags from the PR, after having ran several benchmarks, I noticed that the performances are somewhat identical

@RandomShaper are there any plans to make #38119 make work in release build as well as debug? I'd like to move to 3.2.3 and discard my own fix of this bug, but with a debug-only fix, it seems difficult.

[akien-mga]

Assigning 4.0 milestone as this still happens in 3.2.2+ and master (*) in release builds, so we need a thorough fix for release mode that can also be backported to 3.2.x, possibly 2.1.x, and if there's demand for it 3.0.x or 3.1.x (though I consider 3.0.x nearly EOL).

(*) I couldn't confirm for master yet as GDScript seems broken in target=release builds, I can't get it to print() :)

[xsellier]

I already backported the PR on the v2.1 branch, but it changes a lot of files, plus it does not compile for Windows debug since the __atomic functions are not builtin in the MSVC compiler (only in GCC). There a 2 solutions, make a wrapper for MSVC (but it would be tricky), or use \

Or even better, find a workaround that does not require the __atomic functions

[girng]

On 2.1.6, it prints (control:[Deleted Object])

[xsellier]

On 2.1.6, it prints (control:[Deleted Object])

@girng I don't have the same stuff here (this is v2.1.6 official):

    var a = {
        control = Control.new()
    }

    print(a)
    # Print: Control[559]
    a.control.free()
    Control.new()
    print(a)
    # Print: Control[560] instead of [Deleted Object]

[RandomShaper]

The main bug report here is indeed what is expected to happen.

@xsellier, also found that in some cases the bug doesn't show up even in release builds. After thinking about this, I can say that in a release build either can happen: a new object is put where a freed one was or the new one gets a different memory address. What happens depends on how the allocator works, the memory size of the object types, etc.

Since @xsellier also did some benchmarking with positive results and so many people is asking for dangling Variants to be prevented on release builds as well, I think it's time to really consider it. That would also affect 4.0. The approach there is different so it should require some separate benchmarking and some further discussions.

[xsellier]

@RandomShaper Hey,

I made a fix for the dangling pointer issue, it is inspired from your own fix, but not using ObjectRC, only the instance_id value.
So far so good on linux. Haven't tested it on Windows or OSX yet. I will do it this weekend and update the PR afterwise.