From 3770010473d5e759c2698e049ef14cfb7132873e Mon Sep 17 00:00:00 2001 From: jinhyeong jang Date: Thu, 10 Sep 2026 14:59:11 +0900 Subject: [PATCH 1/3] fix: bind VBlob data views to their arena --- .../app/photofox/vipsffm/VBlobArenaTest.java | 97 +++++++++++++++++++ .../main/java/app/photofox/vipsffm/VBlob.java | 6 +- .../vipsffm/sample/VBlobByteBufferSample.java | 4 + 3 files changed, 105 insertions(+), 2 deletions(-) create mode 100644 core/src/integrationTest/java/app/photofox/vipsffm/VBlobArenaTest.java diff --git a/core/src/integrationTest/java/app/photofox/vipsffm/VBlobArenaTest.java b/core/src/integrationTest/java/app/photofox/vipsffm/VBlobArenaTest.java new file mode 100644 index 0000000..1c3d1b9 --- /dev/null +++ b/core/src/integrationTest/java/app/photofox/vipsffm/VBlobArenaTest.java @@ -0,0 +1,97 @@ +package app.photofox.vipsffm; + +import java.lang.foreign.Arena; +import java.lang.foreign.MemorySegment; +import java.nio.ByteBuffer; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; + +class VBlobArenaTest { + + @Test + void scopedBufferExpiresWithArena() { + ByteBuffer buffer; + try (var arena = Arena.ofConfined()) { + var blob = VBlob.newFromBytes(arena, new byte[]{1, 2, 3}); + buffer = blob.asArenaScopedByteBuffer(); + assertTrue(buffer.isDirect()); + assertEquals(3, buffer.remaining()); + assertEquals(1, buffer.get(0)); + } + assertExpired(buffer); + } + + @Test + void dataSegmentUsesBlobArena() { + try (var arena = Arena.ofConfined()) { + var blob = VBlob.newFromBytes(arena, new byte[]{1, 2, 3}); + var data = blob.getUnsafeDataAddress(); + assertEquals(3, data.byteSize()); + assertEquals(arena.scope(), data.scope()); + } + } + + @Test + void confinedBufferRetainsThreadConfinement() { + try (var arena = Arena.ofConfined()) { + var buffer = VBlob.newFromBytes(arena, new byte[]{1}).asArenaScopedByteBuffer(); + var otherThread = new Thread(() -> {}); + assertFalse(MemorySegment.ofBuffer(buffer).isAccessibleBy(otherThread)); + } + } + + @Test + void sharedBufferCanBeReadByAnotherThreadUntilArenaCloses() throws InterruptedException { + ByteBuffer buffer; + try (var arena = Arena.ofShared()) { + buffer = VBlob.newFromBytes(arena, new byte[]{42}).asArenaScopedByteBuffer(); + var value = new AtomicInteger(); + var failure = new AtomicReference(); + var thread = new Thread(() -> { + try { + value.set(buffer.get(0)); + } catch (Throwable error) { + failure.set(error); + } + }); + thread.start(); + thread.join(); + assertNull(failure.get()); + assertEquals(42, value.get()); + } + assertExpired(buffer); + } + + @Test + void copiesRemainUsableAfterArenaCloses() { + byte[] bytes; + ByteBuffer clone; + try (var arena = Arena.ofConfined()) { + var blob = VBlob.newFromBytes(arena, new byte[]{1, 2, 3}); + bytes = blob.getBytes(); + clone = blob.asClonedByteBuffer(); + } + assertArrayEquals(new byte[]{1, 2, 3}, bytes); + assertEquals(ByteBuffer.wrap(bytes), clone); + } + + @Test + void encodedImageBufferAlsoExpiresWithArena() { + ByteBuffer buffer; + try (var arena = Arena.ofConfined()) { + var png = VImage.black(arena, 16, 16).pngsaveBuffer(); + buffer = png.asArenaScopedByteBuffer(); + assertEquals((byte) 137, buffer.get(0)); + } + assertExpired(buffer); + } + + private static void assertExpired(ByteBuffer buffer) { + // Check metadata first: the unpatched implementation must never read freed memory. + assertFalse(MemorySegment.ofBuffer(buffer).scope().isAlive()); + assertThrows(IllegalStateException.class, () -> buffer.get(0)); + } +} diff --git a/core/src/main/java/app/photofox/vipsffm/VBlob.java b/core/src/main/java/app/photofox/vipsffm/VBlob.java index dc54382..5525180 100644 --- a/core/src/main/java/app/photofox/vipsffm/VBlob.java +++ b/core/src/main/java/app/photofox/vipsffm/VBlob.java @@ -67,7 +67,7 @@ public MemorySegment getUnsafeStructAddress() throws VipsError { /// /// Gets the raw [MemorySegment] (C pointer) for the data in this blob /// - /// Sliced to the length of the data, which isn't always null terminated + /// Sized to the length of the data, which isn't always null terminated, and scoped to this blob's arena public MemorySegment getUnsafeDataAddress() throws VipsError { var lengthOutPointer = arena.allocate(C_LONG); var dataPointer = VipsRaw.vips_blob_get( @@ -81,7 +81,9 @@ public MemorySegment getUnsafeDataAddress() throws VipsError { if (length < 0) { throw new VipsError("unexpected length of vblob data " + length); } - return dataPointer.asSlice(0, length); + // Downcall pointers have global scope; slicing alone does not bind their lifetime to the arena. + // The blob already owns cleanup, so this view must not register another deallocator. + return dataPointer.reinterpret(length, arena, null); } /// Size of the data in this blob diff --git a/sample/src/main/java/vipsffm/sample/VBlobByteBufferSample.java b/sample/src/main/java/vipsffm/sample/VBlobByteBufferSample.java index 073dd20..ddef12f 100644 --- a/sample/src/main/java/vipsffm/sample/VBlobByteBufferSample.java +++ b/sample/src/main/java/vipsffm/sample/VBlobByteBufferSample.java @@ -28,6 +28,10 @@ public void run(Arena arena, Path workingDirectory) throws Exception { var rawDataSegment = blob.getUnsafeDataAddress(); var rawByteSize = (int) blob.byteSize(); + if (!rawDataSegment.scope().equals(arena.scope())) { + throw new RuntimeException("blob data view is not scoped to its arena"); + } + var remainingBytes = bytes.remaining(); if (remainingBytes < 50000L || remainingBytes > 100000L) { throw new RuntimeException("number of bytes in buffer out of range " + remainingBytes); From 3212e9c8ce172fdcc147f0a69697a87a998cef8e Mon Sep 17 00:00:00 2001 From: sage <149851+lopcode@users.noreply.github.com> Date: Thu, 10 Sep 2026 08:30:04 +0200 Subject: [PATCH 2/3] Fix other blob return callsites --- .../app/photofox/vipsffm/VBlobArenaTest.java | 41 +++++++++++++++++++ .../java/app/photofox/vipsffm/VImage.java | 2 +- .../java/app/photofox/vipsffm/VipsHelper.java | 4 +- .../main/java/vipsffm/GenerateVClasses.java | 2 +- .../java/vipsffm/GenerateVipsHelperClass.java | 15 ++++++- 5 files changed, 58 insertions(+), 6 deletions(-) diff --git a/core/src/integrationTest/java/app/photofox/vipsffm/VBlobArenaTest.java b/core/src/integrationTest/java/app/photofox/vipsffm/VBlobArenaTest.java index 1c3d1b9..8143213 100644 --- a/core/src/integrationTest/java/app/photofox/vipsffm/VBlobArenaTest.java +++ b/core/src/integrationTest/java/app/photofox/vipsffm/VBlobArenaTest.java @@ -1,7 +1,9 @@ package app.photofox.vipsffm; +import app.photofox.vipsffm.jextract.VipsRaw; import java.lang.foreign.Arena; import java.lang.foreign.MemorySegment; +import java.lang.foreign.ValueLayout; import java.nio.ByteBuffer; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; @@ -89,6 +91,45 @@ void encodedImageBufferAlsoExpiresWithArena() { assertExpired(buffer); } + @Test + void imageMetadataBlobExpiresWithArena() { + ByteBuffer buffer; + try (var arena = Arena.ofConfined()) { + var image = VImage.black(arena, 4, 4) + .set("test-blob", VBlob.newFromBytes(arena, new byte[]{7, 8, 9})); + buffer = image.getBlob("test-blob").asArenaScopedByteBuffer(); + assertEquals(3, buffer.remaining()); + assertEquals(9, buffer.get(2)); + } + assertExpired(buffer); + } + + @Test + void helperBlobGetIsBoundToArena() { + MemorySegment data; + try (var arena = Arena.ofConfined()) { + var blob = VBlob.newFromBytes(arena, new byte[]{4, 5, 6}); + var lengthOut = arena.allocate(VipsRaw.C_LONG); + data = VipsHelper.blob_get(arena, blob.getUnsafeStructAddress(), lengthOut); + assertEquals(arena.scope(), data.scope()); + assertEquals(3, lengthOut.get(VipsRaw.C_LONG, 0)); + assertArrayEquals(new byte[]{4, 5, 6}, data.reinterpret(3).toArray(ValueLayout.JAVA_BYTE)); + } + assertFalse(data.scope().isAlive()); + } + + @Test + void helperImageGetDataIsBoundToArena() { + MemorySegment data; + try (var arena = Arena.ofConfined()) { + var image = VImage.black(arena, 2, 2); + data = VipsHelper.image_get_data(arena, image.getUnsafeStructAddress()); + assertEquals(arena.scope(), data.scope()); + assertEquals(0, data.reinterpret(4).get(ValueLayout.JAVA_BYTE, 0)); + } + assertFalse(data.scope().isAlive()); + } + private static void assertExpired(ByteBuffer buffer) { // Check metadata first: the unpatched implementation must never read freed memory. assertFalse(MemorySegment.ofBuffer(buffer).scope().isAlive()); diff --git a/core/src/main/java/app/photofox/vipsffm/VImage.java b/core/src/main/java/app/photofox/vipsffm/VImage.java index 292461f..5a96adf 100644 --- a/core/src/main/java/app/photofox/vipsffm/VImage.java +++ b/core/src/main/java/app/photofox/vipsffm/VImage.java @@ -10465,7 +10465,7 @@ public VBlob getBlob(String name) { if (blobLength <= 0) { throw new VipsError("failed to read length of type blob from field: " + name); } - var dataSegment = outPointer.get(VipsRaw.C_POINTER, 0).reinterpret(blobLength); + var dataSegment = outPointer.get(VipsRaw.C_POINTER, 0).reinterpret(blobLength, arena, null); return VBlob.newFromDataSegment(arena, dataSegment); } diff --git a/core/src/main/java/app/photofox/vipsffm/VipsHelper.java b/core/src/main/java/app/photofox/vipsffm/VipsHelper.java index 2686df9..17110ad 100644 --- a/core/src/main/java/app/photofox/vipsffm/VipsHelper.java +++ b/core/src/main/java/app/photofox/vipsffm/VipsHelper.java @@ -410,7 +410,7 @@ public static MemorySegment blob_get(Arena arena, MemorySegment blob, MemorySegm if(!VipsValidation.isValidPointer(result)) { VipsValidation.throwInvalidOutputError("vips_blob_get", "result"); } - result = result.reinterpret(arena, VipsRaw::g_object_unref); + result = result.reinterpret(arena, null); return result; } @@ -1645,7 +1645,7 @@ public static MemorySegment image_get_data(Arena arena, MemorySegment image) thr if(!VipsValidation.isValidPointer(result)) { VipsValidation.throwInvalidOutputError("vips_image_get_data", "result"); } - result = result.reinterpret(arena, VipsRaw::g_object_unref); + result = result.reinterpret(arena, null); return result; } diff --git a/generator/src/main/java/vipsffm/GenerateVClasses.java b/generator/src/main/java/vipsffm/GenerateVClasses.java index 7f83713..4f9af2b 100644 --- a/generator/src/main/java/vipsffm/GenerateVClasses.java +++ b/generator/src/main/java/vipsffm/GenerateVClasses.java @@ -1110,7 +1110,7 @@ private static List buildImageGetSetMethods() { .endControlFlow() .build() ); - getMethodBuilder.addStatement("var dataSegment = outPointer.get($T.C_POINTER, 0).reinterpret(blobLength)", vipsRawType); + getMethodBuilder.addStatement("var dataSegment = outPointer.get($T.C_POINTER, 0).reinterpret(blobLength, arena, null)", vipsRawType); getMethodBuilder.addStatement("return VBlob.newFromDataSegment(arena, dataSegment)"); } else if (poetValueType.equals(vimageType)) { // VImage ** diff --git a/generator/src/main/java/vipsffm/GenerateVipsHelperClass.java b/generator/src/main/java/vipsffm/GenerateVipsHelperClass.java index 79b166e..83f336a 100644 --- a/generator/src/main/java/vipsffm/GenerateVipsHelperClass.java +++ b/generator/src/main/java/vipsffm/GenerateVipsHelperClass.java @@ -358,10 +358,21 @@ private static boolean addDeallocCodeblockIfOutType( return true; } + var isReturnType = externType.name().isBlank() && externType.pointerDepth() == 1; + + // const data pointers are borrowed from the object that owns them, so they are bound to the arena but never freed + if (isReturnType && externType.type().equals("void") && externType.isConst()) { + methodBuilder.addCode( + CodeBlock.builder() + .addStatement(name + " = " + name + ".reinterpret(arena, null)") + .build() + ); + return true; + } + // newly allocated return types have a depth of 1 var isNewReturnAlloc = - (externType.name().isBlank() && - externType.pointerDepth() == 1 && !externType.type().equals("char") && !externType.raw().equals("void *")); + (isReturnType && !externType.type().equals("char") && !externType.type().equals("void")); if (isNewReturnAlloc) { methodBuilder.addCode( From a0d57417c3ac170e06d9e2876ea24932f7b0fa15 Mon Sep 17 00:00:00 2001 From: sage <149851+lopcode@users.noreply.github.com> Date: Thu, 10 Sep 2026 08:37:57 +0200 Subject: [PATCH 3/3] Regenerate docs --- docs/app.photofox.vipsffm/app/photofox/vipsffm/VBlob.html | 2 +- docs/index.html | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/app.photofox.vipsffm/app/photofox/vipsffm/VBlob.html b/docs/app.photofox.vipsffm/app/photofox/vipsffm/VBlob.html index 9a341d6..acd7a23 100644 --- a/docs/app.photofox.vipsffm/app/photofox/vipsffm/VBlob.html +++ b/docs/app.photofox.vipsffm/app/photofox/vipsffm/VBlob.html @@ -208,7 +208,7 @@

getUnsafeDataAddress

throws VipsError

Not recommended for use, use asArenaScopedByteBuffer() or asClonedByteBuffer() instead

Gets the raw MemorySegment (C pointer) for the data in this blob

-

Sliced to the length of the data, which isn't always null terminated

+

Sized to the length of the data, which isn't always null terminated, and scoped to this blob's arena

Throws:
diff --git a/docs/index.html b/docs/index.html index 8951746..b5317a5 100644 --- a/docs/index.html +++ b/docs/index.html @@ -70,7 +70,7 @@

Usage

} dependencies { - implementation("app.photofox.vips-ffm:vips-ffm-core:1.9.8") + implementation("app.photofox.vips-ffm:vips-ffm-core:1.9.9") }

Figure out what you're trying to do by looking at the libvips documentation