Fri, 20 Sep 2013 10:53:28 +0200
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
1.1 --- a/src/share/vm/classfile/symbolTable.cpp Wed Sep 18 15:59:41 2013 +0200 1.2 +++ b/src/share/vm/classfile/symbolTable.cpp Fri Sep 20 10:53:28 2013 +0200 1.3 @@ -341,7 +341,7 @@ 1.4 1.5 Symbol* SymbolTable::basic_add(int index_arg, u1 *name, int len, 1.6 unsigned int hashValue_arg, bool c_heap, TRAPS) { 1.7 - assert(!Universe::heap()->is_in_reserved(name) || GC_locker::is_active(), 1.8 + assert(!Universe::heap()->is_in_reserved(name), 1.9 "proposed name of symbol must be stable"); 1.10 1.11 // Don't allow symbols to be created which cannot fit in a Symbol*. 1.12 @@ -685,7 +685,7 @@ 1.13 if (found_string != NULL) return found_string; 1.14 1.15 debug_only(StableMemoryChecker smc(name, len * sizeof(name[0]))); 1.16 - assert(!Universe::heap()->is_in_reserved(name) || GC_locker::is_active(), 1.17 + assert(!Universe::heap()->is_in_reserved(name), 1.18 "proposed name of symbol must be stable"); 1.19 1.20 Handle string;
2.1 --- a/src/share/vm/memory/gcLocker.cpp Wed Sep 18 15:59:41 2013 +0200 2.2 +++ b/src/share/vm/memory/gcLocker.cpp Fri Sep 20 10:53:28 2013 +0200 2.3 @@ -122,7 +122,7 @@ 2.4 // strictly needed. It's added here to make it clear that 2.5 // the GC will NOT be performed if any other caller 2.6 // of GC_locker::lock() still needs GC locked. 2.7 - if (!is_active()) { 2.8 + if (!is_active_internal()) { 2.9 _doing_gc = true; 2.10 { 2.11 // Must give up the lock while at a safepoint
3.1 --- a/src/share/vm/memory/gcLocker.hpp Wed Sep 18 15:59:41 2013 +0200 3.2 +++ b/src/share/vm/memory/gcLocker.hpp Fri Sep 20 10:53:28 2013 +0200 3.3 @@ -88,7 +88,7 @@ 3.4 public: 3.5 // Accessors 3.6 static bool is_active() { 3.7 - assert(_needs_gc || SafepointSynchronize::is_at_safepoint(), "only read at safepoint"); 3.8 + assert(SafepointSynchronize::is_at_safepoint(), "only read at safepoint"); 3.9 return is_active_internal(); 3.10 } 3.11 static bool needs_gc() { return _needs_gc; }