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..8143213 --- /dev/null +++ b/core/src/integrationTest/java/app/photofox/vipsffm/VBlobArenaTest.java @@ -0,0 +1,138 @@ +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; +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); + } + + @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()); + 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/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/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 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( 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);