diff --git a/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp b/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp index 1959ca3082..5eda5de4f2 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp @@ -1283,11 +1283,16 @@ void ShenandoahHeap::object_iterate(ObjectClosure* cl) { Stack oop_stack; - // First, we process GC roots according to current GC cycle. This populates the work stack with initial objects. - ShenandoahHeapIterationRootScanner rp; ObjectIterateScanRootClosure oops(&_aux_bit_map, &oop_stack); - rp.roots_do(&oops); + { + // First, we process GC roots according to current GC cycle. + // This populates the work stack with initial objects. + // It is important to relinquish the associated locks before diving + // into heap dumper. + ShenandoahHeapIterationRootScanner rp; + rp.roots_do(&oops); + } // Work through the oop stack to traverse heap. while (! oop_stack.is_empty()) { diff --git a/src/hotspot/share/gc/shenandoah/shenandoahPacer.cpp b/src/hotspot/share/gc/shenandoah/shenandoahPacer.cpp index 86d52e1978..5a4069a504 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahPacer.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahPacer.cpp @@ -27,6 +27,7 @@ #include "gc/shenandoah/shenandoahHeap.inline.hpp" #include "gc/shenandoah/shenandoahPacer.hpp" #include "runtime/atomic.hpp" +#include "runtime/mutexLocker.hpp" /* * In normal concurrent cycle, we have to pace the application to let GC finish. @@ -48,12 +49,8 @@ * notion of progress is clear: we get reported the "used" size from the processed regions * and use the global heap-used as the baseline. * - * The allocatable space when GC is running is "free" at the start of cycle, but the + * The allocatable space when GC is running is "free" at the start of phase, but the * accounted budget is based on "used". So, we need to adjust the tax knowing that. - * Also, since we effectively count the used space three times (mark, evac, update-refs), - * we need to multiply the tax by 3. Example: for 10 MB free and 90 MB used, GC would - * come back with 3*90 MB budget, and thus for each 1 MB of allocation, we have to pay - * 3*90 / 10 MBs. In the end, we would pay back the entire budget. */ void ShenandoahPacer::setup_for_mark() { @@ -66,7 +63,7 @@ void ShenandoahPacer::setup_for_mark() { size_t taxable = free - non_taxable; double tax = 1.0 * live / taxable; // base tax for available free space - tax *= 3; // mark is phase 1 of 3, claim 1/3 of free for it + tax *= 1; // mark can succeed with immediate garbage, claim all available space tax *= ShenandoahPacingSurcharge; // additional surcharge to help unclutter heap restart_with(non_taxable, tax); @@ -89,7 +86,7 @@ void ShenandoahPacer::setup_for_evac() { size_t taxable = free - non_taxable; double tax = 1.0 * used / taxable; // base tax for available free space - tax *= 2; // evac is phase 2 of 3, claim 1/2 of remaining free + tax *= 2; // evac is followed by update-refs, claim 1/2 of remaining free tax = MAX2(1, tax); // never allocate more than GC processes during the phase tax *= ShenandoahPacingSurcharge; // additional surcharge to help unclutter heap @@ -113,7 +110,7 @@ void ShenandoahPacer::setup_for_updaterefs() { size_t taxable = free - non_taxable; double tax = 1.0 * used / taxable; // base tax for available free space - tax *= 1; // update-refs is phase 3 of 3, claim the remaining free + tax *= 1; // update-refs is the last phase, claim the remaining free tax = MAX2(1, tax); // never allocate more than GC processes during the phase tax *= ShenandoahPacingSurcharge; // additional surcharge to help unclutter heap @@ -238,7 +235,7 @@ void ShenandoahPacer::pace_for_alloc(size_t words) { } // Threads that are attaching should not block at all: they are not - // fully initialized yet. Calling sleep() on them would be awkward. + // fully initialized yet. Blocking them would be awkward. // This is probably the path that allocates the thread oop itself. // Forcefully claim without waiting. if (JavaThread::current()->is_attaching_via_jni()) { @@ -263,7 +260,7 @@ void ShenandoahPacer::pace_for_alloc(size_t words) { } cur = MAX2(1, cur); - JavaThread::current()->sleep(cur); + wait(cur); double end = os::elapsedTime(); total = (size_t)((end - start) * 1000); @@ -288,6 +285,20 @@ void ShenandoahPacer::pace_for_alloc(size_t words) { } } +void ShenandoahPacer::wait(size_t time_ms) { + // Perform timed wait. It works like like sleep(), except without modifying + // the thread interruptible status. MonitorLocker also checks for safepoints. + assert(time_ms > 0, "Should not call this with zero argument, as it would stall until notify"); + assert(time_ms <= LONG_MAX, "Sanity"); + MonitorLocker locker(_wait_monitor); + _wait_monitor->wait((long)time_ms); +} + +void ShenandoahPacer::notify_waiters() { + MonitorLocker locker(_wait_monitor); + _wait_monitor->notify_all(); +} + void ShenandoahPacer::print_on(outputStream* out) const { out->print_cr("ALLOCATION PACING:"); out->cr(); diff --git a/src/hotspot/share/gc/shenandoah/shenandoahPacer.hpp b/src/hotspot/share/gc/shenandoah/shenandoahPacer.hpp index fbee2e0c12..3f406deabe 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahPacer.hpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahPacer.hpp @@ -44,6 +44,7 @@ private: ShenandoahHeap* _heap; BinaryMagnitudeSeq _delays; TruncatedSeq* _progress_history; + Monitor* _wait_monitor; // Set once per phase volatile intptr_t _epoch; @@ -63,6 +64,7 @@ public: ShenandoahPacer(ShenandoahHeap* heap) : _heap(heap), _progress_history(new TruncatedSeq(5)), + _wait_monitor(new Monitor(Mutex::leaf, "_wait_monitor", true, Monitor::_safepoint_check_always)), _epoch(0), _tax_rate(1), _budget(0), @@ -97,6 +99,9 @@ private: void restart_with(size_t non_taxable_bytes, double tax_rate); size_t update_and_get_progress_history(); + + void wait(size_t time_ms); + void notify_waiters(); }; #endif // SHARE_GC_SHENANDOAH_SHENANDOAHPACER_HPP diff --git a/src/java.base/share/classes/java/security/Provider.java b/src/java.base/share/classes/java/security/Provider.java index ceabd44057..2b27de0520 100644 --- a/src/java.base/share/classes/java/security/Provider.java +++ b/src/java.base/share/classes/java/security/Provider.java @@ -868,7 +868,7 @@ public abstract class Provider extends Properties { // For backward compatibility, the registration ordering of // SecureRandom (RNG) algorithms needs to be preserved for // "new SecureRandom()" calls when this provider is used - private transient Set prngServices; + private transient Set prngAlgos; // Map // used for services added via legacy methods, init on demand @@ -1089,7 +1089,7 @@ public abstract class Provider extends Properties { legacyChanged = false; servicesChanged = false; serviceSet = null; - prngServices = null; + prngAlgos = null; super.clear(); putId(); } @@ -1221,7 +1221,7 @@ public abstract class Provider extends Properties { s.className = className; if (type.equals("SecureRandom")) { - updateSecureRandomEntries(true, s); + updateSecureRandomEntries(true, s.algorithm); } } else { // attribute // e.g. put("MessageDigest.SHA-1 ImplementedIn", "Software"); @@ -1383,25 +1383,25 @@ public abstract class Provider extends Properties { synchronized (this) { putPropertyStrings(s); if (type.equals("SecureRandom")) { - updateSecureRandomEntries(true, s); + updateSecureRandomEntries(true, s.algorithm); } } } - private void updateSecureRandomEntries(boolean doAdd, Service s) { + // keep tracks of the registered secure random algos and store them in order + private void updateSecureRandomEntries(boolean doAdd, String s) { Objects.requireNonNull(s); if (doAdd) { - if (prngServices == null) { - prngServices = new LinkedHashSet(); + if (prngAlgos == null) { + prngAlgos = new LinkedHashSet(); } - prngServices.add(s); + prngAlgos.add(s); } else { - prngServices.remove(s); + prngAlgos.remove(s); } if (debug != null) { - debug.println((doAdd? "Add":"Remove") + " SecureRandom algo " + - s.getAlgorithm()); + debug.println((doAdd? "Add":"Remove") + " SecureRandom algo " + s); } } @@ -1411,12 +1411,15 @@ public abstract class Provider extends Properties { checkInitialized(); if (legacyChanged) { - prngServices = null; + prngAlgos = null; ensureLegacyParsed(); } - if (prngServices != null && !prngServices.isEmpty()) { - return prngServices.iterator().next(); + if (prngAlgos != null && !prngAlgos.isEmpty()) { + // IMPORTANT: use the Service obj returned by getService(...) call + // as providers may override putService(...)/getService(...) and + // return their own Service objects + return getService("SecureRandom", prngAlgos.iterator().next()); } return null; @@ -1516,7 +1519,7 @@ public abstract class Provider extends Properties { synchronized (this) { removePropertyStrings(s); if (type.equals("SecureRandom")) { - updateSecureRandomEntries(false, s); + updateSecureRandomEntries(false, s.algorithm); } } } diff --git a/test/jdk/java/security/SecureRandom/DefaultAlgo.java b/test/jdk/java/security/SecureRandom/DefaultAlgo.java index d85d73b2cb..06027f7162 100644 --- a/test/jdk/java/security/SecureRandom/DefaultAlgo.java +++ b/test/jdk/java/security/SecureRandom/DefaultAlgo.java @@ -26,13 +26,12 @@ import java.security.Provider; import java.security.Security; import java.security.SecureRandom; import java.security.Provider.Service; -import java.util.Objects; import java.util.Arrays; import sun.security.provider.SunEntries; /** * @test - * @bug 8228613 8246613 + * @bug 8228613 8246613 8248505 * @summary Ensure that the default SecureRandom algo used is based * on the registration ordering, and falls to next provider * if none are found @@ -40,6 +39,9 @@ import sun.security.provider.SunEntries; */ public class DefaultAlgo { + private static final String SR_IMPLCLASS = + "sun.security.provider.SecureRandom"; + public static void main(String[] args) throws Exception { String[] algos = { "A", "B", "C" }; test3rdParty(algos); @@ -51,7 +53,8 @@ public class DefaultAlgo { private static void test3rdParty(String[] algos) { Provider[] provs = { new SampleLegacyProvider(algos), - new SampleServiceProvider(algos) + new SampleServiceProvider(algos), + new CustomProvider(algos) }; for (Provider p : provs) { checkDefault(p, algos); @@ -72,9 +75,10 @@ public class DefaultAlgo { } private static void checkDefault(Provider p, String ... algos) { - out.println(p.getName() + " with " + Arrays.toString(algos)); - int pos = Security.insertProviderAt(p, 1); String pName = p.getName(); + out.println(pName + " with " + Arrays.toString(algos)); + int pos = Security.insertProviderAt(p, 1); + boolean isLegacy = pName.equals("SampleLegacy"); try { if (isLegacy) { @@ -91,7 +95,13 @@ public class DefaultAlgo { out.println("=> Test Passed"); } finally { if (pos != -1) { - Security.removeProvider(p.getName()); + Security.removeProvider(pName); + } + if (isLegacy) { + // add back the removed algos + for (String s : algos) { + p.put("SecureRandom." + s, SR_IMPLCLASS); + } } } } @@ -100,7 +110,7 @@ public class DefaultAlgo { SampleLegacyProvider(String[] listOfSupportedRNGs) { super("SampleLegacy", "1.0", "test provider using legacy put"); for (String s : listOfSupportedRNGs) { - put("SecureRandom." + s, "sun.security.provider.SecureRandom"); + put("SecureRandom." + s, SR_IMPLCLASS); } } } @@ -110,8 +120,37 @@ public class DefaultAlgo { super("SampleService", "1.0", "test provider using putService"); for (String s : listOfSupportedRNGs) { putService(new Provider.Service(this, "SecureRandom", s, - "sun.security.provider.SecureRandom", null, null)); + SR_IMPLCLASS, null, null)); } } } + + // custom provider which overrides putService(...)/getService() and uses + // its own Service impl + private static class CustomProvider extends Provider { + private static class CustomService extends Provider.Service { + CustomService(Provider p, String type, String algo, String cName) { + super(p, type, algo, cName, null, null); + } + } + + CustomProvider(String[] listOfSupportedRNGs) { + super("Custom", "1.0", "test provider overrides putService with " + + " custom service with legacy registration"); + for (String s : listOfSupportedRNGs) { + putService(new CustomService(this, "SecureRandom", s , + SR_IMPLCLASS)); + } + } + @Override + protected void putService(Provider.Service s) { + // convert to legacy puts + put(s.getType() + "." + s.getAlgorithm(), s.getClassName()); + put(s.getType() + ":" + s.getAlgorithm(), s); + } + @Override + public Provider.Service getService(String type, String algo) { + return (Provider.Service) get(type + ":" + algo); + } + } }