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.

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

RefCounted and callbacks #109

Closed
migueldeicaza opened this issue Sep 8, 2023 · 2 comments
Closed

RefCounted and callbacks #109

migueldeicaza opened this issue Sep 8, 2023 · 2 comments

Comments

@migueldeicaza
Copy link
Owner

When working on the Editor branch, I noticed that when I return a Script instance from an overwritten method, that we crash due to a missing ref. A simple patch fixes this, but I am not sure that doing this to all codepaths that create an object by name is the right approach, or if this is only necessary on value returns (which only happen here).

diff --git a/Generator/Generator/ClassGen.swift b/Generator/Generator/ClassGen.swift
index a88b708..f5414eb 100644
--- a/Generator/Generator/ClassGen.swift
+++ b/Generator/Generator/ClassGen.swift
@@ -619,6 +619,12 @@ func processClass (cdef: JGodotExtensionAPIClass, outputDir: String) {
         p ("/// Ths initializer is invoked by derived classes as they chain through their most derived type name that our framework produced")
         p ("internal override init (name: StringName)") {
             p("super.init (name: name)")
+            if (cdef.name == "RefCounted") {
+                p ("reference ()")
+            }
+        }
+        if cdef.name == "RefCounted" {
+            p ("deinit { print (\"RefCounted - deinit\") } ")
         }
         
         let fastInitOverrides = cdef.inherits != nil ? "override " : ""
@migueldeicaza
Copy link
Owner Author

I suspect that the issue is that we are not retaining our own reference count when we return a value on a callback.

So perhaps the reference is only needed when we return those (we still have a table of live objects, but if our reference goes away, because the C++ takes ownership and drops the count, there is not much we can do).

@migueldeicaza
Copy link
Owner Author

This was wrong as detailed in issue #157

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant