January 17, 2009
http://d.puremagic.com/issues/show_bug.cgi?id=2590

           Summary: Deallocator is not called if constructor fails.
           Product: D
           Version: 2.023
          Platform: PC
        OS/Version: Windows
            Status: NEW
          Severity: normal
          Priority: P2
         Component: DMD
        AssignedTo: bugzilla@digitalmars.com
        ReportedBy: samukha@voliacable.com


The following test case leaks memory:

import std.c.stdlib;

class C
{
    new(size_t size)
    {
        writefln("In new");
        return malloc(size);
    }

    this()
    {
        writefln("In constructor");
        throw new Exception("Exception in ctor");
    }

    ~this()
    {
        writefln("in destructor");
    }

    delete(void* p)
    {
        writefln("In delete");
        free(p);
    }
}

void main()
{
    try
    {
        auto c = new C;
    }
    catch {}
}
----
In new
In constructor

Deallocator should be called to free the memory.


-- 

April 04, 2010
http://d.puremagic.com/issues/show_bug.cgi?id=2590


Don <clugdbug@yahoo.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |clugdbug@yahoo.com.au


--- Comment #1 from Don <clugdbug@yahoo.com.au> 2010-04-04 07:43:39 PDT ---
I'm pretty sure this bug is invalid.

http://herbsutter.com/2008/07/25/constructor-exceptions-in-c-c-and-java/

As Herb says,
"destructors only run on successfully constructed objects."

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
April 04, 2010
http://d.puremagic.com/issues/show_bug.cgi?id=2590



--- Comment #2 from Max Samukha <samukha@voliacable.com> 2010-04-04 10:07:51 PDT ---
He is talking about constructors/destructors, not allocators/deallocators. I totally agree the destructor must not be called on a partially constructed object. Conversely, the memory that has been successfully allocated for an object needs to be properly deallocated even if the constructor fails.

On the other hand, if overloaded new/delete are going to be removed from the language, the problem will pass away.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
April 04, 2010
http://d.puremagic.com/issues/show_bug.cgi?id=2590


Justin Spahr-Summers <Justin.SpahrSummers@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |Justin.SpahrSummers@gmail.c
                   |                            |om


--- Comment #3 from Justin Spahr-Summers <Justin.SpahrSummers@gmail.com> 2010-04-04 15:35:10 CDT ---
(In reply to comment #2)
> He is talking about constructors/destructors, not allocators/deallocators. I totally agree the destructor must not be called on a partially constructed object. Conversely, the memory that has been successfully allocated for an object needs to be properly deallocated even if the constructor fails.
> 
> On the other hand, if overloaded new/delete are going to be removed from the language, the problem will pass away.

When one uses class allocators/deallocators, it's basically like taking memory management into one's own hands. The current behavior seems reasonable, because 'delete' is never invoked on the object (which is necessary for a custom deallocator to be used).

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
April 04, 2010
http://d.puremagic.com/issues/show_bug.cgi?id=2590



--- Comment #4 from Max Samukha <samukha@voliacable.com> 2010-04-04 15:17:17 PDT ---
> The current behavior seems reasonable, because
> 'delete' is never invoked on the object (which is necessary for a custom
> deallocator to be used).

I don't think it is reasonable. Why should I bother to call the deallocator manually while the allocator is called automatically? Think about object creation as a transaction consisting of two operations: allocation and construction. If construction fails, allocation should be automatically rolled back.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
April 04, 2010
http://d.puremagic.com/issues/show_bug.cgi?id=2590



--- Comment #5 from Justin Spahr-Summers <Justin.SpahrSummers@gmail.com> 2010-04-04 18:09:04 CDT ---
(In reply to comment #4)
> > The current behavior seems reasonable, because
> > 'delete' is never invoked on the object (which is necessary for a custom
> > deallocator to be used).
> 
> I don't think it is reasonable. Why should I bother to call the deallocator manually while the allocator is called automatically? Think about object creation as a transaction consisting of two operations: allocation and construction. If construction fails, allocation should be automatically rolled back.

But the allocator is *not* called automatically, strictly speaking. 'new' is your call to the allocator. Since you use malloc() instead of the garbage collector, 'delete' then becomes necessary.

Under normal circumstances, an exception thrown during construction wouldn't leak memory because the garbage collector would eventually collect it; in your code, you took on the task of manually allocating and deallocating memory for objects of class C. It makes sense to me that such custom allocation would entail finer management of exceptional situations.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
April 05, 2010
http://d.puremagic.com/issues/show_bug.cgi?id=2590



--- Comment #6 from Steven Schveighoffer <schveiguy@yahoo.com> 2010-04-05 04:43:18 PDT ---
(In reply to comment #5)
> 
> But the allocator is *not* called automatically, strictly speaking. 'new' is your call to the allocator. Since you use malloc() instead of the garbage collector, 'delete' then becomes necessary.
> 
> Under normal circumstances, an exception thrown during construction wouldn't leak memory because the garbage collector would eventually collect it; in your code, you took on the task of manually allocating and deallocating memory for objects of class C. It makes sense to me that such custom allocation would entail finer management of exceptional situations.

Not that I disagree this bug is obsolete, but what would you call delete on? With the failed construction, you never got a pointer to the class data.

If class allocators were to be saved, I think the correct behavior on a failed constructor should be to call the deallocator.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
April 06, 2010
http://d.puremagic.com/issues/show_bug.cgi?id=2590



--- Comment #7 from Justin Spahr-Summers <Justin.SpahrSummers@gmail.com> 2010-04-05 21:14:23 CDT ---
(In reply to comment #6)
> 
> Not that I disagree this bug is obsolete, but what would you call delete on? With the failed construction, you never got a pointer to the class data.
> 
> If class allocators were to be saved, I think the correct behavior on a failed constructor should be to call the deallocator.

Fair point. I didn't realize that it's impossible to hold onto the pointer.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
November 27, 2010
http://d.puremagic.com/issues/show_bug.cgi?id=2590



--- Comment #8 from Max Samukha <samukha@voliacable.com> 2010-11-27 12:03:42 PST ---
Though D is going to deprecate new/delete operators, it is worth noting that the rule under discussion was adopted by C++ 15 years ago.

From "Counting Objects in C++" article by Scott Meyers (http://blog.csdn.net/LYH_Studio/archive/2006/08/11/1051927.aspx):

"For many years this was a hole in the draft C++ language specification, but in March 1995 the C++ Standards committee adopted the rule that if, during a new expression, the invocation of operator new succeeds and the subsequent constructor call throws an exception, the runtime system must automatically deallocate the memory that operator new allocated. This deallocation is performed by operator delete, the deallocation analogue of operator new."

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
November 27, 2010
http://d.puremagic.com/issues/show_bug.cgi?id=2590



--- Comment #9 from Max Samukha <samukha@voliacable.com> 2010-11-27 12:06:43 PST ---
A better formatted text: http://www.drdobbs.com/cpp/184403484

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Top | Discussion index | About this forum | D home