Fix double-free issue on ownership transfer of array parameters

jwharm · Dec 4, 2024 · 63812b7 · 63812b7
1 parent e5917c2
commit 63812b7
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 3 deletions.
diff --git a/generator/src/main/java/io/github/jwharm/javagi/generators/PreprocessingGenerator.java b/generator/src/main/java/io/github/jwharm/javagi/generators/PreprocessingGenerator.java
@@ -195,7 +195,8 @@ private void transferOwnership(MethodSpec.Builder builder) {
         }
 
         // Same, but for structs/unions: Disable the cleaner
-        else if ((target instanceof StandardLayoutType)
+        else if (target != null
+                && (! (target instanceof Alias a && a.isValueWrapper()))
                 && p.transferOwnership() != TransferOwnership.NONE
                 && !p.isOutParameter()
                 && (type.cType() == null || !type.cType().endsWith("**"))) {
@@ -205,6 +206,33 @@ else if ((target instanceof StandardLayoutType)
                     getName(),
                     ClassNames.MEMORY_CLEANER);
         }
+
+        // Same, but for arrays
+        else if (array != null
+                && array.anyType() instanceof Type elemType
+                && p.transferOwnership() != TransferOwnership.NONE
+                && !p.isOutParameter()
+                && elemType.lookup() != null) {
+            var elemTarget = elemType.lookup();
+            if (checkNull())
+                builder.beginControlFlow("if ($L != null)", getName());
+            builder.beginControlFlow("for (var _element : $L)", getName());
+            if (elemTarget.checkIsGObject()) {
+                builder.beginControlFlow("if (_element instanceof $T _gobject)",
+                                ClassNames.G_OBJECT)
+                        .addStatement("$T.debug($S, _gobject.handle().address())",
+                                ClassNames.GLIB_LOGGER,
+                                "Ref " + elemType.typeName() + " %ld")
+                        .addStatement("_gobject.ref()")
+                        .endControlFlow();
+            } else if (! (target instanceof Alias a && a.isValueWrapper())) {
+                builder.addStatement("if (_element != null) $T.yieldOwnership(_element)",
+                                ClassNames.MEMORY_CLEANER);
+            }
+            builder.endControlFlow();
+            if (checkNull())
+                builder.endControlFlow();
+        }
     }
 
     // Read the value from a pointer to a primitive value and store it

diff --git a/generator/src/main/java/io/github/jwharm/javagi/gir/Array.java b/generator/src/main/java/io/github/jwharm/javagi/gir/Array.java
@@ -70,9 +70,10 @@ public TypedValue length() {
     }
 
     public boolean unknownSize() {
-        return fixedSize() <= 0 && !zeroTerminated() && length() == null;
+        return !"1".equals(attr("fixed-size"))
+                && !"1".equals(attr("zero-terminated"))
+                && attr("length") == null;
     }
-
     public String sizeExpression(boolean upcall) {
         if (attr("fixed-size") != null) return attr("fixed-size");
         TypedValue length = length();