8237351: Add method to SystemABI for explicitly freeing upcall stubs

Reviewed-by: mcimadamore

src/hotspot/share/prims/upcallStubs.cpp

@@ -25,17 +25,20 @@
 #include "runtime/jniHandles.inline.hpp"
 #include "runtime/interfaceSupport.inline.hpp"
 
-JVM_ENTRY(static void, UH_FreeUpcallStub(JNIEnv *env, jobject _unused, jlong addr))
+JVM_ENTRY(static jboolean, UH_FreeUpcallStub(JNIEnv *env, jobject _unused, jlong addr))
   //acquire code cache lock
   MutexLocker mu(CodeCache_lock, Mutex::_no_safepoint_check_flag);
   //find code blob
   CodeBlob* cb = CodeCache::find_blob((char*)addr);
-  assert(cb != NULL, "Attempting to free non-existent stub");
+  if (cb == NULL) {
+    return false;
+  }
   //free global JNI handle
   jobject* rec_ptr = (jobject*)(void*)cb -> content_begin();
   JNIHandles::destroy_global(*rec_ptr);
   //free code blob
   CodeCache::free(cb);
+  return true;
 JVM_END
 
 #define CC (char*)  /*cast a literal from (const char*)*/
@@ -44,7 +47,7 @@ JVM_END
 
 // These are the native methods on jdk.internal.foreign.NativeInvoker.
 static JNINativeMethod UH_methods[] = {
-  {CC "freeUpcallStub",     CC "(J)V",                FN_PTR(UH_FreeUpcallStub)}
+  {CC "freeUpcallStub",     CC "(J)Z",                FN_PTR(UH_FreeUpcallStub)}
 };
 
 /**

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/SystemABI.java

@@ -25,6 +25,7 @@
  */
 package jdk.incubator.foreign;
 
+import jdk.internal.foreign.abi.UpcallStubs;
 import jdk.internal.foreign.abi.aarch64.AArch64ABI;
 import jdk.internal.foreign.abi.x64.sysv.SysVx64ABI;
 import jdk.internal.foreign.abi.x64.windows.Windowsx64ABI;
@@ -58,6 +59,17 @@ public interface SystemABI {
      */
     MemoryAddress upcallStub(MethodHandle target, FunctionDescriptor function);
 
+    /**
+     * Frees an upcall stub given it's memory address.
+     *
+     * @param address the memory address of the upcall stub, returned from
+     *                {@link SystemABI#upcallStub(MethodHandle, FunctionDescriptor)}.
+     * @throws IllegalArgumentException if the given address is not a valid upcall stub address.
+     */
+    default void freeUpcallStub(MemoryAddress address) {
+        UpcallStubs.freeUpcallStub(address);
+    }
+
     /**
      * Obtain an instance of the system ABI.
      * @return system ABI.

src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/abi/UpcallStubs.java

@@ -25,19 +25,28 @@
 package jdk.internal.foreign.abi;
 
 import jdk.incubator.foreign.MemoryAddress;
+import jdk.internal.foreign.MemoryAddressImpl;
 import jdk.internal.foreign.MemoryScope;
 import jdk.internal.foreign.MemorySegmentImpl;
 
 public class UpcallStubs {
 
     public static MemoryAddress upcallAddress(UpcallHandler handler) {
         long addr = handler.entryPoint();
-        return new MemorySegmentImpl(addr, null, 8, 0, Thread.currentThread(),
-                new MemoryScope(null, () -> UpcallStubs.freeUpcallStub(addr))).baseAddress();
+        return MemoryAddress.ofLong(addr);
+    }
+
+    public static void freeUpcallStub(MemoryAddress address) {
+        MemoryAddressImpl ma = (MemoryAddressImpl) address;
+        if (ma.unsafeGetBase() != null || !freeUpcallStub(ma.unsafeGetOffset())) {
+            throw new IllegalArgumentException("Not a stub address: " + address);
+        }
     }
 
     // natives
-    public static native void freeUpcallStub(long addr);
+
+    // returns true if the stub was found (and freed)
+    private static native boolean freeUpcallStub(long addr);
 
     private static native void registerNatives();
     static {

test/jdk/java/foreign/CallGeneratorHelper.java

@@ -333,7 +333,12 @@ static void cleanup(Object arg) {
         if (arg instanceof MemoryAddress) {
             cleanup(((MemoryAddress)arg).segment());
         } else if (arg instanceof MemorySegment) {
-            ((MemorySegment)arg).close();
+            try {
+                ((MemorySegment) arg).close();
+            } catch (IllegalStateException e) {
+                assertEquals(e.getMessage(), "Cannot close a root segment");
+                // fine, NOTHING segment for upcall stubs
+            }
         }
     }
 

test/jdk/java/foreign/StdLibTest.java

@@ -316,6 +316,7 @@ int[] qsort(int[] arr) throws Throwable {
                 //call qsort
                 MemoryAddress qsortUpcallAddr = abi.upcallStub(qsortCompar.bindTo(nativeArr), qsortComparFunction);
                 qsort.invokeExact(nativeArr.baseAddress(), seq.elementCount().getAsLong(), C_INT.byteSize(), qsortUpcallAddr);
+                abi.freeUpcallStub(qsortUpcallAddr);
 
                 //convert back to Java array
                 return LongStream.range(0, arr.length)

test/jdk/java/foreign/TestUpcall.java

@@ -48,6 +48,7 @@
 import java.lang.invoke.MethodHandles;
 import java.lang.invoke.MethodType;
 import java.lang.invoke.VarHandle;
+import java.lang.ref.Cleaner;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.atomic.AtomicReference;
@@ -62,6 +63,8 @@ public class TestUpcall extends CallGeneratorHelper {
 
     static LibraryLookup lib = LibraryLookup.ofLibrary(MethodHandles.lookup(), "TestUpcall");
     static SystemABI abi = SystemABI.getInstance();
+    static final MemoryAddress dummyAddress;
+    static final Cleaner cleaner = Cleaner.create();
 
     static MethodHandle DUMMY;
     static MethodHandle PASS_AND_SAVE;
@@ -70,6 +73,9 @@ public class TestUpcall extends CallGeneratorHelper {
         try {
             DUMMY = MethodHandles.lookup().findStatic(TestUpcall.class, "dummy", MethodType.methodType(void.class));
             PASS_AND_SAVE = MethodHandles.lookup().findStatic(TestUpcall.class, "passAndSave", MethodType.methodType(Object.class, Object[].class, AtomicReference.class));
+
+            dummyAddress = abi.upcallStub(DUMMY, FunctionDescriptor.ofVoid(false));
+            cleaner.register(dummyAddress, () -> abi.freeUpcallStub(dummyAddress));
         } catch (Throwable ex) {
             throw new IllegalStateException(ex);
         }
@@ -125,7 +131,7 @@ static Object[] makeArgs(Ret ret, List<ParamType> params, List<StructFieldType>
     @SuppressWarnings("unchecked")
     static MemoryAddress makeCallback(Ret ret, List<ParamType> params, List<StructFieldType> fields, List<Consumer<Object>> checks, List<Consumer<Object[]>> argChecks) {
         if (params.isEmpty()) {
-            return abi.upcallStub(DUMMY, FunctionDescriptor.ofVoid(false));
+            return dummyAddress;
         }
 
         AtomicReference<Object[]> box = new AtomicReference<>();
@@ -162,7 +168,9 @@ static MemoryAddress makeCallback(Ret ret, List<ParamType> params, List<StructFi
         FunctionDescriptor func = ret != Ret.VOID
                 ? FunctionDescriptor.of(firstlayout, false, paramLayouts)
                 : FunctionDescriptor.ofVoid(false, paramLayouts);
-        return abi.upcallStub(mh, func);
+        MemoryAddress stub = abi.upcallStub(mh, func);
+        cleaner.register(stub, () -> abi.freeUpcallStub(stub));
+        return stub;
     }
 
     private static void assertStructEquals(MemorySegment s1, MemorySegment s2, MemoryLayout layout) {

test/jdk/java/foreign/TestUpcallStubs.java

@@ -0,0 +1,104 @@
+/*
+ * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
+ *  DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ *  This code is free software; you can redistribute it and/or modify it
+ *  under the terms of the GNU General Public License version 2 only, as
+ *  published by the Free Software Foundation.
+ *
+ *  This code 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 General Public License
+ *  version 2 for more details (a copy is included in the LICENSE file that
+ *  accompanied this code).
+ *
+ *  You should have received a copy of the GNU General Public License version
+ *  2 along with this work; if not, write to the Free Software Foundation,
+ *  Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ *   Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ *  or visit www.oracle.com if you need additional information or have any
+ *  questions.
+ *
+ */
+
+/*
+ * @test
+ * @run testng TestUpcallStubs
+ */
+
+import jdk.incubator.foreign.FunctionDescriptor;
+import jdk.incubator.foreign.MemoryAddress;
+import jdk.incubator.foreign.MemorySegment;
+import jdk.incubator.foreign.SystemABI;
+import org.testng.annotations.*;
+
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.MethodType;
+import java.lang.invoke.VarHandle;
+
+import static jdk.incubator.foreign.MemoryLayouts.JAVA_INT;
+
+public class TestUpcallStubs {
+
+    static final SystemABI abi = SystemABI.getInstance();
+    static final MethodHandle MH_dummy;
+
+    static {
+        try {
+            MH_dummy = MethodHandles.lookup()
+                .findStatic(TestUpcallStubs.class, "dummy", MethodType.methodType(void.class));
+        } catch (NoSuchMethodException | IllegalAccessException e) {
+            throw new BootstrapMethodError(e);
+        }
+    }
+
+    private static MemoryAddress getStub() {
+        return abi.upcallStub(MH_dummy, FunctionDescriptor.ofVoid(false));
+    }
+
+    @Test(expectedExceptions = IndexOutOfBoundsException.class)
+    public void testNoAccess() {
+        MemoryAddress stub = getStub();
+        try {
+            VarHandle vh = JAVA_INT.varHandle(int.class);
+            vh.set(stub, 10);
+        } finally {
+            abi.freeUpcallStub(stub);
+        }
+    }
+
+    @Test
+    public void testFree() {
+        MemoryAddress stub = getStub();
+        abi.freeUpcallStub(stub);
+    }
+
+    @Test(expectedExceptions = IllegalArgumentException.class,
+          expectedExceptionsMessageRegExp = ".*Not a stub address: .*")
+    public void testAlreadyFreed() {
+        MemoryAddress stub = getStub();
+        abi.freeUpcallStub(stub);
+        abi.freeUpcallStub(stub); // should fail
+    }
+
+    @Test(expectedExceptions = IllegalArgumentException.class,
+          expectedExceptionsMessageRegExp = ".*Not a stub address: .*",
+          dataProvider = "badAddresses")
+    public void testCanNotFree(MemoryAddress ma) {
+        abi.freeUpcallStub(ma);
+    }
+
+    @DataProvider
+    public static Object[][] badAddresses() {
+        return new Object[][]{
+            { MemoryAddress.ofLong(42) /* random address */ },
+            { MemorySegment.ofArray(new int []{ 1, 2, 3 }).baseAddress() /* heap address */ }
+        };
+    }
+
+    // where
+    public static void dummy() {}
+
+}