Mercurial > hg > graal-compiler
changeset 12301:2c022e432e10
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
author | stefank |
---|---|
date | Fri, 20 Sep 2013 10:53:28 +0200 |
parents | 719e886d4f72 |
children | 9361de86a50f |
files | src/share/vm/classfile/symbolTable.cpp src/share/vm/memory/gcLocker.cpp src/share/vm/memory/gcLocker.hpp |
diffstat | 3 files changed, 4 insertions(+), 4 deletions(-) [+] |
line wrap: on
line diff
--- a/src/share/vm/classfile/symbolTable.cpp Wed Sep 18 15:59:41 2013 +0200 +++ b/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/src/share/vm/memory/gcLocker.cpp Wed Sep 18 15:59:41 2013 +0200 +++ b/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/src/share/vm/memory/gcLocker.hpp Wed Sep 18 15:59:41 2013 +0200 +++ b/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; }