From bf72e20818b8a73b783fe4b62452b65d46c9254c Mon Sep 17 00:00:00 2001 From: Jan-Willem Harmannij Date: Mon, 13 Jan 2025 22:24:48 +0100 Subject: [PATCH] Do not free unowned GList/GSlist (fixes #183) --- .../javagi/configuration/ClassNames.java | 3 +- .../generators/TypedValueGenerator.java | 6 +- .../jwharm/javagi/base/TransferOwnership.java | 41 ++++++++++++ .../main/java/org/gnome/glib/HashTable.java | 9 +-- .../src/main/java/org/gnome/glib/List.java | 63 ++++++++++--------- .../src/main/java/org/gnome/glib/SList.java | 63 ++++++++++--------- 6 files changed, 118 insertions(+), 67 deletions(-) create mode 100644 modules/glib/src/main/java/io/github/jwharm/javagi/base/TransferOwnership.java diff --git a/generator/src/main/java/io/github/jwharm/javagi/configuration/ClassNames.java b/generator/src/main/java/io/github/jwharm/javagi/configuration/ClassNames.java index 21e8a8a3..62977428 100644 --- a/generator/src/main/java/io/github/jwharm/javagi/configuration/ClassNames.java +++ b/generator/src/main/java/io/github/jwharm/javagi/configuration/ClassNames.java @@ -1,5 +1,5 @@ /* Java-GI - Java language bindings for GObject-Introspection-based libraries - * Copyright (C) 2022-2024 the Java-GI developers + * Copyright (C) 2022-2025 the Java-GI developers * * SPDX-License-Identifier: LGPL-2.1-or-later * @@ -43,6 +43,7 @@ public final class ClassNames { public static final ClassName OUT = get(PKG_BASE, "Out"); public static final ClassName PROXY = get(PKG_BASE, "Proxy"); public static final ClassName PROXY_INSTANCE = get(PKG_BASE, "ProxyInstance"); + public static final ClassName TRANSFER_OWNERSHIP = get(PKG_BASE, "TransferOwnership"); public static final ClassName UNSUPPORTED_PLATFORM_EXCEPTION = get(PKG_BASE, "UnsupportedPlatformException"); public static final ClassName ARENAS = get(PKG_INTEROP, "Arenas"); diff --git a/generator/src/main/java/io/github/jwharm/javagi/generators/TypedValueGenerator.java b/generator/src/main/java/io/github/jwharm/javagi/generators/TypedValueGenerator.java index 69bc489c..61a42f53 100644 --- a/generator/src/main/java/io/github/jwharm/javagi/generators/TypedValueGenerator.java +++ b/generator/src/main/java/io/github/jwharm/javagi/generators/TypedValueGenerator.java @@ -1,5 +1,5 @@ /* Java-GI - Java language bindings for GObject-Introspection-based libraries - * Copyright (C) 2022-2024 the Java-GI developers + * Copyright (C) 2022-2025 the Java-GI developers * * SPDX-License-Identifier: LGPL-2.1-or-later * @@ -341,7 +341,9 @@ PartialStatement marshalNativeToJava(Type type, String identifier, boolean upcal if (elementDestructor != null) stmt.add(", ").add(elementDestructor); - return stmt.add(", " + (transferOwnership == FULL)).add(")"); + return stmt.add(", $transferOwnership:T." + transferOwnership.toString(), + "transferOwnership", ClassNames.TRANSFER_OWNERSHIP) + .add(")"); } // Generate constructor call for HashTable with generic types for keys and values diff --git a/modules/glib/src/main/java/io/github/jwharm/javagi/base/TransferOwnership.java b/modules/glib/src/main/java/io/github/jwharm/javagi/base/TransferOwnership.java new file mode 100644 index 00000000..a2f0dbbb --- /dev/null +++ b/modules/glib/src/main/java/io/github/jwharm/javagi/base/TransferOwnership.java @@ -0,0 +1,41 @@ +/* Java-GI - Java language bindings for GObject-Introspection-based libraries + * Copyright (C) 2022-2024 Jan-Willem Harmannij + * + * SPDX-License-Identifier: LGPL-2.1-or-later + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, see . + */ + +package io.github.jwharm.javagi.base; + +/** + * Types of ownership transfer used by GObject-Introspection for many elements, + * for example, a returned value. + */ +public enum TransferOwnership { + /** + * The recipient does not own the value + */ + NONE, + + /** + * The recipient owns the container but not the values + */ + CONTAINER, + + /** + * The recipient owns the entire value + */ + FULL +} diff --git a/modules/glib/src/main/java/org/gnome/glib/HashTable.java b/modules/glib/src/main/java/org/gnome/glib/HashTable.java index f6e44f60..e61b4678 100644 --- a/modules/glib/src/main/java/org/gnome/glib/HashTable.java +++ b/modules/glib/src/main/java/org/gnome/glib/HashTable.java @@ -1,5 +1,5 @@ /* Java-GI - Java language bindings for GObject-Introspection-based libraries - * Copyright (C) 2022-2024 Jan-Willem Harmannij + * Copyright (C) 2022-2025 Jan-Willem Harmannij * * SPDX-License-Identifier: LGPL-2.1-or-later * @@ -20,6 +20,7 @@ package org.gnome.glib; import io.github.jwharm.javagi.base.Out; +import io.github.jwharm.javagi.base.TransferOwnership; import io.github.jwharm.javagi.base.Proxy; import io.github.jwharm.javagi.interop.Interop; import io.github.jwharm.javagi.interop.MemoryCleaner; @@ -403,7 +404,7 @@ public List getKeys() { } catch (Throwable _err) { throw new AssertionError(_err); } - return new List<>(_result, makeKey, null, false); + return new List<>(_result, makeKey, null, TransferOwnership.CONTAINER); } /** @@ -441,7 +442,7 @@ public MemorySegment[] getKeysAsArray() { throw new AssertionError(_err); } length.set(_lengthPointer.get(ValueLayout.JAVA_INT, 0)); - return Interop.getAddressArrayFrom(_result, length.get().intValue(), true); + return Interop.getAddressArrayFrom(_result, length.get(), true); } } @@ -489,7 +490,7 @@ public List getValues() { } catch (Throwable _err) { throw new AssertionError(_err); } - return new List<>(_result, makeValue, null, false); + return new List<>(_result, makeValue, null, TransferOwnership.CONTAINER); } /** diff --git a/modules/glib/src/main/java/org/gnome/glib/List.java b/modules/glib/src/main/java/org/gnome/glib/List.java index d29e3557..f91a9c32 100644 --- a/modules/glib/src/main/java/org/gnome/glib/List.java +++ b/modules/glib/src/main/java/org/gnome/glib/List.java @@ -1,5 +1,5 @@ /* Java-GI - Java language bindings for GObject-Introspection-based libraries - * Copyright (C) 2022-2024 Jan-Willem Harmannij + * Copyright (C) 2022-2025 Jan-Willem Harmannij * * SPDX-License-Identifier: LGPL-2.1-or-later * @@ -19,6 +19,7 @@ package org.gnome.glib; +import io.github.jwharm.javagi.base.TransferOwnership; import io.github.jwharm.javagi.base.Proxy; import io.github.jwharm.javagi.base.ProxyInstance; import io.github.jwharm.javagi.interop.Interop; @@ -66,60 +67,62 @@ public class List extends AbstractSequentialList implements Proxy { // operations on an List can change/remove the head private ListNode head; - // Ownership is "container" (memory of item is not managed) or "full" - private final boolean fullOwnership; + // Install a cleaner when ownership is "container" (memory of item is not + // managed) or "full" + private final TransferOwnership ownership; /** * Create a new {@code GLib.List} wrapper. * - * @param address the memory address of the head element of the List - * @param make a function to construct element instances - * @param free a function to free element instances. If - * {@code fullOwnership} is {@code false}, this can - * safely be set to {@code null}. - * @param fullOwnership whether to free element instances automatically + * @param address the memory address of the head element of the List + * @param make a function to construct element instances + * @param free a function to free element instances. If + * {@code ownership} is "none" or "container", this can + * safely be set to {@code null}. + * @param ownership whether to free memory automatically */ public List(MemorySegment address, Function make, Consumer free, - boolean fullOwnership) { + TransferOwnership ownership) { this.head = MemorySegment.NULL.equals(address) ? null : new ListNode(address); this.make = make; this.free = free; - this.fullOwnership = fullOwnership; + this.ownership = ownership; - var finalizer = new List.Finalizer<>(address, make, free, fullOwnership); - CLEANER.register(this, finalizer); + if (ownership != TransferOwnership.NONE) { + var finalizer = new List.Finalizer<>(address, make, free, ownership); + CLEANER.register(this, finalizer); + } } /** * Create a wrapper for a new, empty {@code GLib.List}. * - * @param make a function to construct element instances - * @param free a function to free element instances. If - * {@code fullOwnership} is {@code false}, this can - * safely be set to {@code null}. - * @param fullOwnership whether to free element instances automatically + * @param make a function to construct element instances + * @param free a function to free element instances. If + * {@code ownership} is "none" or "container", this can + * safely be set to {@code null}. + * @param ownership whether to free memory automatically */ public List(Function make, Consumer free, - boolean fullOwnership) { - this(null, make, free, fullOwnership); + TransferOwnership ownership) { + this(null, make, free, ownership); } /** * Create a new {@code GLib.List} wrapper. * - * @param address the memory address of the head element of the List - * @param make a function to construct element instances - * @param fullOwnership whether to free element instances automatically - * with {@link GLib#free} + * @param address the memory address of the head element of the List + * @param make a function to construct element instances + * @param ownership whether to free memory automatically */ public List(MemorySegment address, Function make, - boolean fullOwnership) { - this(address, make, null, fullOwnership); + TransferOwnership ownership) { + this(address, make, null, ownership); } /** @@ -205,7 +208,7 @@ public void remove() { head = ListNode.deleteLink(head, node); var data = node.readData(); - if (fullOwnership && data != null) { + if (ownership == TransferOwnership.FULL && data != null) { if (free == null) GLib.free(data); else @@ -219,7 +222,7 @@ public void set(E e) { throw new IllegalStateException(); var data = last.readData(); - if (fullOwnership && data != null) { + if (ownership == TransferOwnership.FULL && data != null) { if (free == null) GLib.free(data); else @@ -456,7 +459,7 @@ static void free(ListNode list) { private record Finalizer(MemorySegment address, Function make, Consumer free, - boolean fullOwnership) implements Runnable { + TransferOwnership ownership) implements Runnable { public void run() { if (address == null || MemorySegment.NULL.equals(address)) return; @@ -464,7 +467,7 @@ public void run() { // The calls to GLib.free() and ListNode.free() must run on the // main thread, not in the Cleaner thread. SourceFunc action = () -> { - if (fullOwnership) { + if (ownership == TransferOwnership.FULL) { var node = new ListNode(address); do { if (free == null) diff --git a/modules/glib/src/main/java/org/gnome/glib/SList.java b/modules/glib/src/main/java/org/gnome/glib/SList.java index a3c31861..58cdc738 100644 --- a/modules/glib/src/main/java/org/gnome/glib/SList.java +++ b/modules/glib/src/main/java/org/gnome/glib/SList.java @@ -1,5 +1,5 @@ /* Java-GI - Java language bindings for GObject-Introspection-based libraries - * Copyright (C) 2022-2024 Jan-Willem Harmannij + * Copyright (C) 2022-2025 Jan-Willem Harmannij * * SPDX-License-Identifier: LGPL-2.1-or-later * @@ -19,6 +19,7 @@ package org.gnome.glib; +import io.github.jwharm.javagi.base.TransferOwnership; import io.github.jwharm.javagi.base.Proxy; import io.github.jwharm.javagi.base.ProxyInstance; import io.github.jwharm.javagi.interop.Interop; @@ -69,60 +70,62 @@ public class SList extends AbstractSequentialList implements Proxy { // operations on an SList can change/remove the head private SListNode head; - // Ownership is "container" (memory of item is not managed) or "full" - private final boolean fullOwnership; + // Install a cleaner when ownership is "container" (memory of item is not + // managed) or "full" + private final TransferOwnership ownership; /** * Create a new {@code GLib.SList} wrapper. * - * @param address the memory address of the head element of the SList - * @param make a function to construct element instances - * @param free a function to free element instances. If - * {@code fullOwnership} is {@code false}, this can - * safely be set to {@code null}. - * @param fullOwnership whether to free element instances automatically + * @param address the memory address of the head element of the SList + * @param make a function to construct element instances + * @param free a function to free element instances. If + * {@code ownership} is "none" or "container", this can + * safely be set to {@code null}. + * @param ownership whether to free memory automatically */ public SList(MemorySegment address, Function make, Consumer free, - boolean fullOwnership) { + TransferOwnership ownership) { this.head = MemorySegment.NULL.equals(address) ? null : new SListNode(address); this.make = make; this.free = free; - this.fullOwnership = fullOwnership; + this.ownership = ownership; - var finalizer = new Finalizer<>(address, make, free, fullOwnership); - CLEANER.register(this, finalizer); + if (ownership != TransferOwnership.NONE) { + var finalizer = new Finalizer<>(address, make, free, ownership); + CLEANER.register(this, finalizer); + } } /** * Create a wrapper for a new, empty {@code GLib.SList}. * - * @param make a function to construct element instances - * @param free a function to free element instances. If - * {@code fullOwnership} is {@code false}, this can - * safely be set to {@code null}. - * @param fullOwnership whether to free element instances automatically + * @param make a function to construct element instances + * @param free a function to free element instances. If + * {@code ownership} is "none" or "container", this can + * safely be set to {@code null}. + * @param ownership whether to free memory automatically */ public SList(Function make, Consumer free, - boolean fullOwnership) { - this(null, make, free, fullOwnership); + TransferOwnership ownership) { + this(null, make, free, ownership); } /** * Create a new {@code GLib.SList} wrapper. * - * @param address the memory address of the head element of the SList - * @param make a function to construct element instances - * @param fullOwnership whether to free element instances automatically - * with {@link GLib#free} + * @param address the memory address of the head element of the SList + * @param make a function to construct element instances + * @param ownership whether to free memory automatically */ public SList(MemorySegment address, Function make, - boolean fullOwnership) { - this(address, make, null, fullOwnership); + TransferOwnership ownership) { + this(address, make, null, ownership); } /** @@ -217,7 +220,7 @@ public void remove() { } index--; - if (fullOwnership && data != null) { + if (ownership == TransferOwnership.FULL && data != null) { if (free == null) GLib.free(data); else @@ -231,7 +234,7 @@ public void set(E e) { throw new IllegalStateException(); var data = last.readData(); - if (fullOwnership && data != null) { + if (ownership == TransferOwnership.FULL && data != null) { if (free == null) GLib.free(data); else @@ -459,7 +462,7 @@ static void free(SListNode list) { private record Finalizer(MemorySegment address, Function make, Consumer free, - boolean fullOwnership) implements Runnable { + TransferOwnership ownership) implements Runnable { public void run() { if (address == null || MemorySegment.NULL.equals(address)) return; @@ -467,7 +470,7 @@ public void run() { // The calls to GLib.free() and SListNode.free() must run on the // main thread, not in the Cleaner thread. SourceFunc action = () -> { - if (fullOwnership) { + if (ownership == TransferOwnership.FULL) { var node = new SListNode(address); do { if (free == null)