Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

call destructors before Object dealloc #468

Merged
1 commit merged into from

2 participants

@galchinsky

It obviously reduces amount of memory leaks.
The only inherited from Object class that uses destructor is ValueHolder. ~ValueHolder() hasn't been called without this patch.

@jckarter
Owner

The only change necessary should be to make Pointer invoke ~Object rather than dealloc.

@galchinsky

I tried it first. This makes double delete happen. Virtual methods in destructors are not really virtual and destructors are called in reverse order. That's why calls virtual ~ANode() { ANode::dealloc(); }) + virtual ~Object() { Object::dealloc(); }) cause this:

ANodeAllocator->Deallocate(this);
::operator delete(this); //boom
@jckarter
Owner

Of course, you're right. Looks good then.

@galchinsky

ANode uses custom allocator, it can't be written so simply. But after writting custom delete it looks more concise.

@galchinsky

Isn't this the issue why destructors in evaluator are not called? I see the FIXME line

ValueHolder::~ValueHolder()
{
    // FIXME: call clay 'destroy'
    free(this->buf);
}
@jckarter
Owner

@galchinsky, @stepancheg is right that delete this is exactly equivalent to calling the destructor then operator delete. ANode handles its custom deallocator by overriding dealloc, which is necessary because operator delete is not dynamically dispatched.

I'm not sure that the ValueHolder destructor is the right place to invoke evaluator destructors. It would be more consistent to invoke them on scope exit, as is done in runtime codegen.

@galchinsky

I looked through stackoverflow before patching and found the reference to §12.5.7 of the standard.

"Since member allocation and deallocation functions are static they cannot be virtual. [Note: however, when the cast-expression of a delete-expression refers to an object of class type, because the deallocation function actually called is looked up in the scope of the class that is the dynamic type of the object, if the destructor is virtual, the effect is the same."

operator delete is dinamically dispatched when called from virtual destructor . I checked it using printf.

@jckarter
Owner

I didn't know that, thanks! Guess i still have some lingering brain damage from pre-standard c++ implementations.

@galchinsky

Mmm. Pull? Anyone?

@ghost ghost merged commit f07a85f into jckarter:master
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 10, 2013
  1. @galchinsky

    call destructors before Object dealloc

    galchinsky authored
    ...
    
    ...
    
    ...
This page is out of date. Refresh to see the latest.
Showing with 10 additions and 7 deletions.
  1. +10 −7 compiler/clay.hpp
View
17 compiler/clay.hpp
@@ -434,12 +434,11 @@ struct Object {
: refCount(0), objKind(objKind) {}
void incRef() { ++refCount; }
void decRef() {
- if (--refCount == 0)
- dealloc();
+ if (--refCount == 0) {
+ delete this;
+ }
}
- void operator delete(void*) {}
- virtual void dealloc() { ::operator delete(this); }
- virtual ~Object() { dealloc(); }
+ virtual ~Object() {}
};
typedef Pointer<Object> ObjectPtr;
@@ -997,7 +996,9 @@ struct ANode : public Object {
void *operator new(size_t num_bytes) {
return ANodeAllocator->Allocate(num_bytes, llvm::AlignOf<ANode>::Alignment);
}
- virtual void dealloc() { ANodeAllocator->Deallocate(this); }
+ void operator delete(void* anode) {
+ ANodeAllocator->Deallocate(anode);
+ }
};
struct Identifier : public ANode {
@@ -2598,7 +2599,9 @@ struct Type : public Object {
void *operator new(size_t num_bytes) {
return ANodeAllocator->Allocate(num_bytes, llvm::AlignOf<Type>::Alignment);
}
- virtual void dealloc() { ANodeAllocator->Deallocate(this); }
+ void operator delete(void* type) {
+ ANodeAllocator->Deallocate(type);
+ }
llvm::DIType getDebugInfo() { return llvm::DIType(debugInfo); }
};
Something went wrong with that request. Please try again.