JDK-4630632 : ResourceObj::operator new(size_t, allocation_type) leaks memory
  • Type: Bug
  • Component: hotspot
  • Sub-Component: gc
  • Affected Version: 2.0
  • Priority: P4
  • Status: Closed
  • Resolution: Duplicate
  • OS: generic
  • CPU: generic
  • Submitted: 2002-01-30
  • Updated: 2005-09-27
  • Resolved: 2005-09-27
Related Reports
Duplicate :  
JDK-6306741 - Memory leaks of C-heap allocated ResourceObjs
Description
ResourceObj's can be allocated on the C heap by calling

    new(ResourceObj::C_HEAP) Foo(...)

Unfortunately, there is then no way to recover the space that is 
allocated, because ResourceObj::operator delete(void*) is a noop.

###@###.### 2002-01-30
Comments
EVALUATION As part of the below bug fixes, the delete operator was implemented for C_HEAP allocated resource objects. The delete operator will call FreeHeap on the object to deallocate memory. For GrowableArrays the destructor will call clear_and_deallocate() to free memory for the elements, then 'delete' calls FreeHeap() to clean the whole thing up. So you can have destructors for ResourceObjs if they are allocated on C_HEAP. If a ResourceObj is allocated in the resource area or an arena, the object cannot have a destructor or call the delete operator. There isn't an automatic way to assert the former, although the latter has an assertion. This is still a wart in this scheme. All of the resource objs were manually checked to make sure they didn't have any nontrivial destructors, so this isn't a bug at the current time. Not sure how to close this bug (fixed but not in putback message so won't get marked as integrated). I'm going to close this as a duplicate of a later bug, although this isn't really the way we usually do that. Fixed 6306746: Add assertions to GrowableArray and ResourceObj Fixed 6306730: Incorrect management of C-heap GrowableArrays (gc) Fixed 6306736: Incorrect management of C-heap (jvmti) Fixed 6306741: Memory leaks of C-heap allocated ResourceObjs (gc) Fixed 6306743: Memory leaks of C-heap allocated ResourceObjs (jvmti) Fixed 6306745: Memory leaks of C-heap allocated ResourceObjs (runtime) Fixed 6306738: Memory leaks of C-heap allocated ResourceObjs (c2) Require GrowableArrays whose elements are allocated on the C heap to also be allocated on the C heap. This prevents leaking the GrowableArray object itself or worse, having it go out of scope of it's containing ResourceMark. Which is usually why C_heap was used for the arguments themselves. I added a field to ResourceObj to track whether new allocates on c_heap, resource area, or arena, and implemented the ResourceObj::operator delete to call FreeHeap() if the allocation was on C heap. An assert was added to the delete() operator to check that delete can only be called on a C_HEAP allocated object, because it's not needed for resource and arena allocated objects. Also, added a GrowableArray destructor to call clear_and_deallocate() so that all callers need to do is: GrowableArray<T> *ga = new (C_HEAP) GrowableArray<T>(,true); ... delete ga; // call dtor and ResourceObj::operator delete() Also I removed NonPrintingResourceObj because it's supposedly an optimization of ResourceObj. In PRODUCT mode, the virtual print() functions aren't included anyway, so I didn't get the reason for the optimization. Timed runThese -full with fastdebug and it came out about the same, actually it came out faster(?). Also, fixed compilation warning from tagged stack interpreter putback. Fix verified: y Verified by: Fixed asserts that were found with instances of Resource allocated growable arrays with C_heap elements. PRT Other testing: nsk vm.nightly.testlist (derived from vm.quick.testlist) (include jvmti tests) with -client/-server runThese -full -client/-server Initial version reviewed by: Steve B, Alan B, Ken, Mandy, Ramki, John R Final version Reviewed by: Steve B, John C, Steve G, Vladimir, Alan B
27-09-2005
EVALUATION ###@###.### 2002-09-16 A call to Foo *my_foo = new(ResourceObj::C_HEAP) Foo(...) turns into a call to AllocateHeap(size_t n), so the resulting storage can be freed by calling FREE_C_HEAP_ARRAY(my_foo). This is not pretty, but is workable. As far as calling the destructor, that's not available given the current implementation of ResourceObj, Arena and Chunk. The ResourceMark destructor reclaims storage for ResourceObjs in the 'normal' case (when they're not allocated with new(ResourceObj::C_HEAP, ...). It eventually calls Chunk::next_chop(), where the memory is typed as a Chunk* and c++ delete is called. A Chunk (allocation.hpp:122) does not have a virtual destructor (or even a destructor for that matter), so any subclass of ResourceObj will never have a destructor called. Some method is needed to automatically verify that no subclass of ResourceObj has a destructor, or even better to prevent any subclass from declaring a destructor. As far as deleting a 'c-heap allocated ResourceObj', some syntactic sugar can be added that does the equivalent of FREE_C_HEAP_ARRAY(this). One possibility is adding an alternative operator delete to ResourceObj: class ResourceObj ... { ... static operator delete(void *p, allocation_type type) { if (type == C_HEAP) { FREE_C_HEAP_ARRAY(p); } } }; The syntax to call it is not ideal: Foo *my_foo = new (ResourceObj::C_HEAP) Foo(...); ... use my_foo ... ResourceObj::operator delete(my_foo, ResourceObj::C_HEAP); The simplest solution is to add another #define to allocation.hpp specifically for c-heap-allocated ResourceObjs: // Free ResourceObjs allocated on the c heap via // new(ResourceObj::C_HEAP) FooType #define FREE_C_HEAP_RESOURCE_OBJ(p) FREE_C_HEAP_ARRAY(p) ###@###.### 2005-06-08 Yet another alternative to allow the storage to be freed: add a new static method to ResourceObj static ResourceObj::delete_c_heap_obj(void* p) { FreeHeap(p); } It doesn't solve the destructor problem, but that is not solvable without making ResourceObjs larger and rewriting the existing code that handles ResourceObjs that are allocated in the Resource Area. ###@###.### 2005-06-09 05:48:54 GMT
09-06-2005
SUGGESTED FIX The suggested fix is to add a method class ResourceObj ALLOCATION_SUPER_CLASS_SPEC { public: // Release storage associated with an instance. static void release(ResourceObj* instance, allocation_type type) { switch (type) { default: ShouldNotReachHere(); break; case C_HEAP: instance->~ResourceObj(); // Call virtual destructor(s). FreeHeap(instance); // Free the memory for the instance. break; case RESOURCE_AREA: delete instance; // Call virtual destructors. break; } } // Virtual destructor here forces all destructors to be virtual. virtual ~ResourceObj() { // Nothing to do. } }; that clients can call to match calls to Foo* instance = new (ResourceObj::C_HEAP) Foo(...); with calls to ResourceObj::release(instance); which is not as nice as calling delete instance; but it's better than leaking storage. The problem with this scheme is that in PRODUCT builds ResourceObj's don't have vtables, so adding a virtual destructor will bloat all ResourceObj instances. I think that means we can't add a virtual destructor to ResourceObj, and so can't call the destructor from ResourceObj::releasebut instead clients have to call instance->~Foo(); ResourceObj::release(instance); themselves. We could encourage people who use ResourceObj's as CHeapObj's to have mathed static factory create and destroy methods: class Foo : public ResourceObj { public: static Foo* create(....) { return new(new(ResourceObj::C_HEAP) Foo(....); } static void destroy(Foo* instance) { instance->~Foo(); ResourceObj::destroy(instance); } }; to hide the ugliness of allocation and destruction in methods local to class Foo. ###@###.### 2002-01-30
30-01-2002