8335409: Can't allocate and retain memory from resource area in frame::oops_interpreted_do oop closure after 8329665

src/hotspot/share/interpreter/oopMapCache.cpp

@@ -66,9 +66,6 @@ class OopMapCacheEntry: private InterpreterOopMap {
  public:
   OopMapCacheEntry() : InterpreterOopMap() {
     _next = nullptr;
-#ifdef ASSERT
-    _resource_allocate_bit_mask = false;
-#endif
   }
 };
 
@@ -178,10 +175,18 @@ class VerifyClosure : public OffsetClosure {
 InterpreterOopMap::InterpreterOopMap() {
   initialize();
 #ifdef ASSERT
-  _resource_allocate_bit_mask = true;
+  _used = false;
 #endif
 }
 
+InterpreterOopMap::~InterpreterOopMap() {
+  if (mask_size() > small_mask_limit) {
+    assert(!Thread::current()->resource_area()->contains((void*)_bit_mask[0]),
+           "The bit mask should be allocated from the C heap");
+    FREE_C_HEAP_ARRAY(uintptr_t, _bit_mask[0]);
+  }
+}
+
 bool InterpreterOopMap::is_empty() const {
   bool result = _method == nullptr;
   assert(_method != nullptr || (_bci == 0 &&
@@ -400,10 +405,10 @@ void OopMapCacheEntry::deallocate(OopMapCacheEntry* const entry) {
 // Implementation of OopMapCache
 
 void InterpreterOopMap::resource_copy(OopMapCacheEntry* from) {
-  assert(_resource_allocate_bit_mask,
-    "Should not resource allocate the _bit_mask");
-  assert(from->has_valid_mask(),
-    "Cannot copy entry with an invalid mask");
+  // The expectation is that this InterpreterOopMap is a recently created
+  // and empty. It is used to get a copy of a cached entry.
+  assert(!_used, "InterpreterOopMap object can only be filled once");
+  assert(from->has_valid_mask(), "Cannot copy entry with an invalid mask");
 
   set_method(from->method());
   set_bci(from->bci());
@@ -416,21 +421,11 @@ void InterpreterOopMap::resource_copy(OopMapCacheEntry* from) {
     memcpy((void *)_bit_mask, (void *)from->_bit_mask,
       mask_word_size() * BytesPerWord);
   } else {
-    // The expectation is that this InterpreterOopMap is a recently created
-    // and empty. It is used to get a copy of a cached entry.
-    // If the bit mask has a value, it should be in the
-    // resource area.
-    assert(_bit_mask[0] == 0 ||
-      Thread::current()->resource_area()->contains((void*)_bit_mask[0]),
-      "The bit mask should have been allocated from a resource area");
-    // Allocate the bit_mask from a Resource area for performance.  Allocating
-    // from the C heap as is done for OopMapCache has a significant
-    // performance impact.
-    _bit_mask[0] = (uintptr_t) NEW_RESOURCE_ARRAY(uintptr_t, mask_word_size());
+    _bit_mask[0] = (uintptr_t) NEW_C_HEAP_ARRAY(uintptr_t, mask_word_size(), mtClass);
     assert(_bit_mask[0] != 0, "bit mask was not allocated");
-    memcpy((void*) _bit_mask[0], (void*) from->_bit_mask[0],
-      mask_word_size() * BytesPerWord);
+    memcpy((void*) _bit_mask[0], (void*) from->_bit_mask[0], mask_word_size() * BytesPerWord);
   }
+  DEBUG_ONLY(_used = true);
 }
 
 inline unsigned int OopMapCache::hash_value_for(const methodHandle& method, int bci) const {

src/hotspot/share/interpreter/oopMapCache.hpp

@@ -36,12 +36,13 @@
 // OopMapCache's are allocated lazily per InstanceKlass.
 
 // The oopMap (InterpreterOopMap) is stored as a bit mask. If the
-// bit_mask can fit into two words it is stored in
+// bit_mask can fit into four words it is stored in
 // the _bit_mask array, otherwise it is allocated on the heap.
 // For OopMapCacheEntry the bit_mask is allocated in the C heap
 // because these entries persist between garbage collections.
-// For InterpreterOopMap the bit_mask is allocated in
-// a resource area for better performance.  InterpreterOopMap
+// For InterpreterOopMap the bit_mask is allocated in the C heap
+// to avoid issues with allocations from the resource area that have
+// to live accross the oop closure (see 8335409). InterpreterOopMap
 // should only be created and deleted during same garbage collection.
 //
 // If ENABBLE_ZAP_DEAD_LOCALS is defined, two bits are used
@@ -89,7 +90,7 @@ class InterpreterOopMap: ResourceObj {
 
  protected:
 #ifdef ASSERT
-  bool _resource_allocate_bit_mask;
+  bool           _used;
 #endif
   int            _num_oops;
   intptr_t       _bit_mask[N];    // the bit mask if
@@ -128,11 +129,12 @@ class InterpreterOopMap: ResourceObj {
 
  public:
   InterpreterOopMap();
+  ~InterpreterOopMap();
 
   // Copy the OopMapCacheEntry in parameter "from" into this
   // InterpreterOopMap.  If the _bit_mask[0] in "from" points to
   // allocated space (i.e., the bit mask was to large to hold
-  // in-line), allocate the space from a Resource area.
+  // in-line), allocate the space from the C heap.
   void resource_copy(OopMapCacheEntry* from);
 
   void iterate_oop(OffsetClosure* oop_closure) const;

src/hotspot/share/runtime/frame.cpp

@@ -947,7 +947,6 @@ void frame::oops_interpreted_do(OopClosure* f, const RegisterMap* map, bool quer
   InterpreterFrameClosure blk(this, max_locals, m->max_stack(), f);
 
   // process locals & expression stack
-  ResourceMark rm(thread);
   InterpreterOopMap mask;
   if (query_oop_map_cache) {
     m->mask_for(m, bci, &mask);