# HG changeset patch # User ehelin # Date 1411568960 -7200 # Wed Sep 24 16:29:20 2014 +0200 # Node ID d527d89ab0fdebbd905f05e6420e31ac7c3b52aa # Parent 74305fe8f5099ac8566abec130a1058f019465ee 8049599: MetaspaceGC::_capacity_until_GC can overflow diff --git a/src/share/vm/memory/metaspace.cpp b/src/share/vm/memory/metaspace.cpp --- a/src/share/vm/memory/metaspace.cpp +++ b/src/share/vm/memory/metaspace.cpp @@ -1413,17 +1413,39 @@ size_t MetaspaceGC::capacity_until_GC() size_t value = (size_t)OrderAccess::load_ptr_acquire(&_capacity_until_GC); assert(value >= MetaspaceSize, "Not initialied properly?"); return value; } size_t MetaspaceGC::inc_capacity_until_GC(size_t v) { assert_is_size_aligned(v, Metaspace::commit_alignment()); - return (size_t)Atomic::add_ptr(v, &_capacity_until_GC); + size_t capacity_until_GC = (size_t) _capacity_until_GC; + size_t new_value = capacity_until_GC + v; + + if (new_value < capacity_until_GC) { + // the addition wrapped around, set new_value to max value + new_value = max_uintx; + } + + bool cas_succeeded = false; + size_t num_retries = 5; // just to limit the number of CAS attempts + for (size_t i = 0; i < num_retries; i++) { + intptr_t cmp = (intptr_t) capacity_until_GC; + intptr_t swap = (intptr_t) new_value; + intptr_t prev = Atomic::cmpxchg_ptr(swap, &_capacity_until_GC, cmp); + + if (prev == cmp) { + cas_succeeded = true; + break; + } + } + + // if the CAS did not succeed, just return the current value + return cas_succeeded ? new_value : _capacity_until_GC; } size_t MetaspaceGC::dec_capacity_until_GC(size_t v) { assert_is_size_aligned(v, Metaspace::commit_alignment()); return (size_t)Atomic::add_ptr(-(intptr_t)v, &_capacity_until_GC); } diff --git a/src/share/vm/prims/whitebox.cpp b/src/share/vm/prims/whitebox.cpp --- a/src/share/vm/prims/whitebox.cpp +++ b/src/share/vm/prims/whitebox.cpp @@ -815,16 +815,35 @@ WB_ENTRY(void, WB_FreeMetaspace(JNIEnv* oop class_loader_oop = JNIHandles::resolve(class_loader); ClassLoaderData* cld = class_loader_oop != NULL ? java_lang_ClassLoader::loader_data(class_loader_oop) : ClassLoaderData::the_null_class_loader_data(); MetadataFactory::free_array(cld, (Array*)(uintptr_t)addr); WB_END +WB_ENTRY(jlong, WB_IncMetaspaceCapacityUntilGC(JNIEnv* env, jobject wb, jlong size)) + if (size < 0) { + THROW_MSG_0(vmSymbols::java_lang_IllegalArgumentException(), + err_msg("WB_IncMetaspaceCapacityUntilGC: size is negative: " JLONG_FORMAT, size)); + } + + if (size > (jlong) ((size_t) -1)) { + THROW_MSG_0(vmSymbols::java_lang_IllegalArgumentException(), + err_msg("WB_IncMetaspaceCapacityUntilGC: size does not fit in size_t: " JLONG_FORMAT, size)); + } + + size_t new_val = MetaspaceGC::inc_capacity_until_GC(align_size_down((size_t) size, Metaspace::commit_alignment())); + return (jlong) new_val; +WB_END + +WB_ENTRY(jlong, WB_MetaspaceCapacityUntilGC(JNIEnv* env, jobject wb)) + return (jlong) MetaspaceGC::capacity_until_GC(); +WB_END + //Some convenience methods to deal with objects from java int WhiteBox::offset_for_field(const char* field_name, oop object, Symbol* signature_symbol) { assert(field_name != NULL && strlen(field_name) > 0, "Field name not valid"); Thread* THREAD = Thread::current(); //Get the class of our object Klass* arg_klass = object->klass(); @@ -986,16 +1005,18 @@ static JNINativeMethod methods[] = { {CC"isInStringTable", CC"(Ljava/lang/String;)Z", (void*)&WB_IsInStringTable }, {CC"fullGC", CC"()V", (void*)&WB_FullGC }, {CC"youngGC", CC"()V", (void*)&WB_YoungGC }, {CC"readReservedMemory", CC"()V", (void*)&WB_ReadReservedMemory }, {CC"allocateMetaspace", CC"(Ljava/lang/ClassLoader;J)J", (void*)&WB_AllocateMetaspace }, {CC"freeMetaspace", CC"(Ljava/lang/ClassLoader;JJ)V", (void*)&WB_FreeMetaspace }, + {CC"incMetaspaceCapacityUntilGC", CC"(J)J", (void*)&WB_IncMetaspaceCapacityUntilGC }, + {CC"metaspaceCapacityUntilGC", CC"()J", (void*)&WB_MetaspaceCapacityUntilGC }, {CC"getCPUFeatures", CC"()Ljava/lang/String;", (void*)&WB_GetCPUFeatures }, {CC"getNMethod", CC"(Ljava/lang/reflect/Executable;Z)[Ljava/lang/Object;", (void*)&WB_GetNMethod }, {CC"getThreadStackSize", CC"()J", (void*)&WB_GetThreadStackSize }, {CC"getThreadRemainingStackSize", CC"()J", (void*)&WB_GetThreadRemainingStackSize }, }; #undef CC diff --git a/test/gc/metaspace/TestCapacityUntilGCWrapAround.java b/test/gc/metaspace/TestCapacityUntilGCWrapAround.java new file mode 100644 --- /dev/null +++ b/test/gc/metaspace/TestCapacityUntilGCWrapAround.java @@ -0,0 +1,58 @@ +/* + * Copyright (c) 2014, 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 + * @key gc + * @bug 8049831 + * @library /testlibrary /testlibrary/whitebox + * @build TestCapacityUntilGCWrapAround + * @run main ClassFileInstaller sun.hotspot.WhiteBox + * sun.hotspot.WhiteBox$WhiteBoxPermission + * @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI TestCapacityUntilGCWrapAround + */ + +import sun.hotspot.WhiteBox; + +import com.oracle.java.testlibrary.Asserts; +import com.oracle.java.testlibrary.Platform; + +public class TestCapacityUntilGCWrapAround { + private static long MB = 1024 * 1024; + private static long GB = 1024 * MB; + private static long MAX_UINT = 4 * GB - 1; // On 32-bit platforms + + public static void main(String[] args) { + if (Platform.is32bit()) { + WhiteBox wb = WhiteBox.getWhiteBox(); + + long before = wb.metaspaceCapacityUntilGC(); + long after = wb.incMetaspaceCapacityUntilGC(MAX_UINT); + + Asserts.assertGTE(after, before, + String.format("Increasing with MAX_UINT should not cause wrap around: %d < %d", after, before)); + Asserts.assertEQ(after, MAX_UINT, + String.format("Increasing with MAX_UINT should set Metaspace::_capacityUntilGC to max_uint, not: %d", after)); + } + } +} diff --git a/test/testlibrary/whitebox/sun/hotspot/WhiteBox.java b/test/testlibrary/whitebox/sun/hotspot/WhiteBox.java --- a/test/testlibrary/whitebox/sun/hotspot/WhiteBox.java +++ b/test/testlibrary/whitebox/sun/hotspot/WhiteBox.java @@ -147,16 +147,18 @@ public class WhiteBox { // Intered strings public native boolean isInStringTable(String str); // Memory public native void readReservedMemory(); public native long allocateMetaspace(ClassLoader classLoader, long size); public native void freeMetaspace(ClassLoader classLoader, long addr, long size); + public native long incMetaspaceCapacityUntilGC(long increment); + public native long metaspaceCapacityUntilGC(); // force Young GC public native void youngGC(); // force Full GC public native void fullGC(); // Tests on ReservedSpace/VirtualSpace classes # HG changeset patch # User ehelin # Date 1411568964 -7200 # Wed Sep 24 16:29:24 2014 +0200 # Node ID 39b8dfa5571426df8f8d236e0a2158e0f7472c8f # Parent d527d89ab0fdebbd905f05e6420e31ac7c3b52aa 8049599: MetaspaceGC::_capacity_until_GC can overflow (01) diff --git a/src/share/vm/memory/metaspace.cpp b/src/share/vm/memory/metaspace.cpp --- a/src/share/vm/memory/metaspace.cpp +++ b/src/share/vm/memory/metaspace.cpp @@ -1410,42 +1410,41 @@ size_t MetaspaceGC::delta_capacity_until } size_t MetaspaceGC::capacity_until_GC() { size_t value = (size_t)OrderAccess::load_ptr_acquire(&_capacity_until_GC); assert(value >= MetaspaceSize, "Not initialied properly?"); return value; } -size_t MetaspaceGC::inc_capacity_until_GC(size_t v) { +bool MetaspaceGC::inc_capacity_until_GC(size_t v, size_t* new_cap_until_GC, size_t* old_cap_until_GC) { assert_is_size_aligned(v, Metaspace::commit_alignment()); size_t capacity_until_GC = (size_t) _capacity_until_GC; size_t new_value = capacity_until_GC + v; if (new_value < capacity_until_GC) { - // the addition wrapped around, set new_value to max value - new_value = max_uintx; + // the addition wrapped around, set new_value to aligned max value + new_value = align_size_down(max_uintx, Metaspace::commit_alignment()); } - bool cas_succeeded = false; - size_t num_retries = 5; // just to limit the number of CAS attempts - for (size_t i = 0; i < num_retries; i++) { - intptr_t cmp = (intptr_t) capacity_until_GC; - intptr_t swap = (intptr_t) new_value; - intptr_t prev = Atomic::cmpxchg_ptr(swap, &_capacity_until_GC, cmp); - - if (prev == cmp) { - cas_succeeded = true; - break; - } + intptr_t expected = (intptr_t) capacity_until_GC; + intptr_t actual = Atomic::cmpxchg((intptr_t) new_value, &_capacity_until_GC, expected); + + if (expected != actual) { + return false; } - // if the CAS did not succeed, just return the current value - return cas_succeeded ? new_value : _capacity_until_GC; + if (new_cap_until_GC != NULL) { + *new_cap_until_GC = new_value; + } + if (old_cap_until_GC != NULL) { + *old_cap_until_GC = capacity_until_GC; + } + return true; } size_t MetaspaceGC::dec_capacity_until_GC(size_t v) { assert_is_size_aligned(v, Metaspace::commit_alignment()); return (size_t)Atomic::add_ptr(-(intptr_t)v, &_capacity_until_GC); } @@ -1535,17 +1534,20 @@ void MetaspaceGC::compute_new_size() { size_t shrink_bytes = 0; if (capacity_until_GC < minimum_desired_capacity) { // If we have less capacity below the metaspace HWM, then // increment the HWM. size_t expand_bytes = minimum_desired_capacity - capacity_until_GC; expand_bytes = align_size_up(expand_bytes, Metaspace::commit_alignment()); // Don't expand unless it's significant if (expand_bytes >= MinMetaspaceExpansion) { - size_t new_capacity_until_GC = MetaspaceGC::inc_capacity_until_GC(expand_bytes); + size_t new_capacity_until_GC = 0; + bool succeeded = MetaspaceGC::inc_capacity_until_GC(expand_bytes, &new_capacity_until_GC); + assert(succeeded, "Should always succesfully increment HWM when at safepoint"); + Metaspace::tracer()->report_gc_threshold(capacity_until_GC, new_capacity_until_GC, MetaspaceGCThresholdUpdater::ComputeNewSize); if (PrintGCDetails && Verbose) { gclog_or_tty->print_cr(" expanding:" " minimum_desired_capacity: %6.1fKB" " expand_bytes: %6.1fKB" " MinMetaspaceExpansion: %6.1fKB" @@ -3338,29 +3340,38 @@ MetaWord* Metaspace::allocate(size_t wor return vsm()->allocate(word_size); } } MetaWord* Metaspace::expand_and_allocate(size_t word_size, MetadataType mdtype) { size_t delta_bytes = MetaspaceGC::delta_capacity_until_GC(word_size * BytesPerWord); assert(delta_bytes > 0, "Must be"); - size_t after_inc = MetaspaceGC::inc_capacity_until_GC(delta_bytes); - - // capacity_until_GC might be updated concurrently, must calculate previous value. - size_t before_inc = after_inc - delta_bytes; - - tracer()->report_gc_threshold(before_inc, after_inc, - MetaspaceGCThresholdUpdater::ExpandAndAllocate); - if (PrintGCDetails && Verbose) { - gclog_or_tty->print_cr("Increase capacity to GC from " SIZE_FORMAT - " to " SIZE_FORMAT, before_inc, after_inc); + size_t before_inc = 0; + size_t after_inc = 0; + MetaWord* res = NULL; + bool inc_succeeded = false; + + while (!inc_succeeded && res == NULL) { + inc_succeeded = MetaspaceGC::inc_capacity_until_GC(delta_bytes, + &after_inc, + &before_inc); + res = allocate(word_size, mdtype); } - return allocate(word_size, mdtype); + if (inc_succeeded) { + tracer()->report_gc_threshold(before_inc, after_inc, + MetaspaceGCThresholdUpdater::ExpandAndAllocate); + if (PrintGCDetails && Verbose) { + gclog_or_tty->print_cr("Increase capacity to GC from " SIZE_FORMAT + " to " SIZE_FORMAT, before_inc, after_inc); + } + } + + return res; } // Space allocated in the Metaspace. This may // be across several metadata virtual spaces. char* Metaspace::bottom() const { assert(DumpSharedSpaces, "only useful and valid for dumping shared spaces"); return (char*)vsm()->current_chunk()->bottom(); } diff --git a/src/share/vm/memory/metaspace.hpp b/src/share/vm/memory/metaspace.hpp --- a/src/share/vm/memory/metaspace.hpp +++ b/src/share/vm/memory/metaspace.hpp @@ -402,17 +402,19 @@ class MetaspaceGC : AllStatic { void set_shrink_factor(uint v) { _shrink_factor = v; } public: static void initialize(); static void post_initialize(); static size_t capacity_until_GC(); - static size_t inc_capacity_until_GC(size_t v); + static bool inc_capacity_until_GC(size_t v, + size_t* new_cap_until_GC = NULL, + size_t* old_cap_until_GC = NULL); static size_t dec_capacity_until_GC(size_t v); static bool should_concurrent_collect() { return _should_concurrent_collect; } static void set_should_concurrent_collect(bool v) { _should_concurrent_collect = v; } // The amount to increase the high-water-mark (_capacity_until_GC) diff --git a/src/share/vm/prims/whitebox.cpp b/src/share/vm/prims/whitebox.cpp --- a/src/share/vm/prims/whitebox.cpp +++ b/src/share/vm/prims/whitebox.cpp @@ -821,22 +821,30 @@ WB_ENTRY(void, WB_FreeMetaspace(JNIEnv* WB_END WB_ENTRY(jlong, WB_IncMetaspaceCapacityUntilGC(JNIEnv* env, jobject wb, jlong size)) if (size < 0) { THROW_MSG_0(vmSymbols::java_lang_IllegalArgumentException(), err_msg("WB_IncMetaspaceCapacityUntilGC: size is negative: " JLONG_FORMAT, size)); } - if (size > (jlong) ((size_t) -1)) { + jlong max_size_t = (jlong) ((size_t) -1); + if (size > max_size_t) { THROW_MSG_0(vmSymbols::java_lang_IllegalArgumentException(), err_msg("WB_IncMetaspaceCapacityUntilGC: size does not fit in size_t: " JLONG_FORMAT, size)); } - size_t new_val = MetaspaceGC::inc_capacity_until_GC(align_size_down((size_t) size, Metaspace::commit_alignment())); + size_t new_val = 0; + size_t aligned_increment = align_size_down((size_t) size, Metaspace::commit_alignment()); + bool success = MetaspaceGC::inc_capacity_until_GC(aligned_increment, &new_val); + if (!success) { + THROW_MSG_0(vmSymbols::java_lang_IllegalStateException(), + "WB_IncMetaspaceCapacityUntilGC: could not increase capacity until GC " + "due to contention with another thread"); + } return (jlong) new_val; WB_END WB_ENTRY(jlong, WB_MetaspaceCapacityUntilGC(JNIEnv* env, jobject wb)) return (jlong) MetaspaceGC::capacity_until_GC(); WB_END //Some convenience methods to deal with objects from java diff --git a/test/gc/metaspace/TestCapacityUntilGCWrapAround.java b/test/gc/metaspace/TestCapacityUntilGCWrapAround.java --- a/test/gc/metaspace/TestCapacityUntilGCWrapAround.java +++ b/test/gc/metaspace/TestCapacityUntilGCWrapAround.java @@ -45,14 +45,12 @@ public class TestCapacityUntilGCWrapArou public static void main(String[] args) { if (Platform.is32bit()) { WhiteBox wb = WhiteBox.getWhiteBox(); long before = wb.metaspaceCapacityUntilGC(); long after = wb.incMetaspaceCapacityUntilGC(MAX_UINT); Asserts.assertGTE(after, before, - String.format("Increasing with MAX_UINT should not cause wrap around: %d < %d", after, before)); - Asserts.assertEQ(after, MAX_UINT, - String.format("Increasing with MAX_UINT should set Metaspace::_capacityUntilGC to max_uint, not: %d", after)); + "Increasing with MAX_UINT should not cause wrap around: " + after + " < " + before); } } }