www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 2590] New: Deallocator is not called if constructor fails.

reply d-bugmail puremagic.com writes:
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.


--
Jan 17 2009
next sibling parent d-bugmail puremagic.com writes:
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: -------
Apr 04 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
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: -------
Apr 04 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
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: -------
Apr 04 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
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: -------
Apr 04 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
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: -------
Apr 04 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
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: -------
Apr 05 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
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: -------
Apr 05 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
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: -------
Nov 27 2010
prev sibling parent d-bugmail puremagic.com writes:
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: -------
Nov 27 2010