TestScopedOperations does not work as expected

Reviewed-by: jvernee
openjdk · Sep 30, 2021 · 4511190 · 4511190
1 parent ab61f06
commit 4511190
Show file tree

Hide file tree

Showing 5 changed files with 42 additions and 30 deletions.
diff --git a/...ator.foreign/share/classes/jdk/internal/foreign/abi/aarch64/linux/LinuxAArch64VaList.java b/...ator.foreign/share/classes/jdk/internal/foreign/abi/aarch64/linux/LinuxAArch64VaList.java
@@ -26,6 +26,7 @@
 package jdk.internal.foreign.abi.aarch64.linux;
 
 import jdk.incubator.foreign.*;
+import jdk.internal.foreign.ResourceScopeImpl;
 import jdk.internal.foreign.Scoped;
 import jdk.internal.foreign.Utils;
 import jdk.internal.foreign.abi.SharedUtils;
@@ -323,6 +324,7 @@ private Object read(Class<?> carrier, MemoryLayout layout, SegmentAllocator allo
     @Override
     public void skip(MemoryLayout... layouts) {
         Objects.requireNonNull(layouts);
+        ((ResourceScopeImpl)segment.scope()).checkValidStateSlow();
         for (MemoryLayout layout : layouts) {
             Objects.requireNonNull(layout);
             TypeClass typeClass = TypeClass.classifyLayout(layout);

diff --git a/...ator.foreign/share/classes/jdk/internal/foreign/abi/aarch64/macos/MacOsAArch64VaList.java b/...ator.foreign/share/classes/jdk/internal/foreign/abi/aarch64/macos/MacOsAArch64VaList.java
@@ -128,6 +128,7 @@ private Object read(Class<?> carrier, MemoryLayout layout, SegmentAllocator allo
     @Override
     public void skip(MemoryLayout... layouts) {
         Objects.requireNonNull(layouts);
+        ((ResourceScopeImpl)scope).checkValidStateSlow();
 
         for (MemoryLayout layout : layouts) {
             Objects.requireNonNull(layout);

diff --git a/src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/abi/x64/sysv/SysVVaList.java b/src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/abi/x64/sysv/SysVVaList.java
@@ -26,6 +26,7 @@
 package jdk.internal.foreign.abi.x64.sysv;
 
 import jdk.incubator.foreign.*;
+import jdk.internal.foreign.ResourceScopeImpl;
 import jdk.internal.foreign.Scoped;
 import jdk.internal.foreign.Utils;
 import jdk.internal.foreign.abi.SharedUtils;
@@ -279,6 +280,7 @@ private Object read(Class<?> carrier, MemoryLayout layout, SegmentAllocator allo
     @Override
     public void skip(MemoryLayout... layouts) {
         Objects.requireNonNull(layouts);
+        ((ResourceScopeImpl)segment.scope()).checkValidStateSlow();
         for (MemoryLayout layout : layouts) {
             Objects.requireNonNull(layout);
             TypeClass typeClass = TypeClass.classifyLayout(layout);

diff --git a/src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/abi/x64/windows/WinVaList.java b/src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/abi/x64/windows/WinVaList.java
@@ -133,6 +133,7 @@ private Object read(Class<?> carrier, MemoryLayout layout, SegmentAllocator allo
     @Override
     public void skip(MemoryLayout... layouts) {
         Objects.requireNonNull(layouts);
+        ((ResourceScopeImpl)scope).checkValidStateSlow();
         Stream.of(layouts).forEach(Objects::requireNonNull);
         segment = segment.asSlice(layouts.length * VA_SLOT_SIZE_BYTES);
     }

diff --git a/test/jdk/java/foreign/TestScopedOperations.java b/test/jdk/java/foreign/TestScopedOperations.java
@@ -44,10 +44,14 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.BiConsumer;
 import java.util.function.Consumer;
 import java.util.function.Function;
+import java.util.function.Supplier;
 
 import static jdk.incubator.foreign.ValueLayout.JAVA_BYTE;
+import static jdk.incubator.foreign.ValueLayout.JAVA_INT;
+import static jdk.incubator.foreign.ValueLayout.JAVA_LONG;
 import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertNotNull;
 import static org.testng.Assert.assertTrue;
@@ -68,24 +72,26 @@ public class TestScopedOperations {
     }
 
     @Test(dataProvider = "scopedOperations")
-    public void testOpAfterClose(String name, ScopedOperation scopedOperation) {
+    public <Z> void testOpAfterClose(String name, ScopedOperation<Z> scopedOperation) {
         ResourceScope scope = ResourceScope.newConfinedScope();
+        Z obj = scopedOperation.apply(scope);
         scope.close();
         try {
-            scopedOperation.accept(scope);
+            scopedOperation.accept(obj);
             fail();
         } catch (IllegalStateException ex) {
             assertTrue(ex.getMessage().contains("closed"));
         }
     }
 
     @Test(dataProvider = "scopedOperations")
-    public void testOpOutsideConfinement(String name, ScopedOperation scopedOperation) {
+    public <Z> void testOpOutsideConfinement(String name, ScopedOperation<Z> scopedOperation) {
         try (ResourceScope scope = ResourceScope.newConfinedScope()) {
+            Z obj = scopedOperation.apply(scope);
             AtomicReference<Throwable> failed = new AtomicReference<>();
             Thread t = new Thread(() -> {
                 try {
-                    scopedOperation.accept(scope);
+                    scopedOperation.accept(obj);
                 } catch (Throwable ex) {
                     failed.set(ex);
                 }
@@ -119,13 +125,12 @@ public void testOpOutsideConfinement(String name, ScopedOperation scopedOperatio
                 fail();
             }
         }, "MemorySegment::mapFromFile");
-        ScopedOperation.ofScope(scope -> VaList.make(b -> {}, scope), "VaList::make");
+        ScopedOperation.ofScope(scope -> VaList.make(b -> b.addVarg(JAVA_INT, 42), scope), "VaList::make");
         ScopedOperation.ofScope(scope -> VaList.ofAddress(MemoryAddress.ofLong(42), scope), "VaList::make");
         ScopedOperation.ofScope(SegmentAllocator::arenaUnbounded, "SegmentAllocator::arenaAllocator");
         // segment operations
         ScopedOperation.ofSegment(s -> s.toArray(JAVA_BYTE), "MemorySegment::toArray(BYTE)");
         ScopedOperation.ofSegment(MemorySegment::address, "MemorySegment::address");
-        ScopedOperation.ofSegment(s -> MemoryLayout.sequenceLayout(s.byteSize(), JAVA_BYTE), "MemorySegment::spliterator");
         ScopedOperation.ofSegment(s -> s.copyFrom(s), "MemorySegment::copyFrom");
         ScopedOperation.ofSegment(s -> s.mismatch(s), "MemorySegment::mismatch");
         ScopedOperation.ofSegment(s -> s.fill((byte) 0), "MemorySegment::fill");
@@ -164,47 +169,52 @@ static Object[][] scopedOperations() {
         return scopedOperations.stream().map(op -> new Object[] { op.name, op }).toArray(Object[][]::new);
     }
 
-    static class ScopedOperation implements Consumer<ResourceScope> {
+    static class ScopedOperation<X> implements Consumer<X>, Function<ResourceScope, X> {
 
-        final Consumer<ResourceScope> scopeConsumer;
+        final Function<ResourceScope, X> factory;
+        final Consumer<X> operation;
         final String name;
 
-        private ScopedOperation(Consumer<ResourceScope> scopeConsumer, String name) {
-            this.scopeConsumer = scopeConsumer;
+        private ScopedOperation(Function<ResourceScope, X> factory, Consumer<X> operation, String name) {
+            this.factory = factory;
+            this.operation = operation;
             this.name = name;
         }
 
         @Override
-        public void accept(ResourceScope scope) {
-            scopeConsumer.accept(scope);
+        public void accept(X obj) {
+            operation.accept(obj);
+        }
+
+        @Override
+        public X apply(ResourceScope scope) {
+            return factory.apply(scope);
         }
 
         static void ofScope(Consumer<ResourceScope> scopeConsumer, String name) {
-            scopedOperations.add(new ScopedOperation(scopeConsumer::accept, name));
+            scopedOperations.add(new ScopedOperation<>(Function.identity(), scopeConsumer, name));
         }
 
         static void ofVaList(Consumer<VaList> vaListConsumer, String name) {
-            scopedOperations.add(new ScopedOperation(scope -> {
-                VaList vaList = VaList.make((builder) -> {}, scope);
-                vaListConsumer.accept(vaList);
-            }, name));
+            scopedOperations.add(new ScopedOperation<>(scope -> VaList.make(builder -> builder.addVarg(JAVA_LONG, 42), scope),
+                    vaListConsumer, name));
         }
 
         static void ofSegment(Consumer<MemorySegment> segmentConsumer, String name) {
             for (SegmentFactory segmentFactory : SegmentFactory.values()) {
-                scopedOperations.add(new ScopedOperation(scope -> {
-                    MemorySegment segment = segmentFactory.segmentFactory.apply(scope);
-                    segmentConsumer.accept(segment);
-                }, segmentFactory.name() + "/" + name));
+                scopedOperations.add(new ScopedOperation<>(
+                        segmentFactory.segmentFactory,
+                        segmentConsumer,
+                        segmentFactory.name() + "/" + name));
             }
         }
 
         static void ofAllocator(Consumer<SegmentAllocator> allocatorConsumer, String name) {
             for (AllocatorFactory allocatorFactory : AllocatorFactory.values()) {
-                scopedOperations.add(new ScopedOperation(scope -> {
-                    SegmentAllocator allocator = allocatorFactory.allocatorFactory.apply(scope);
-                    allocatorConsumer.accept(allocator);
-                }, allocatorFactory.name() + "/" + name));
+                scopedOperations.add(new ScopedOperation<>(
+                        allocatorFactory.allocatorFactory,
+                        allocatorConsumer,
+                        allocatorFactory.name() + "/" + name));
             }
         }
 
@@ -239,11 +249,7 @@ enum SegmentFactory {
 
         enum AllocatorFactory {
             ARENA_BOUNDED(scope -> SegmentAllocator.arenaBounded(1000, scope)),
-            ARENA_UNBOUNDED(SegmentAllocator::arenaUnbounded),
-            FROM_SEGMENT(scope -> {
-                MemorySegment segment = MemorySegment.allocateNative(10, scope);
-                return SegmentAllocator.prefixAllocator(segment);
-            });
+            ARENA_UNBOUNDED(SegmentAllocator::arenaUnbounded);
 
             final Function<ResourceScope, SegmentAllocator> allocatorFactory;