8024974: Incorrect use of GC_locker::is_active()
Summary: SymbolTable and StringTable can make calls to GC_locker::is_active() outside a safepoint. This isn't safe because the GC_locker active state (lock count) is only updated at a safepoint and only remains valid as long as _needs_gc is true. However, outside a safepoint_needs_gc can change to false at any time, which makes it impossible to do a correct call to is_active() in that context. In this case these calls can just be removed since the input argument to basic_add() should never be on the heap and so there's no need to check the GC_locker state. This change also adjusts the assert() in is_active() to makes sure all calls to this function are always done under a safepoint.
Reviewed-by: brutisso, dcubed
Contributed-by: per.liden@oracle.com
--- a/hotspot/src/share/vm/classfile/symbolTable.cpp Wed Sep 18 15:59:41 2013 +0200
+++ b/hotspot/src/share/vm/classfile/symbolTable.cpp Fri Sep 20 10:53:28 2013 +0200
@@ -341,7 +341,7 @@
Symbol* SymbolTable::basic_add(int index_arg, u1 *name, int len,
unsigned int hashValue_arg, bool c_heap, TRAPS) {
- assert(!Universe::heap()->is_in_reserved(name) || GC_locker::is_active(),
+ assert(!Universe::heap()->is_in_reserved(name),
"proposed name of symbol must be stable");
// Don't allow symbols to be created which cannot fit in a Symbol*.
@@ -685,7 +685,7 @@
if (found_string != NULL) return found_string;
debug_only(StableMemoryChecker smc(name, len * sizeof(name[0])));
- assert(!Universe::heap()->is_in_reserved(name) || GC_locker::is_active(),
+ assert(!Universe::heap()->is_in_reserved(name),
"proposed name of symbol must be stable");
Handle string;
--- a/hotspot/src/share/vm/memory/gcLocker.cpp Wed Sep 18 15:59:41 2013 +0200
+++ b/hotspot/src/share/vm/memory/gcLocker.cpp Fri Sep 20 10:53:28 2013 +0200
@@ -122,7 +122,7 @@
// strictly needed. It's added here to make it clear that
// the GC will NOT be performed if any other caller
// of GC_locker::lock() still needs GC locked.
- if (!is_active()) {
+ if (!is_active_internal()) {
_doing_gc = true;
{
// Must give up the lock while at a safepoint
--- a/hotspot/src/share/vm/memory/gcLocker.hpp Wed Sep 18 15:59:41 2013 +0200
+++ b/hotspot/src/share/vm/memory/gcLocker.hpp Fri Sep 20 10:53:28 2013 +0200
@@ -88,7 +88,7 @@
public:
// Accessors
static bool is_active() {
- assert(_needs_gc || SafepointSynchronize::is_at_safepoint(), "only read at safepoint");
+ assert(SafepointSynchronize::is_at_safepoint(), "only read at safepoint");
return is_active_internal();
}
static bool needs_gc() { return _needs_gc; }