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

Do not free unowned GList/GSlist (fixes #183) #190

Merged
merged 1 commit into from
Jan 13, 2025
+118 −67
Merged

Do not free unowned GList/GSlist (fixes #183) #190

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion generator/src/main/java/io/github/jwharm/javagi/configuration/ClassNames.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
*
Expand Down Expand Up @@ -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");
Expand Down
6 changes: 4 additions & 2 deletions generator/src/main/java/io/github/jwharm/javagi/generators/TypedValueGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
*
Expand Down Expand Up @@ -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
Expand Down
41 changes: 41 additions & 0 deletions modules/glib/src/main/java/io/github/jwharm/javagi/base/TransferOwnership.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 <http://www.gnu.org/licenses/>.
*/

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
}
9 changes: 5 additions & 4 deletions modules/glib/src/main/java/org/gnome/glib/HashTable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
*
Expand All @@ -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;
Expand Down Expand Up @@ -403,7 +404,7 @@ public List<K> getKeys() {
} catch (Throwable _err) {
throw new AssertionError(_err);
}
return new List<>(_result, makeKey, null, false);
return new List<>(_result, makeKey, null, TransferOwnership.CONTAINER);
}

/**
Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -489,7 +490,7 @@ public List<V> getValues() {
} catch (Throwable _err) {
throw new AssertionError(_err);
}
return new List<>(_result, makeValue, null, false);
return new List<>(_result, makeValue, null, TransferOwnership.CONTAINER);
}

/**
Expand Down
63 changes: 33 additions & 30 deletions modules/glib/src/main/java/org/gnome/glib/List.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
*
Expand All @@ -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;
Expand Down Expand Up @@ -66,60 +67,62 @@ public class List<E> extends AbstractSequentialList<E> 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<MemorySegment, E> make,
Consumer<E> 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<MemorySegment, E> make,
Consumer<E> 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<MemorySegment, E> make,
boolean fullOwnership) {
this(address, make, null, fullOwnership);
TransferOwnership ownership) {
this(address, make, null, ownership);
}

/**
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -456,15 +459,15 @@ static void free(ListNode list) {
private record Finalizer<E>(MemorySegment address,
Function<MemorySegment, E> make,
Consumer<E> free,
boolean fullOwnership) implements Runnable {
TransferOwnership ownership) implements Runnable {
public void run() {
if (address == null || MemorySegment.NULL.equals(address))
return;

// 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)
Expand Down
63 changes: 33 additions & 30 deletions modules/glib/src/main/java/org/gnome/glib/SList.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
*
Expand All @@ -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;
Expand Down Expand Up @@ -69,60 +70,62 @@ public class SList<E> extends AbstractSequentialList<E> 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<MemorySegment, E> make,
Consumer<E> 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<MemorySegment, E> make,
Consumer<E> 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<MemorySegment, E> make,
boolean fullOwnership) {
this(address, make, null, fullOwnership);
TransferOwnership ownership) {
this(address, make, null, ownership);
}

/**
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -459,15 +462,15 @@ static void free(SListNode list) {
private record Finalizer<E>(MemorySegment address,
Function<MemorySegment, E> make,
Consumer<E> free,
boolean fullOwnership) implements Runnable {
TransferOwnership ownership) implements Runnable {
public void run() {
if (address == null || MemorySegment.NULL.equals(address))
return;

// 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)
Expand Down