From d624debe23f60d778d7be43f28d06e9454057217 Mon Sep 17 00:00:00 2001 From: Christoph Langer Date: Sun, 11 Dec 2022 13:50:39 +0000 Subject: [PATCH 1/6] 8298459: Fix msys2 linking and handling out of tree build directory for source zip creation Reviewed-by: erikj --- make/ZipSource.gmk | 7 ++++--- make/common/MakeBase.gmk | 27 +++++++++++++++++++++++---- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/make/ZipSource.gmk b/make/ZipSource.gmk index 49d13433372..341af7e9847 100644 --- a/make/ZipSource.gmk +++ b/make/ZipSource.gmk @@ -1,5 +1,5 @@ # -# Copyright (c) 2014, 2020, Oracle and/or its affiliates. All rights reserved. +# Copyright (c) 2014, 2022, 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 @@ -31,6 +31,7 @@ include JavaCompilation.gmk include Modules.gmk SRC_ZIP_WORK_DIR := $(SUPPORT_OUTPUTDIR)/src +$(if $(filter $(TOPDIR)/%, $(SUPPORT_OUTPUTDIR)), $(eval SRC_ZIP_BASE := $(TOPDIR)), $(eval SRC_ZIP_BASE := $(SUPPORT_OUTPUTDIR))) # Hook to include the corresponding custom file, if present. $(eval $(call IncludeCustomExtension, ZipSource.gmk)) @@ -45,10 +46,10 @@ ALL_MODULES := $(FindAllModules) # again to create src.zip. $(foreach m, $(ALL_MODULES), \ $(foreach d, $(call FindModuleSrcDirs, $m), \ - $(eval $d_TARGET := $(SRC_ZIP_WORK_DIR)/$(patsubst $(TOPDIR)/%,%,$d)/$m) \ + $(eval $d_TARGET := $(SRC_ZIP_WORK_DIR)/$(patsubst $(TOPDIR)/%,%,$(patsubst $(SUPPORT_OUTPUTDIR)/%,%,$d))/$m) \ $(if $(SRC_GENERATED), , \ $(eval $$($d_TARGET): $d ; \ - $$(if $(filter $(TOPDIR)/%, $d), $$(link-file-relative), $$(link-file-absolute)) \ + $$(if $(filter $(SRC_ZIP_BASE)/%, $d), $$(link-file-relative), $$(link-file-absolute)) \ ) \ ) \ $(eval SRC_ZIP_SRCS += $$($d_TARGET)) \ diff --git a/make/common/MakeBase.gmk b/make/common/MakeBase.gmk index 8743f8a7619..252d9dd50da 100644 --- a/make/common/MakeBase.gmk +++ b/make/common/MakeBase.gmk @@ -306,17 +306,36 @@ endef # There are two versions, either creating a relative or an absolute link. Be # careful when using this on Windows since the symlink created is only valid in # the unix emulation environment. -define link-file-relative +# In msys2 we use mklink /J because its ln would perform a deep copy of the target. +# This inhibits performance and can lead to issues with long paths. With mklink /J +# relative linking does not work, so we handle the link as absolute path. +ifeq ($(OPENJDK_BUILD_OS_ENV), windows.msys2) + define link-file-relative + $(call MakeTargetDir) + $(RM) '$(call DecodeSpace, $@)' + cmd //c "mklink /J $(call FixPath, $(call DecodeSpace, $@)) $(call FixPath, $(call DecodeSpace, $<))" + endef +else + define link-file-relative $(call MakeTargetDir) $(RM) '$(call DecodeSpace, $@)' $(LN) -s '$(call DecodeSpace, $(call RelativePath, $<, $(@D)))' '$(call DecodeSpace, $@)' -endef + endef +endif -define link-file-absolute +ifeq ($(OPENJDK_BUILD_OS_ENV), windows.msys2) + define link-file-absolute + $(call MakeTargetDir) + $(RM) '$(call DecodeSpace, $@)' + cmd //c "mklink /J $(call FixPath, $(call DecodeSpace, $@)) $(call FixPath, $(call DecodeSpace, $<))" + endef +else + define link-file-absolute $(call MakeTargetDir) $(RM) '$(call DecodeSpace, $@)' $(LN) -s '$(call DecodeSpace, $<)' '$(call DecodeSpace, $@)' -endef + endef +endif ################################################################################ From cf93933e21d146fe296b1e4b8e2ef06b699175d6 Mon Sep 17 00:00:00 2001 From: Severin Gehwolf Date: Mon, 12 Dec 2022 15:49:31 +0000 Subject: [PATCH 2/6] 8298271: java/security/SignedJar/spi-calendar-provider/TestSPISigned.java failing on Windows Reviewed-by: mullan --- test/jdk/ProblemList.txt | 2 -- .../SignedJar/spi-calendar-provider/TestSPISigned.java | 3 ++- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/test/jdk/ProblemList.txt b/test/jdk/ProblemList.txt index d94e0b77c0b..4e00b2c1482 100644 --- a/test/jdk/ProblemList.txt +++ b/test/jdk/ProblemList.txt @@ -618,8 +618,6 @@ sun/security/pkcs11/rsa/TestKeyPairGenerator.java 8295343 linux-al sun/security/pkcs11/rsa/TestKeyFactory.java 8295343 linux-all sun/security/pkcs11/KeyStore/Basic.java 8295343 linux-all -java/security/SignedJar/spi-calendar-provider/TestSPISigned.java 8298271 windows-all - ############################################################################ # jdk_sound diff --git a/test/jdk/java/security/SignedJar/spi-calendar-provider/TestSPISigned.java b/test/jdk/java/security/SignedJar/spi-calendar-provider/TestSPISigned.java index e7cf5146252..69fe8effe70 100644 --- a/test/jdk/java/security/SignedJar/spi-calendar-provider/TestSPISigned.java +++ b/test/jdk/java/security/SignedJar/spi-calendar-provider/TestSPISigned.java @@ -33,6 +33,7 @@ import java.util.ArrayList; import java.nio.file.Paths; import java.nio.file.Path; import java.nio.file.Files; +import java.io.File; import static java.util.Calendar.WEDNESDAY; /* @@ -95,7 +96,7 @@ public class TestSPISigned { testRun.add("-Djava.locale.providers=SPI"); testRun.add("-cp"); String classPath = System.getProperty("java.class.path"); - classPath = classPath + ":" + SIGNED_JAR.toAbsolutePath().toString(); + classPath = classPath + File.pathSeparator + SIGNED_JAR.toAbsolutePath().toString(); testRun.add(classPath); testRun.add(TestSPISigned.class.getSimpleName()); testRun.add("run-test"); From 0267aa528b83be9914fee4bea8f548b8404b31f8 Mon Sep 17 00:00:00 2001 From: Naoto Sato Date: Mon, 12 Dec 2022 17:59:25 +0000 Subject: [PATCH 3/6] 8297288: Example code in Scanner class Reviewed-by: lancea, bpb, alanb --- .../share/classes/java/util/Scanner.java | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/src/java.base/share/classes/java/util/Scanner.java b/src/java.base/share/classes/java/util/Scanner.java index 9e544880ae2..f270a46cc49 100644 --- a/src/java.base/share/classes/java/util/Scanner.java +++ b/src/java.base/share/classes/java/util/Scanner.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2021, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2022, 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 @@ -51,24 +51,28 @@ import sun.util.locale.provider.ResourceBundleBasedAdapter; * various {@code next} methods. * *

For example, this code allows a user to read a number from - * {@code System.in}: - *

{@code
- *     Scanner sc = new Scanner(System.in);
- *     int i = sc.nextInt();
- * }
+ * the console. + * {@snippet : + * var con = System.console(); + * if (con != null) { + * // @link substring="reader()" target="java.io.Console#reader()" : + * Scanner sc = new Scanner(con.reader()); + * int i = sc.nextInt(); + * } + * } * *

As another example, this code allows {@code long} types to be * assigned from entries in a file {@code myNumbers}: - *

{@code
+ * {@snippet :
  *      Scanner sc = new Scanner(new File("myNumbers"));
  *      while (sc.hasNextLong()) {
  *          long aLong = sc.nextLong();
  *      }
- * }
+ * } * *

The scanner can also use delimiters other than whitespace. This * example reads several items in from a string: - *

{@code
+ * {@snippet :
  *     String input = "1 fish 2 fish red fish blue fish";
  *     Scanner s = new Scanner(input).useDelimiter("\\s*fish\\s*");
  *     System.out.println(s.nextInt());
@@ -76,7 +80,7 @@ import sun.util.locale.provider.ResourceBundleBasedAdapter;
  *     System.out.println(s.next());
  *     System.out.println(s.next());
  *     s.close();
- * }
+ * } *

* prints the following output: *

{@code
@@ -88,7 +92,7 @@ import sun.util.locale.provider.ResourceBundleBasedAdapter;
  *
  * 

The same output can be generated with this code, which uses a regular * expression to parse all four tokens at once: - *

{@code
+ * {@snippet :
  *     String input = "1 fish 2 fish red fish blue fish";
  *     Scanner s = new Scanner(input);
  *     s.findInLine("(\\d+) fish (\\d+) fish (\\w+) fish (\\w+)");
@@ -96,7 +100,7 @@ import sun.util.locale.provider.ResourceBundleBasedAdapter;
  *     for (int i=1; i<=result.groupCount(); i++)
  *         System.out.println(result.group(i));
  *     s.close();
- * }
+ * } * *

The default whitespace delimiter used * by a scanner is as recognized by {@link Character#isWhitespace(char) From 8962c723a8ae62a8638e9e0a89c20001aea1549a Mon Sep 17 00:00:00 2001 From: Alexander Matveev Date: Mon, 12 Dec 2022 22:51:02 +0000 Subject: [PATCH 4/6] 8298488: [macos13] tools/jpackage tests failing with "Exit code: 137" on macOS Reviewed-by: asemenyuk --- .../classes/jdk/jpackage/internal/MacAppImageBuilder.java | 2 +- test/jdk/tools/jpackage/helpers/jdk/jpackage/test/TKit.java | 4 ---- test/jdk/tools/jpackage/share/AppContentTest.java | 2 +- 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacAppImageBuilder.java b/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacAppImageBuilder.java index 543bbe29875..0e35d7f4fcc 100644 --- a/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacAppImageBuilder.java +++ b/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacAppImageBuilder.java @@ -401,7 +401,7 @@ public class MacAppImageBuilder extends AbstractAppImageBuilder { ENTITLEMENTS.fetchFrom(params)); } restoreKeychainList(params); - } else if (Platform.isArmMac()) { + } else if (Platform.isMac()) { signAppBundle(params, root, "-", null, null); } else { // Calling signAppBundle() without signingIdentity will result in diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/TKit.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/TKit.java index 60492b2529f..9e4d2f86c00 100644 --- a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/TKit.java +++ b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/TKit.java @@ -180,10 +180,6 @@ final public class TKit { return (OS.contains("mac")); } - public static boolean isArmMac() { - return (isOSX() && "aarch64".equals(System.getProperty("os.arch"))); - } - public static boolean isLinux() { return ((OS.contains("nix") || OS.contains("nux"))); } diff --git a/test/jdk/tools/jpackage/share/AppContentTest.java b/test/jdk/tools/jpackage/share/AppContentTest.java index a493bc39127..666fafb7168 100644 --- a/test/jdk/tools/jpackage/share/AppContentTest.java +++ b/test/jdk/tools/jpackage/share/AppContentTest.java @@ -101,7 +101,7 @@ public class AppContentTest { }) // On macOS aarch64 we always signing app image and signing will fail, since // test produces invalid app bundle. - .setExpectedExitCode(testPathArgs.contains(TEST_BAD) || TKit.isArmMac() ? 1 : 0) + .setExpectedExitCode(testPathArgs.contains(TEST_BAD) || TKit.isOSX() ? 1 : 0) .run(); } } From d4531903007dbe8dcdd163e423d23e8cefba61c8 Mon Sep 17 00:00:00 2001 From: Coleen Phillimore Date: Tue, 13 Dec 2022 00:49:16 +0000 Subject: [PATCH 5/6] 8296955: Kitchensink.java failed with "double free or corruption (!prev): " Reviewed-by: kbarrett, eosterlund, sspitsyn, dcubed --- src/hotspot/share/oops/instanceKlass.cpp | 50 ++++++++------------ src/hotspot/share/oops/method.cpp | 58 ++++++++---------------- src/hotspot/share/oops/method.hpp | 6 +-- 3 files changed, 39 insertions(+), 75 deletions(-) diff --git a/src/hotspot/share/oops/instanceKlass.cpp b/src/hotspot/share/oops/instanceKlass.cpp index d7a4c653726..7432a607d3b 100644 --- a/src/hotspot/share/oops/instanceKlass.cpp +++ b/src/hotspot/share/oops/instanceKlass.cpp @@ -2148,15 +2148,8 @@ jmethodID InstanceKlass::get_jmethod_id(const methodHandle& method_h) { // the cache can't grow so we can just get the current values get_jmethod_id_length_value(jmeths, idnum, &length, &id); } else { - // cache can grow so we have to be more careful - if (Threads::number_of_threads() == 0 || - SafepointSynchronize::is_at_safepoint()) { - // we're single threaded or at a safepoint - no locking needed - get_jmethod_id_length_value(jmeths, idnum, &length, &id); - } else { - MutexLocker ml(JmethodIdCreation_lock, Mutex::_no_safepoint_check_flag); - get_jmethod_id_length_value(jmeths, idnum, &length, &id); - } + MutexLocker ml(JmethodIdCreation_lock, Mutex::_no_safepoint_check_flag); + get_jmethod_id_length_value(jmeths, idnum, &length, &id); } } // implied else: @@ -2166,8 +2159,8 @@ jmethodID InstanceKlass::get_jmethod_id(const methodHandle& method_h) { length <= idnum || // cache is too short id == NULL) { // cache doesn't contain entry - // This function can be called by the VMThread so we have to do all - // things that might block on a safepoint before grabbing the lock. + // This function can be called by the VMThread or GC worker threads so we + // have to do all things that might block on a safepoint before grabbing the lock. // Otherwise, we can deadlock with the VMThread or have a cache // consistency issue. These vars keep track of what we might have // to free after the lock is dropped. @@ -2186,25 +2179,20 @@ jmethodID InstanceKlass::get_jmethod_id(const methodHandle& method_h) { } // allocate a new jmethodID that might be used - jmethodID new_id = NULL; - if (method_h->is_old() && !method_h->is_obsolete()) { - // The method passed in is old (but not obsolete), we need to use the current version - Method* current_method = method_with_idnum((int)idnum); - assert(current_method != NULL, "old and but not obsolete, so should exist"); - new_id = Method::make_jmethod_id(class_loader_data(), current_method); - } else { - // It is the current version of the method or an obsolete method, - // use the version passed in - new_id = Method::make_jmethod_id(class_loader_data(), method_h()); - } - - if (Threads::number_of_threads() == 0 || - SafepointSynchronize::is_at_safepoint()) { - // we're single threaded or at a safepoint - no locking needed - id = get_jmethod_id_fetch_or_update(idnum, new_id, new_jmeths, - &to_dealloc_id, &to_dealloc_jmeths); - } else { + { MutexLocker ml(JmethodIdCreation_lock, Mutex::_no_safepoint_check_flag); + jmethodID new_id = NULL; + if (method_h->is_old() && !method_h->is_obsolete()) { + // The method passed in is old (but not obsolete), we need to use the current version + Method* current_method = method_with_idnum((int)idnum); + assert(current_method != NULL, "old and but not obsolete, so should exist"); + new_id = Method::make_jmethod_id(class_loader_data(), current_method); + } else { + // It is the current version of the method or an obsolete method, + // use the version passed in + new_id = Method::make_jmethod_id(class_loader_data(), method_h()); + } + id = get_jmethod_id_fetch_or_update(idnum, new_id, new_jmeths, &to_dealloc_id, &to_dealloc_jmeths); } @@ -2254,9 +2242,7 @@ jmethodID InstanceKlass::get_jmethod_id_fetch_or_update( assert(new_id != NULL, "sanity check"); assert(to_dealloc_id_p != NULL, "sanity check"); assert(to_dealloc_jmeths_p != NULL, "sanity check"); - assert(Threads::number_of_threads() == 0 || - SafepointSynchronize::is_at_safepoint() || - JmethodIdCreation_lock->owned_by_self(), "sanity check"); + assert(JmethodIdCreation_lock->owned_by_self(), "sanity check"); // reacquire the cache - we are locked, single threaded or at a safepoint jmethodID* jmeths = methods_jmethod_ids_acquire(); diff --git a/src/hotspot/share/oops/method.cpp b/src/hotspot/share/oops/method.cpp index 39ba8f395bb..c8b9409ddaa 100644 --- a/src/hotspot/share/oops/method.cpp +++ b/src/hotspot/share/oops/method.cpp @@ -2165,50 +2165,29 @@ JNIMethodBlockNode::JNIMethodBlockNode(int num_methods) : _top(0), _next(NULL) { } } -void Method::ensure_jmethod_ids(ClassLoaderData* loader_data, int capacity) { - ClassLoaderData* cld = loader_data; - if (!SafepointSynchronize::is_at_safepoint()) { - // Have to add jmethod_ids() to class loader data thread-safely. - // Also have to add the method to the list safely, which the lock - // protects as well. - MutexLocker ml(JmethodIdCreation_lock, Mutex::_no_safepoint_check_flag); - if (cld->jmethod_ids() == NULL) { - cld->set_jmethod_ids(new JNIMethodBlock(capacity)); - } else { - cld->jmethod_ids()->ensure_methods(capacity); - } +void Method::ensure_jmethod_ids(ClassLoaderData* cld, int capacity) { + // Have to add jmethod_ids() to class loader data thread-safely. + // Also have to add the method to the list safely, which the lock + // protects as well. + MutexLocker ml(JmethodIdCreation_lock, Mutex::_no_safepoint_check_flag); + if (cld->jmethod_ids() == NULL) { + cld->set_jmethod_ids(new JNIMethodBlock(capacity)); } else { - // At safepoint, we are single threaded and can set this. - if (cld->jmethod_ids() == NULL) { - cld->set_jmethod_ids(new JNIMethodBlock(capacity)); - } else { - cld->jmethod_ids()->ensure_methods(capacity); - } + cld->jmethod_ids()->ensure_methods(capacity); } } // Add a method id to the jmethod_ids -jmethodID Method::make_jmethod_id(ClassLoaderData* loader_data, Method* m) { - ClassLoaderData* cld = loader_data; - - if (!SafepointSynchronize::is_at_safepoint()) { - // Have to add jmethod_ids() to class loader data thread-safely. - // Also have to add the method to the list safely, which the lock - // protects as well. - MutexLocker ml(JmethodIdCreation_lock, Mutex::_no_safepoint_check_flag); - if (cld->jmethod_ids() == NULL) { - cld->set_jmethod_ids(new JNIMethodBlock()); - } - // jmethodID is a pointer to Method* - return (jmethodID)cld->jmethod_ids()->add_method(m); - } else { - // At safepoint, we are single threaded and can set this. - if (cld->jmethod_ids() == NULL) { - cld->set_jmethod_ids(new JNIMethodBlock()); - } - // jmethodID is a pointer to Method* - return (jmethodID)cld->jmethod_ids()->add_method(m); +jmethodID Method::make_jmethod_id(ClassLoaderData* cld, Method* m) { + // Have to add jmethod_ids() to class loader data thread-safely. + // Also have to add the method to the list safely, which the lock + // protects as well. + assert(JmethodIdCreation_lock->owned_by_self(), "sanity check"); + if (cld->jmethod_ids() == NULL) { + cld->set_jmethod_ids(new JNIMethodBlock()); } + // jmethodID is a pointer to Method* + return (jmethodID)cld->jmethod_ids()->add_method(m); } jmethodID Method::jmethod_id() { @@ -2218,8 +2197,7 @@ jmethodID Method::jmethod_id() { // Mark a jmethodID as free. This is called when there is a data race in // InstanceKlass while creating the jmethodID cache. -void Method::destroy_jmethod_id(ClassLoaderData* loader_data, jmethodID m) { - ClassLoaderData* cld = loader_data; +void Method::destroy_jmethod_id(ClassLoaderData* cld, jmethodID m) { Method** ptr = (Method**)m; assert(cld->jmethod_ids() != NULL, "should have method handles"); cld->jmethod_ids()->destroy_method(ptr); diff --git a/src/hotspot/share/oops/method.hpp b/src/hotspot/share/oops/method.hpp index 1fc28c86df3..4a5df8134cc 100644 --- a/src/hotspot/share/oops/method.hpp +++ b/src/hotspot/share/oops/method.hpp @@ -774,13 +774,13 @@ public: // however, can be GC'ed away if the class is unloaded or if the method is // made obsolete or deleted -- in these cases, the jmethodID // refers to NULL (as is the case for any weak reference). - static jmethodID make_jmethod_id(ClassLoaderData* loader_data, Method* mh); - static void destroy_jmethod_id(ClassLoaderData* loader_data, jmethodID mid); + static jmethodID make_jmethod_id(ClassLoaderData* cld, Method* mh); + static void destroy_jmethod_id(ClassLoaderData* cld, jmethodID mid); // Ensure there is enough capacity in the internal tracking data // structures to hold the number of jmethodIDs you plan to generate. // This saves substantial time doing allocations. - static void ensure_jmethod_ids(ClassLoaderData* loader_data, int capacity); + static void ensure_jmethod_ids(ClassLoaderData* cld, int capacity); // Use resolve_jmethod_id() in situations where the caller is expected // to provide a valid jmethodID; the only sanity checks are in asserts; From 04b8d0cf5c964e16c583b66d9ab43a8c9a85fd72 Mon Sep 17 00:00:00 2001 From: Coleen Phillimore Date: Tue, 13 Dec 2022 13:02:23 +0000 Subject: [PATCH 6/6] 8298084: Memory leak in Method::build_profiling_method_data Co-authored-by: Justin King Reviewed-by: kbarrett, eosterlund, dholmes, jcking, thartmann --- src/hotspot/share/memory/metadataFactory.hpp | 14 +++++++++++--- src/hotspot/share/oops/instanceKlass.cpp | 18 ++++++++++-------- src/hotspot/share/oops/instanceKlass.hpp | 2 +- src/hotspot/share/oops/method.cpp | 7 +++---- src/hotspot/share/oops/methodData.cpp | 10 ++++++++++ src/hotspot/share/oops/methodData.hpp | 5 +++-- 6 files changed, 38 insertions(+), 18 deletions(-) diff --git a/src/hotspot/share/memory/metadataFactory.hpp b/src/hotspot/share/memory/metadataFactory.hpp index 45237efd678..4f9f7490e11 100644 --- a/src/hotspot/share/memory/metadataFactory.hpp +++ b/src/hotspot/share/memory/metadataFactory.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010, 2021, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2010, 2022, 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 @@ -30,6 +30,7 @@ #include "oops/array.inline.hpp" #include "utilities/exceptions.hpp" #include "utilities/globalDefinitions.hpp" +#include class MetadataFactory : AllStatic { public: @@ -61,14 +62,21 @@ class MetadataFactory : AllStatic { // Deallocation method for metadata template - static void free_metadata(ClassLoaderData* loader_data, T md) { + static void free_metadata(ClassLoaderData* loader_data, T* md) { if (md != NULL) { assert(loader_data != NULL, "shouldn't pass null"); int size = md->size(); - // Call metadata's deallocate function which will call deallocate fields + // Call metadata's deallocate function which will deallocate fields and release_C_heap_structures assert(!md->on_stack(), "can't deallocate things on stack"); assert(!md->is_shared(), "cannot deallocate if in shared spaces"); md->deallocate_contents(loader_data); + // Call the destructor. This is currently used for MethodData which has a member + // that needs to be destructed to release resources. Most Metadata derived classes have noop + // destructors and/or cleanup using deallocate_contents. + // T is a potentially const or volatile qualified pointer. Remove any const + // or volatile so we can call the destructor of the type T points to. + using U = std::remove_cv_t; + md->~U(); loader_data->metaspace_non_null()->deallocate((MetaWord*)md, size, md->is_klass()); } } diff --git a/src/hotspot/share/oops/instanceKlass.cpp b/src/hotspot/share/oops/instanceKlass.cpp index 7432a607d3b..dade330a248 100644 --- a/src/hotspot/share/oops/instanceKlass.cpp +++ b/src/hotspot/share/oops/instanceKlass.cpp @@ -595,10 +595,10 @@ void InstanceKlass::deallocate_contents(ClassLoaderData* loader_data) { // Release C heap allocated data that this points to, which includes // reference counting symbol names. - // Can't release the constant pool here because the constant pool can be - // deallocated separately from the InstanceKlass for default methods and - // redefine classes. - release_C_heap_structures(/* release_constant_pool */ false); + // Can't release the constant pool or MethodData C heap data here because the constant + // pool can be deallocated separately from the InstanceKlass for default methods and + // redefine classes. MethodData can also be released separately. + release_C_heap_structures(/* release_sub_metadata */ false); deallocate_methods(loader_data, methods()); set_methods(NULL); @@ -2650,13 +2650,15 @@ static void method_release_C_heap_structures(Method* m) { m->release_C_heap_structures(); } -// Called also by InstanceKlass::deallocate_contents, with false for release_constant_pool. -void InstanceKlass::release_C_heap_structures(bool release_constant_pool) { +// Called also by InstanceKlass::deallocate_contents, with false for release_sub_metadata. +void InstanceKlass::release_C_heap_structures(bool release_sub_metadata) { // Clean up C heap Klass::release_C_heap_structures(); // Deallocate and call destructors for MDO mutexes - methods_do(method_release_C_heap_structures); + if (release_sub_metadata) { + methods_do(method_release_C_heap_structures); + } // Destroy the init_monitor delete _init_monitor; @@ -2696,7 +2698,7 @@ void InstanceKlass::release_C_heap_structures(bool release_constant_pool) { FREE_C_HEAP_ARRAY(char, _source_debug_extension); - if (release_constant_pool) { + if (release_sub_metadata) { constants()->release_C_heap_structures(); } } diff --git a/src/hotspot/share/oops/instanceKlass.hpp b/src/hotspot/share/oops/instanceKlass.hpp index 9911176b71d..56545831f48 100644 --- a/src/hotspot/share/oops/instanceKlass.hpp +++ b/src/hotspot/share/oops/instanceKlass.hpp @@ -1012,7 +1012,7 @@ public: // callbacks for actions during class unloading static void unload_class(InstanceKlass* ik); - virtual void release_C_heap_structures(bool release_constant_pool = true); + virtual void release_C_heap_structures(bool release_sub_metadata = true); // Naming const char* signature_name() const; diff --git a/src/hotspot/share/oops/method.cpp b/src/hotspot/share/oops/method.cpp index c8b9409ddaa..b5355a156bb 100644 --- a/src/hotspot/share/oops/method.cpp +++ b/src/hotspot/share/oops/method.cpp @@ -141,10 +141,9 @@ void Method::deallocate_contents(ClassLoaderData* loader_data) { void Method::release_C_heap_structures() { if (method_data()) { -#if INCLUDE_JVMCI - FailedSpeculation::free_failed_speculations(method_data()->get_failed_speculations_address()); -#endif - // Destroy MethodData + method_data()->release_C_heap_structures(); + + // Destroy MethodData embedded lock method_data()->~MethodData(); } } diff --git a/src/hotspot/share/oops/methodData.cpp b/src/hotspot/share/oops/methodData.cpp index c8fb31123ca..c8b12329b92 100644 --- a/src/hotspot/share/oops/methodData.cpp +++ b/src/hotspot/share/oops/methodData.cpp @@ -1821,3 +1821,13 @@ void MethodData::clean_weak_method_links() { clean_extra_data(&cl); verify_extra_data_clean(&cl); } + +void MethodData::deallocate_contents(ClassLoaderData* loader_data) { + release_C_heap_structures(); +} + +void MethodData::release_C_heap_structures() { +#if INCLUDE_JVMCI + FailedSpeculation::free_failed_speculations(get_failed_speculations_address()); +#endif +} diff --git a/src/hotspot/share/oops/methodData.hpp b/src/hotspot/share/oops/methodData.hpp index 9dcff2b7ed9..508278e6d74 100644 --- a/src/hotspot/share/oops/methodData.hpp +++ b/src/hotspot/share/oops/methodData.hpp @@ -2449,8 +2449,9 @@ public: virtual void metaspace_pointers_do(MetaspaceClosure* iter); virtual MetaspaceObj::Type type() const { return MethodDataType; } - // Deallocation support - no metaspace pointer fields to deallocate - void deallocate_contents(ClassLoaderData* loader_data) {} + // Deallocation support + void deallocate_contents(ClassLoaderData* loader_data); + void release_C_heap_structures(); // GC support void set_size(int object_size_in_bytes) { _size = object_size_in_bytes; }