8024974: Incorrect use of GC_locker::is_active()

Fri, 20 Sep 2013 10:53:28 +0200

author
stefank
date
Fri, 20 Sep 2013 10:53:28 +0200
changeset 5769
2c022e432e10
parent 5719
719e886d4f72
child 5770
9361de86a50f

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

src/share/vm/classfile/symbolTable.cpp file | annotate | diff | comparison | revisions
src/share/vm/memory/gcLocker.cpp file | annotate | diff | comparison | revisions
src/share/vm/memory/gcLocker.hpp file | annotate | diff | comparison | revisions
     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;                        }

mercurial