Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add tests cases for, and fix, #513 #524

Merged
merged 3 commits into from
Aug 20, 2024
Merged

Add tests cases for, and fix, #513 #524

merged 3 commits into from
Aug 20, 2024

Commits on Aug 19, 2024

  1. add some support for using a local libgodot build

    Rob Mayoff committed Aug 19, 2024
    Configuration menu
    Copy the full SHA
    f8913ef View commit details
    Browse the repository at this point in the history

  2. fix better the first leak of migueldeicaza#513 

    The changes in d1d6ca4 made several `Geometry2DTests` fail.

The actual unbalanced retain causing the leak in
`VariantRepresentable.swift`:

``` swift
extension ContentVariantRepresentable {
    public init? (_ variant: Variant) {
        guard Self.godotType == variant.gtype else { return nil }

        var content = Self.zero
        withUnsafeMutablePointer(to: &content) { ptr in
            variant.toType(Self.godotType, dest: ptr) // <-- THIS IS THE UNBALANCED RETAIN
        }

        self.init(content: content)
    }
}
```

The `variant.toType` function calls into Godot to copy out the built-in
type stored in `variant`. If that built-in type has an internal
reference count, Godot increments it. Then, the call to
`self.init(content: content)` increments the count again. The retain
performed by `variant.toType` is never balanced.

This patch fixes the bug by generating a new
`init(alreadyOwnedContent:)` initializer for each Godot builtin class
type. This needs to be on the builtin wrappers (like `GDictionary`,
`GArray`, etc.), rather than on `Variant` which is where d1d6ca4 put it.

This patch also adds a test case to check for the leak by looking at
Godot's `MEMORY_STATIC` performance counter, which is only enabled if
Godot was built with `DEBUG_ENABLED`.
    Rob Mayoff committed Aug 19, 2024
    Configuration menu
    Copy the full SHA
    eecdd30 View commit details
    Browse the repository at this point in the history

  3. fix second leak of migueldeicaza#513 

    This patch adds a test for the second leak described in migueldeicaza#513, and fixes
the leak.

Without this patch, the leak happens because `Variant.asObject` has an
unneeded call to `rc.reference()` which increments the `RefCounted`
object's reference count. As far as I can tell, nothing balances this
increment.

`Variant.asObject` calls `lookupObject(nativeHandle:)`, which returns
a Swift wrapper for an object whose reference count has already been
incremented (if the object is `RefCounted`). The Swift wrapper
balances that increment with a decrement in its `deinit`.
    Rob Mayoff committed Aug 19, 2024
    Configuration menu
    Copy the full SHA
    430478e View commit details
    Browse the repository at this point in the history