# HG changeset patch
# User stefank
# Date 1379667208 -7200
# Node ID 2c022e432e102a0b6a095a3f2b9b62f92f99153f
# Parent  719e886d4f726f1bdde8592b928b6583b0a555f7
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

diff -r 719e886d4f72 -r 2c022e432e10 src/share/vm/classfile/symbolTable.cpp
--- 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;
diff -r 719e886d4f72 -r 2c022e432e10 src/share/vm/memory/gcLocker.cpp
--- 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
diff -r 719e886d4f72 -r 2c022e432e10 src/share/vm/memory/gcLocker.hpp
--- 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;                        }