Skip to content

Commit

Permalink
Do not free unowned GList/GSlist (fixes #183)
Browse files Browse the repository at this point in the history
  • Loading branch information
@jwharm
jwharm committed Jan 13, 2025
1 parent 88fea8b commit bf72e20
Show file tree
Hide file tree
Showing 6 changed files with 118 additions and 67 deletions.
3 changes: 2 additions & 1 deletion generator/src/main/java/io/github/jwharm/javagi/configuration/ClassNames.java
View file
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
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
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
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
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
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

0 comments on commit bf72e20

Please sign in to comment.