From 427aeb32e408e40d2c3ab01aa730477391a8fc3c Mon Sep 17 00:00:00 2001
From: stefank <unknown>
Date: Fri, 20 Sep 2013 10:53:28 +0200
Subject: [PATCH] 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 | 4 ++--
 src/share/vm/memory/gcLocker.cpp       | 2 +-
 src/share/vm/memory/gcLocker.hpp       | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/share/vm/classfile/symbolTable.cpp b/src/share/vm/classfile/symbolTable.cpp
index c00e95817..d0d33d878 100644
--- a/src/share/vm/classfile/symbolTable.cpp
+++ b/src/share/vm/classfile/symbolTable.cpp
@@ -341,7 +341,7 @@ Symbol* SymbolTable::new_permanent_symbol(const char* name, TRAPS) {
 
 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 @@ oop StringTable::intern(Handle string_or_null, jchar* name,
   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 --git a/src/share/vm/memory/gcLocker.cpp b/src/share/vm/memory/gcLocker.cpp
index eda728b70..1b0c94ee1 100644
--- a/src/share/vm/memory/gcLocker.cpp
+++ b/src/share/vm/memory/gcLocker.cpp
@@ -122,7 +122,7 @@ void GC_locker::jni_unlock(JavaThread* thread) {
     // 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 --git a/src/share/vm/memory/gcLocker.hpp b/src/share/vm/memory/gcLocker.hpp
index 8ede8fb27..45b8a8f89 100644
--- a/src/share/vm/memory/gcLocker.hpp
+++ b/src/share/vm/memory/gcLocker.hpp
@@ -88,7 +88,7 @@ class GC_locker: public AllStatic {
  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;                        }
-- 
GitLab