8207359: Make SymbolTable increment_refcount disallow zero
authorcoleenp
Fri, 20 Jul 2018 14:52:11 -0400
changeset 51179 516acf6956a2
parent 51178 416a76fe8067
child 51180 b7eb9cc56277
8207359: Make SymbolTable increment_refcount disallow zero Summary: Use cmpxchg for non permanent symbol refcounting, and pack refcount and length into an int. Reviewed-by: gziemski, kbarrett, iklam
make/hotspot/src/native/dtrace/generateJvmOffsets.cpp
src/hotspot/os/solaris/dtrace/jhelper.d
src/hotspot/share/classfile/compactHashtable.inline.hpp
src/hotspot/share/classfile/symbolTable.cpp
src/hotspot/share/classfile/systemDictionary.cpp
src/hotspot/share/oops/symbol.cpp
src/hotspot/share/oops/symbol.hpp
src/hotspot/share/runtime/vmStructs.cpp
src/java.base/solaris/native/libjvm_db/libjvm_db.c
src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/Symbol.java
test/hotspot/gtest/classfile/test_symbolTable.cpp
test/hotspot/gtest/threadHelper.inline.hpp
test/hotspot/gtest/utilities/test_concurrentHashtable.cpp
test/hotspot/gtest/utilities/test_globalCounter.cpp
test/hotspot/gtest/utilities/utilitiesHelper.inline.hpp
--- a/make/hotspot/src/native/dtrace/generateJvmOffsets.cpp	Fri Jul 20 11:55:05 2018 -0700
+++ b/make/hotspot/src/native/dtrace/generateJvmOffsets.cpp	Fri Jul 20 14:52:11 2018 -0400
@@ -212,7 +212,7 @@
   GEN_VALUE(AccessFlags_NATIVE, JVM_ACC_NATIVE);
   GEN_VALUE(ConstMethod_has_linenumber_table, ConstMethod::_has_linenumber_table);
   GEN_OFFS(AccessFlags, _flags);
-  GEN_OFFS(Symbol, _length);
+  GEN_OFFS(Symbol, _length_and_refcount);
   GEN_OFFS(Symbol, _body);
   printf("\n");
 
--- a/src/hotspot/os/solaris/dtrace/jhelper.d	Fri Jul 20 11:55:05 2018 -0700
+++ b/src/hotspot/os/solaris/dtrace/jhelper.d	Fri Jul 20 14:52:11 2018 -0400
@@ -111,7 +111,7 @@
   copyin_offset(OFFSET_HeapBlockHeader_used);
   copyin_offset(OFFSET_oopDesc_metadata);
 
-  copyin_offset(OFFSET_Symbol_length);
+  copyin_offset(OFFSET_Symbol_length_and_refcount);
   copyin_offset(OFFSET_Symbol_body);
 
   copyin_offset(OFFSET_Method_constMethod);
@@ -463,15 +463,17 @@
   /* The symbol is a CPSlot and has lower bit set to indicate metadata */
   this->nameSymbol &= (~1); /* remove metadata lsb */
 
+  /* Because sparc is big endian, the top half length is at the correct offset. */
   this->nameSymbolLength = copyin_uint16(this->nameSymbol +
-      OFFSET_Symbol_length);
+      OFFSET_Symbol_length_and_refcount);
 
   this->signatureSymbol = copyin_ptr(this->constantPool +
       this->signatureIndex * sizeof (pointer) + SIZE_ConstantPool);
   this->signatureSymbol &= (~1); /* remove metadata lsb */
 
+  /* Because sparc is big endian, the top half length is at the correct offset. */
   this->signatureSymbolLength = copyin_uint16(this->signatureSymbol +
-      OFFSET_Symbol_length);
+      OFFSET_Symbol_length_and_refcount);
 
   this->klassPtr = copyin_ptr(this->constantPool +
       OFFSET_ConstantPool_pool_holder);
@@ -479,8 +481,9 @@
   this->klassSymbol = copyin_ptr(this->klassPtr +
       OFFSET_Klass_name);
 
+  /* Because sparc is big endian, the top half length is at the correct offset. */
   this->klassSymbolLength = copyin_uint16(this->klassSymbol +
-      OFFSET_Symbol_length);
+      OFFSET_Symbol_length_and_refcount);
 
   /*
    * Enough for three strings, plus the '.', plus the trailing '\0'.
--- a/src/hotspot/share/classfile/compactHashtable.inline.hpp	Fri Jul 20 11:55:05 2018 -0700
+++ b/src/hotspot/share/classfile/compactHashtable.inline.hpp	Fri Jul 20 14:52:11 2018 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -36,7 +36,7 @@
                                                     u4 offset, const char* name, int len) {
   Symbol* sym = (Symbol*)(_base_address + offset);
   if (sym->equals(name, len)) {
-    assert(sym->refcount() == -1, "must be shared");
+    assert(sym->refcount() == PERM_REFCOUNT, "must be shared");
     return sym;
   }
 
--- a/src/hotspot/share/classfile/symbolTable.cpp	Fri Jul 20 11:55:05 2018 -0700
+++ b/src/hotspot/share/classfile/symbolTable.cpp	Fri Jul 20 14:52:11 2018 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2018, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -190,6 +190,7 @@
   assert(SafepointSynchronize::is_at_safepoint(), "must be at safepoint");
   // This should never happen with -Xshare:dump but it might in testing mode.
   if (DumpSharedSpaces) return;
+
   // Create a new symbol table
   SymbolTable* new_table = new SymbolTable();
 
@@ -212,10 +213,16 @@
     count++;  // count all entries in this bucket, not just ones with same hash
     if (e->hash() == hash) {
       Symbol* sym = e->literal();
-      if (sym->equals(name, len)) {
-        // something is referencing this symbol now.
-        sym->increment_refcount();
-        return sym;
+      // Skip checking already dead symbols in the bucket.
+      if (sym->refcount() == 0) {
+        count--;   // Don't count this symbol towards rehashing.
+      } else if (sym->equals(name, len)) {
+        if (sym->try_increment_refcount()) {
+          // something is referencing this symbol now.
+          return sym;
+        } else {
+          count--;   // don't count this symbol.
+        }
       }
     }
   }
--- a/src/hotspot/share/classfile/systemDictionary.cpp	Fri Jul 20 11:55:05 2018 -0700
+++ b/src/hotspot/share/classfile/systemDictionary.cpp	Fri Jul 20 14:52:11 2018 -0400
@@ -2226,12 +2226,14 @@
   ClassLoaderData* loader_data2 = class_loader_data(class_loader2);
 
   Symbol* constraint_name = NULL;
+  // Needs to be in same scope as constraint_name in case a Symbol is created and
+  // assigned to constraint_name.
+  FieldArrayInfo fd;
   if (!FieldType::is_array(class_name)) {
     constraint_name = class_name;
   } else {
     // For array classes, their Klass*s are not kept in the
     // constraint table. The element classes are.
-    FieldArrayInfo fd;
     BasicType t = FieldType::get_array_info(class_name, fd, CHECK_(false));
     // primitive types always pass
     if (t != T_OBJECT) {
--- a/src/hotspot/share/oops/symbol.cpp	Fri Jul 20 11:55:05 2018 -0700
+++ b/src/hotspot/share/oops/symbol.cpp	Fri Jul 20 14:52:11 2018 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2018, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -34,11 +34,22 @@
 #include "runtime/atomic.hpp"
 #include "runtime/os.hpp"
 
+uint32_t Symbol::pack_length_and_refcount(int length, int refcount) {
+  STATIC_ASSERT(max_symbol_length == ((1 << 16) - 1));
+  STATIC_ASSERT(PERM_REFCOUNT == ((1 << 16) - 1));
+  assert(length >= 0, "negative length");
+  assert(length <= max_symbol_length, "too long symbol");
+  assert(refcount >= 0, "negative refcount");
+  assert(refcount <= PERM_REFCOUNT, "invalid refcount");
+  uint32_t hi = length;
+  uint32_t lo = refcount;
+  return (hi << 16) | lo;
+}
+
 Symbol::Symbol(const u1* name, int length, int refcount) {
-  _refcount = refcount;
-  _length = length;
+  _length_and_refcount =  pack_length_and_refcount(length, refcount);
   _identity_hash = (short)os::random();
-  for (int i = 0; i < _length; i++) {
+  for (int i = 0; i < length; i++) {
     byte_at_put(i, name[i]);
   }
 }
@@ -207,26 +218,68 @@
   return AltHashing::murmur3_32(seed, (const jbyte*)as_C_string(), utf8_length());
 }
 
-void Symbol::increment_refcount() {
-  // Only increment the refcount if non-negative.  If negative either
-  // overflow has occurred or it is a permanent symbol in a read only
-  // shared archive.
-  if (_refcount >= 0) { // not a permanent symbol
-    Atomic::inc(&_refcount);
-    NOT_PRODUCT(Atomic::inc(&_total_count);)
+// Increment refcount while checking for zero.  If the Symbol's refcount becomes zero
+// a thread could be concurrently removing the Symbol.  This is used during SymbolTable
+// lookup to avoid reviving a dead Symbol.
+bool Symbol::try_increment_refcount() {
+  uint32_t found = _length_and_refcount;
+  while (true) {
+    uint32_t old_value = found;
+    int refc = extract_refcount(old_value);
+    if (refc == PERM_REFCOUNT) {
+      return true;  // sticky max or created permanent
+    } else if (refc == 0) {
+      return false; // dead, can't revive.
+    } else {
+      found = Atomic::cmpxchg(old_value + 1, &_length_and_refcount, old_value);
+      if (found == old_value) {
+        return true; // successfully updated.
+      }
+      // refcount changed, try again.
+    }
   }
 }
 
-void Symbol::decrement_refcount() {
-  if (_refcount >= 0) { // not a permanent symbol
-    short new_value = Atomic::add(short(-1), &_refcount);
+// The increment_refcount() is called when not doing lookup. It is assumed that you
+// have a symbol with a non-zero refcount and it can't become zero while referenced by
+// this caller.
+void Symbol::increment_refcount() {
+  if (!try_increment_refcount()) {
 #ifdef ASSERT
-    if (new_value == -1) { // we have transitioned from 0 -> -1
+    print();
+    fatal("refcount has gone to zero");
+#endif
+  }
+#ifndef PRODUCT
+  if (refcount() != PERM_REFCOUNT) { // not a permanent symbol
+    NOT_PRODUCT(Atomic::inc(&_total_count);)
+  }
+#endif
+}
+
+// Decrement refcount potentially while racing increment, so we need
+// to check the value after attempting to decrement so that if another
+// thread increments to PERM_REFCOUNT the value is not decremented.
+void Symbol::decrement_refcount() {
+  uint32_t found = _length_and_refcount;
+  while (true) {
+    uint32_t old_value = found;
+    int refc = extract_refcount(old_value);
+    if (refc == PERM_REFCOUNT) {
+      return;  // refcount is permanent, permanent is sticky
+    } else if (refc == 0) {
+#ifdef ASSERT
       print();
-      assert(false, "reference count underflow for symbol");
+      fatal("refcount underflow");
+#endif
+      return;
+    } else {
+      found = Atomic::cmpxchg(old_value - 1, &_length_and_refcount, old_value);
+      if (found == old_value) {
+        return;  // successfully updated.
+      }
+      // refcount changed, try again.
     }
-#endif
-    (void)new_value;
   }
 }
 
--- a/src/hotspot/share/oops/symbol.hpp	Fri Jul 20 11:55:05 2018 -0700
+++ b/src/hotspot/share/oops/symbol.hpp	Fri Jul 20 14:52:11 2018 -0400
@@ -96,9 +96,9 @@
 // type without virtual functions.
 class ClassLoaderData;
 
-// Set _refcount to PERM_REFCOUNT to prevent the Symbol from being GC'ed.
+// Set _refcount to PERM_REFCOUNT to prevent the Symbol from being freed.
 #ifndef PERM_REFCOUNT
-#define PERM_REFCOUNT -1
+#define PERM_REFCOUNT ((1 << 16) - 1)
 #endif
 
 class Symbol : public MetaspaceObj {
@@ -107,15 +107,15 @@
   friend class MoveSymbols;
 
  private:
-  ATOMIC_SHORT_PAIR(
-    volatile short _refcount,  // needs atomic operation
-    unsigned short _length     // number of UTF8 characters in the symbol (does not need atomic op)
-  );
+
+  // This is an int because it needs atomic operation on the refcount.  Mask length
+  // in high half word. length is the number of UTF8 characters in the symbol
+  volatile uint32_t _length_and_refcount;
   short _identity_hash;
   jbyte _body[2];
 
   enum {
-    // max_symbol_length is constrained by type of _length
+    // max_symbol_length must fit into the top 16 bits of _length_and_refcount
     max_symbol_length = (1 << 16) -1
   };
 
@@ -129,7 +129,7 @@
   }
 
   void byte_at_put(int index, int value) {
-    assert(index >=0 && index < _length, "symbol index overflow");
+    assert(index >=0 && index < length(), "symbol index overflow");
     _body[index] = value;
   }
 
@@ -140,6 +140,12 @@
 
   void  operator delete(void* p);
 
+  static int extract_length(uint32_t value)   { return value >> 16; }
+  static int extract_refcount(uint32_t value) { return value & 0xffff; }
+  static uint32_t pack_length_and_refcount(int length, int refcount);
+
+  int length() const   { return extract_length(_length_and_refcount); }
+
  public:
   // Low-level access (used with care, since not GC-safe)
   const jbyte* base() const { return &_body[0]; }
@@ -155,28 +161,29 @@
   unsigned identity_hash() const {
     unsigned addr_bits = (unsigned)((uintptr_t)this >> (LogMinObjAlignmentInBytes + 3));
     return ((unsigned)_identity_hash & 0xffff) |
-           ((addr_bits ^ (_length << 8) ^ (( _body[0] << 8) | _body[1])) << 16);
+           ((addr_bits ^ (length() << 8) ^ (( _body[0] << 8) | _body[1])) << 16);
   }
 
   // For symbol table alternate hashing
   unsigned int new_hash(juint seed);
 
   // Reference counting.  See comments above this class for when to use.
-  int refcount() const      { return _refcount; }
+  int refcount() const { return extract_refcount(_length_and_refcount); }
+  bool try_increment_refcount();
   void increment_refcount();
   void decrement_refcount();
   bool is_permanent() {
-    return (_refcount == PERM_REFCOUNT);
+    return (refcount() == PERM_REFCOUNT);
   }
 
   int byte_at(int index) const {
-    assert(index >=0 && index < _length, "symbol index overflow");
+    assert(index >=0 && index < length(), "symbol index overflow");
     return base()[index];
   }
 
   const jbyte* bytes() const { return base(); }
 
-  int utf8_length() const { return _length; }
+  int utf8_length() const { return length(); }
 
   // Compares the symbol with a string.
   bool equals(const char* str, int len) const {
--- a/src/hotspot/share/runtime/vmStructs.cpp	Fri Jul 20 11:55:05 2018 -0700
+++ b/src/hotspot/share/runtime/vmStructs.cpp	Fri Jul 20 14:52:11 2018 -0400
@@ -330,9 +330,8 @@
   nonstatic_field(ConstMethod,                 _size_of_parameters,                           u2)                                    \
   nonstatic_field(ObjArrayKlass,               _element_klass,                                Klass*)                                \
   nonstatic_field(ObjArrayKlass,               _bottom_klass,                                 Klass*)                                \
-  volatile_nonstatic_field(Symbol,             _refcount,                                     short)                                 \
+  volatile_nonstatic_field(Symbol,             _length_and_refcount,                          unsigned int)                          \
   nonstatic_field(Symbol,                      _identity_hash,                                short)                                 \
-  nonstatic_field(Symbol,                      _length,                                       unsigned short)                        \
   unchecked_nonstatic_field(Symbol,            _body,                                         sizeof(jbyte)) /* NOTE: no type */     \
   nonstatic_field(Symbol,                      _body[0],                                      jbyte)                                 \
   nonstatic_field(TypeArrayKlass,              _max_length,                                   jint)                                  \
--- a/src/java.base/solaris/native/libjvm_db/libjvm_db.c	Fri Jul 20 11:55:05 2018 -0700
+++ b/src/java.base/solaris/native/libjvm_db/libjvm_db.c	Fri Jul 20 14:52:11 2018 -0400
@@ -552,7 +552,8 @@
   CHECK_FAIL(err);
   // The symbol is a CPSlot and has lower bit set to indicate metadata
   nameSymbol &= (~1); // remove metadata lsb
-  err = ps_pread(J->P, nameSymbol + OFFSET_Symbol_length, &nameSymbolLength, 2);
+  // The length is in the top half of the word.
+  err = ps_pread(J->P, nameSymbol + OFFSET_Symbol_length_and_refcount, &nameSymbolLength, 2);
   CHECK_FAIL(err);
   nameString = (char*)calloc(nameSymbolLength + 1, 1);
   err = ps_pread(J->P, nameSymbol + OFFSET_Symbol_body, nameString, nameSymbolLength);
@@ -564,7 +565,7 @@
   err = read_pointer(J, constantPool + signatureIndex * POINTER_SIZE + SIZE_ConstantPool, &signatureSymbol);
   CHECK_FAIL(err);
   signatureSymbol &= (~1);  // remove metadata lsb
-  err = ps_pread(J->P, signatureSymbol + OFFSET_Symbol_length, &signatureSymbolLength, 2);
+  err = ps_pread(J->P, signatureSymbol + OFFSET_Symbol_length_and_refcount, &signatureSymbolLength, 2);
   CHECK_FAIL(err);
   signatureString = (char*)calloc(signatureSymbolLength + 1, 1);
   err = ps_pread(J->P, signatureSymbol + OFFSET_Symbol_body, signatureString, signatureSymbolLength);
@@ -575,7 +576,7 @@
   CHECK_FAIL(err);
   err = read_pointer(J, klassPtr + OFFSET_Klass_name, &klassSymbol);
   CHECK_FAIL(err);
-  err = ps_pread(J->P, klassSymbol + OFFSET_Symbol_length, &klassSymbolLength, 2);
+  err = ps_pread(J->P, klassSymbol + OFFSET_Symbol_length_and_refcount, &klassSymbolLength, 2);
   CHECK_FAIL(err);
   klassString = (char*)calloc(klassSymbolLength + 1, 1);
   err = ps_pread(J->P, klassSymbol + OFFSET_Symbol_body, klassString, klassSymbolLength);
--- a/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/Symbol.java	Fri Jul 20 11:55:05 2018 -0700
+++ b/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/Symbol.java	Fri Jul 20 14:52:11 2018 -0400
@@ -45,17 +45,11 @@
 
   private static synchronized void initialize(TypeDataBase db) throws WrongTypeException {
     Type type  = db.lookupType("Symbol");
-    length     = type.getCIntegerField("_length");
+    lengthAndRefcount = type.getCIntegerField("_length_and_refcount");
     baseOffset = type.getField("_body").getOffset();
     idHash = type.getCIntegerField("_identity_hash");
   }
 
-  // Format:
-  //   [header]
-  //   [klass ]
-  //   [length] byte size of uft8 string
-  //   ..body..
-
   public static Symbol create(Address addr) {
     if (addr == null) {
       return null;
@@ -72,10 +66,13 @@
   private static long baseOffset; // tells where the array part starts
 
   // Fields
-  private static CIntegerField length;
+  private static CIntegerField lengthAndRefcount;
 
   // Accessors for declared fields
-  public long   getLength() { return          length.getValue(this.addr); }
+  public long getLength() {
+    long i = lengthAndRefcount.getValue(this.addr);
+    return (i >> 16) & 0xffff;
+  }
 
   public byte getByteAt(long index) {
     return addr.getJByteAt(baseOffset + index);
--- a/test/hotspot/gtest/classfile/test_symbolTable.cpp	Fri Jul 20 11:55:05 2018 -0700
+++ b/test/hotspot/gtest/classfile/test_symbolTable.cpp	Fri Jul 20 14:52:11 2018 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -24,6 +24,7 @@
 #include "precompiled.hpp"
 #include "runtime/interfaceSupport.inline.hpp"
 #include "classfile/symbolTable.hpp"
+#include "threadHelper.inline.hpp"
 #include "unittest.hpp"
 
 TEST_VM(SymbolTable, temp_new_symbol) {
@@ -74,4 +75,76 @@
   }
   ASSERT_EQ(xyz->refcount(), xyzcount - 1)
           << "Should have been decremented by dtor in inner scope";
+
+  // Test overflowing refcount making symbol permanent
+  Symbol* bigsym = SymbolTable::new_symbol("bigsym", CATCH);
+  for (int i = 0; i < PERM_REFCOUNT + 100; i++) {
+    bigsym->increment_refcount();
+  }
+  ASSERT_EQ(bigsym->refcount(), PERM_REFCOUNT) << "should not have overflowed";
+
+  // Test that PERM_REFCOUNT is sticky
+  for (int i = 0; i < 10; i++) {
+    bigsym->decrement_refcount();
+  }
+  ASSERT_EQ(bigsym->refcount(), PERM_REFCOUNT) << "should be sticky";
 }
+
+// TODO: Make two threads one decrementing the refcount and the other trying to increment.
+// try_increment_refcount should return false
+
+#define SYM_NAME_LENGTH 30
+static char symbol_name[SYM_NAME_LENGTH];
+
+class SymbolThread : public JavaTestThread {
+  public:
+  SymbolThread(Semaphore* post) : JavaTestThread(post) {}
+  virtual ~SymbolThread() {}
+  void main_run() {
+    Thread* THREAD = Thread::current();
+    for (int i = 0; i < 1000; i++) {
+      TempNewSymbol sym = SymbolTable::new_symbol(symbol_name, CATCH);
+      // Create and destroy new symbol
+      EXPECT_TRUE(sym->refcount() != 0) << "Symbol refcount unexpectedly zeroed";
+    }
+  }
+};
+
+#define SYM_TEST_THREAD_COUNT 5
+
+class DriverSymbolThread : public JavaTestThread {
+public:
+  Semaphore _done;
+  DriverSymbolThread(Semaphore* post) : JavaTestThread(post) { };
+  virtual ~DriverSymbolThread(){}
+
+  void main_run() {
+    Semaphore done(0);
+
+    Thread* THREAD = Thread::current();
+
+    // Find a symbol where there will probably be only one instance.
+    for (int i = 0; i < 100; i++) {
+       snprintf(symbol_name, SYM_NAME_LENGTH, "some_symbol%d", i);
+       TempNewSymbol ts = SymbolTable::new_symbol(symbol_name, CATCH);
+       if (ts->refcount() == 1) {
+         EXPECT_TRUE(ts->refcount() == 1) << "Symbol is just created";
+         break;  // found a unique symbol
+       }
+    }
+
+    SymbolThread* st[SYM_TEST_THREAD_COUNT];
+    for (int i = 0; i < SYM_TEST_THREAD_COUNT; i++) {
+      st[i] = new SymbolThread(&done);
+      st[i]->doit();
+    }
+
+    for (int i = 0; i < 4; i++) {
+      done.wait();
+    }
+  }
+};
+
+TEST_VM(SymbolTable, test_symbol_refcount_parallel) {
+  mt_test_doer<DriverSymbolThread>();
+}
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/hotspot/gtest/threadHelper.inline.hpp	Fri Jul 20 14:52:11 2018 -0400
@@ -0,0 +1,155 @@
+/*
+ * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+#ifndef GTEST_THREADHELPER_INLINE_HPP
+#define GTEST_THREADHELPER_INLINE_HPP
+
+#include "runtime/mutex.hpp"
+#include "runtime/semaphore.hpp"
+#include "runtime/thread.hpp"
+#include "runtime/vmThread.hpp"
+#include "runtime/vm_operations.hpp"
+#include "unittest.hpp"
+
+class VM_StopSafepoint : public VM_Operation {
+public:
+  Semaphore* _test_complete;
+  VM_StopSafepoint(Semaphore* wait_for) : _test_complete(wait_for) {}
+  VMOp_Type type() const          { return VMOp_None; }
+  Mode evaluation_mode() const    { return _no_safepoint; }
+  bool is_cheap_allocated() const { return false; }
+  void doit()                     { _test_complete->wait(); }
+};
+
+// This class and thread keep the non-safepoint op running while we do our testing.
+class VMThreadBlocker : public JavaThread {
+public:
+  Semaphore* _unblock;
+  Semaphore* _done;
+  VMThreadBlocker(Semaphore* ub, Semaphore* done) : _unblock(ub), _done(done) {
+  }
+  virtual ~VMThreadBlocker() {}
+  void run() {
+    this->set_thread_state(_thread_in_vm);
+    {
+      MutexLocker ml(Threads_lock);
+      Threads::add(this);
+    }
+    VM_StopSafepoint ss(_unblock);
+    VMThread::execute(&ss);
+    _done->signal();
+    Threads::remove(this);
+    this->smr_delete();
+  }
+  void doit() {
+    if (os::create_thread(this, os::os_thread)) {
+      os::start_thread(this);
+    } else {
+      ASSERT_TRUE(false);
+    }
+  }
+};
+
+// For testing in a real JavaThread.
+class JavaTestThread : public JavaThread {
+public:
+  Semaphore* _post;
+  JavaTestThread(Semaphore* post)
+    : _post(post) {
+  }
+  virtual ~JavaTestThread() {}
+
+  void prerun() {
+    this->set_thread_state(_thread_in_vm);
+    {
+      MutexLocker ml(Threads_lock);
+      Threads::add(this);
+    }
+    {
+      MutexLockerEx ml(SR_lock(), Mutex::_no_safepoint_check_flag);
+    }
+  }
+
+  virtual void main_run() = 0;
+
+  void run() {
+    prerun();
+    main_run();
+    postrun();
+  }
+
+  void postrun() {
+    Threads::remove(this);
+    _post->signal();
+    this->smr_delete();
+  }
+
+  void doit() {
+    if (os::create_thread(this, os::os_thread)) {
+      os::start_thread(this);
+    } else {
+      ASSERT_TRUE(false);
+    }
+  }
+};
+
+template <typename FUNC>
+class SingleTestThread : public JavaTestThread {
+public:
+  FUNC& _f;
+  SingleTestThread(Semaphore* post, FUNC& f)
+    : JavaTestThread(post), _f(f) {
+  }
+
+  virtual ~SingleTestThread(){}
+
+  virtual void main_run() {
+    _f(this);
+  }
+};
+
+template <typename TESTFUNC>
+static void nomt_test_doer(TESTFUNC &f) {
+  Semaphore post, block_done, vmt_done;
+  VMThreadBlocker* blocker = new VMThreadBlocker(&block_done, &vmt_done);
+  blocker->doit();
+  SingleTestThread<TESTFUNC>* stt = new SingleTestThread<TESTFUNC>(&post, f);
+  stt->doit();
+  post.wait();
+  block_done.signal();
+  vmt_done.wait();
+}
+
+template <typename RUNNER>
+static void mt_test_doer() {
+  Semaphore post, block_done, vmt_done;
+  VMThreadBlocker* blocker = new VMThreadBlocker(&block_done, &vmt_done);
+  blocker->doit();
+  RUNNER* runner = new RUNNER(&post);
+  runner->doit();
+  post.wait();
+  block_done.signal();
+  vmt_done.wait();
+}
+
+#endif // include guard
--- a/test/hotspot/gtest/utilities/test_concurrentHashtable.cpp	Fri Jul 20 11:55:05 2018 -0700
+++ b/test/hotspot/gtest/utilities/test_concurrentHashtable.cpp	Fri Jul 20 14:52:11 2018 -0400
@@ -29,7 +29,7 @@
 #include "runtime/vm_operations.hpp"
 #include "utilities/concurrentHashTable.inline.hpp"
 #include "utilities/concurrentHashTableTasks.inline.hpp"
-#include "utilitiesHelper.inline.hpp"
+#include "threadHelper.inline.hpp"
 #include "unittest.hpp"
 
 // NOTE: On win32 gtest asserts are not mt-safe.
--- a/test/hotspot/gtest/utilities/test_globalCounter.cpp	Fri Jul 20 11:55:05 2018 -0700
+++ b/test/hotspot/gtest/utilities/test_globalCounter.cpp	Fri Jul 20 14:52:11 2018 -0400
@@ -27,7 +27,7 @@
 #include "runtime/os.hpp"
 #include "utilities/globalCounter.hpp"
 #include "utilities/globalCounter.inline.hpp"
-#include "utilitiesHelper.inline.hpp"
+#include "threadHelper.inline.hpp"
 
 #define GOOD_VALUE 1337
 #define BAD_VALUE  4711
--- a/test/hotspot/gtest/utilities/utilitiesHelper.inline.hpp	Fri Jul 20 11:55:05 2018 -0700
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,155 +0,0 @@
-/*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
- * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
- *
- * This code is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License version 2 only, as
- * published by the Free Software Foundation.
- *
- * This code is distributed in the hope that it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
- * version 2 for more details (a copy is included in the LICENSE file that
- * accompanied this code).
- *
- * You should have received a copy of the GNU General Public License version
- * 2 along with this work; if not, write to the Free Software Foundation,
- * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
- *
- * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
- * or visit www.oracle.com if you need additional information or have any
- * questions.
- */
-
-#ifndef GTEST_UTILITIES_HELPER_INLINE_HPP
-#define GTEST_UTILITIES_HELPER_INLINE_HPP
-
-#include "runtime/mutex.hpp"
-#include "runtime/semaphore.hpp"
-#include "runtime/thread.hpp"
-#include "runtime/vmThread.hpp"
-#include "runtime/vm_operations.hpp"
-#include "unittest.hpp"
-
-class VM_StopSafepoint : public VM_Operation {
-public:
-  Semaphore* _test_complete;
-  VM_StopSafepoint(Semaphore* wait_for) : _test_complete(wait_for) {}
-  VMOp_Type type() const          { return VMOp_None; }
-  Mode evaluation_mode() const    { return _no_safepoint; }
-  bool is_cheap_allocated() const { return false; }
-  void doit()                     { _test_complete->wait(); }
-};
-
-// This class and thread keep the non-safepoint op running while we do our testing.
-class VMThreadBlocker : public JavaThread {
-public:
-  Semaphore* _unblock;
-  Semaphore* _done;
-  VMThreadBlocker(Semaphore* ub, Semaphore* done) : _unblock(ub), _done(done) {
-  }
-  virtual ~VMThreadBlocker() {}
-  void run() {
-    this->set_thread_state(_thread_in_vm);
-    {
-      MutexLocker ml(Threads_lock);
-      Threads::add(this);
-    }
-    VM_StopSafepoint ss(_unblock);
-    VMThread::execute(&ss);
-    _done->signal();
-    Threads::remove(this);
-    this->smr_delete();
-  }
-  void doit() {
-    if (os::create_thread(this, os::os_thread)) {
-      os::start_thread(this);
-    } else {
-      ASSERT_TRUE(false);
-    }
-  }
-};
-
-// For testing in a real JavaThread.
-class JavaTestThread : public JavaThread {
-public:
-  Semaphore* _post;
-  JavaTestThread(Semaphore* post)
-    : _post(post) {
-  }
-  virtual ~JavaTestThread() {}
-
-  void prerun() {
-    this->set_thread_state(_thread_in_vm);
-    {
-      MutexLocker ml(Threads_lock);
-      Threads::add(this);
-    }
-    {
-      MutexLockerEx ml(SR_lock(), Mutex::_no_safepoint_check_flag);
-    }
-  }
-
-  virtual void main_run() = 0;
-
-  void run() {
-    prerun();
-    main_run();
-    postrun();
-  }
-
-  void postrun() {
-    Threads::remove(this);
-    _post->signal();
-    this->smr_delete();
-  }
-
-  void doit() {
-    if (os::create_thread(this, os::os_thread)) {
-      os::start_thread(this);
-    } else {
-      ASSERT_TRUE(false);
-    }
-  }
-};
-
-template <typename FUNC>
-class SingleTestThread : public JavaTestThread {
-public:
-  FUNC& _f;
-  SingleTestThread(Semaphore* post, FUNC& f)
-    : JavaTestThread(post), _f(f) {
-  }
-
-  virtual ~SingleTestThread(){}
-
-  virtual void main_run() {
-    _f(this);
-  }
-};
-
-template <typename TESTFUNC>
-static void nomt_test_doer(TESTFUNC &f) {
-  Semaphore post, block_done, vmt_done;
-  VMThreadBlocker* blocker = new VMThreadBlocker(&block_done, &vmt_done);
-  blocker->doit();
-  SingleTestThread<TESTFUNC>* stt = new SingleTestThread<TESTFUNC>(&post, f);
-  stt->doit();
-  post.wait();
-  block_done.signal();
-  vmt_done.wait();
-}
-
-template <typename RUNNER>
-static void mt_test_doer() {
-  Semaphore post, block_done, vmt_done;
-  VMThreadBlocker* blocker = new VMThreadBlocker(&block_done, &vmt_done);
-  blocker->doit();
-  RUNNER* runner = new RUNNER(&post);
-  runner->doit();
-  post.wait();
-  block_done.signal();
-  vmt_done.wait();
-}
-
-#endif // include guard