View mode: basic / threaded / horizontal-split · Log in · Help
June 25, 2012
An idea to avoid a narrow class of bugs
I've suggested to add a little piece of static analysis inside 
the D compiler, able to catch all cases of a specific kind of 
bugs:

http://d.puremagic.com/issues/show_bug.cgi?id=8293


A short thread in D.learn about a 
core.exception.InvalidMemoryOperationError:
http://forum.dlang.org/thread/js649p$1707$1@digitalmars.com

Caused by this code:

class X {
    string[string] s;
    this() {
        s["s"] = "S";
    }
    ~this() {
        s.remove("s");
    }
}
void main() {
    X x = new X();
}



Jonathan M Davis:

> you should never do anything in a class' destructor/finalizer 
> which could ever trigger the GC in any way.

In past I have seen other similar bugs discussed in D.learn.

I think a small amount of static analysis code added to the D 
front-end can statically avoid every bug of this kind. This code 
has to look inside ~this(){} and work recursively (like purity 
and nothrow enforcement do), searching for functions that perform 
GC activity.

(This is a bit different from the enforcement of the @noheap 
annotation I have suggested in Issue 5219 because it's OK to 
manage C-heap memory inside the destructor, while @noheap looks 
for C-heap activity too (malloc/realloc/calloc)).

Bye,
bearophile
June 25, 2012
Re: An idea to avoid a narrow class of bugs
I like this idea, I've had some nasty bugs because of this when I 
just started with D.

But IIRC the language doesn't forbid use of the GC in 
destructors, meaning it's an implementation issue. I don't know 
what the problems involved in allowing allocations during sweeps 
are, but I'd prefer to have support for GC usage in destructors. 
Having to work around this limitation often results in a load of 
ugly and overly complex code that should have been only a few 
straightforward lines.
June 25, 2012
Re: An idea to avoid a narrow class of bugs
Le 25/06/2012 13:35, bearophile a écrit :
> I've suggested to add a little piece of static analysis inside the D
> compiler, able to catch all cases of a specific kind of bugs:
>
> http://d.puremagic.com/issues/show_bug.cgi?id=8293
>
>
> A short thread in D.learn about a
> core.exception.InvalidMemoryOperationError:
> http://forum.dlang.org/thread/js649p$1707$1@digitalmars.com
>
> Caused by this code:
>
> class X {
>      string[string] s;
>      this() {
>          s["s"] = "S";
>      }
>      ~this() {
>          s.remove("s");
>      }
> }
> void main() {
>      X x = new X();
> }
>
>
>
> Jonathan M Davis:
>
>> you should never do anything in a class' destructor/finalizer which
>> could ever trigger the GC in any way.
>
> In past I have seen other similar bugs discussed in D.learn.
>
> I think a small amount of static analysis code added to the D front-end
> can statically avoid every bug of this kind. This code has to look
> inside ~this(){} and work recursively (like purity and nothrow
> enforcement do), searching for functions that perform GC activity.
>
> (This is a bit different from the enforcement of the @noheap annotation
> I have suggested in Issue 5219 because it's OK to manage C-heap memory
> inside the destructor, while @noheap looks for C-heap activity too
> (malloc/realloc/calloc)).
>
> Bye,
> bearophile

To me, it is a GC implementation issue. You should be able to allocate 
in destructors.
June 25, 2012
Re: An idea to avoid a narrow class of bugs
On 25-06-2012 15:17, deadalnix wrote:
> Le 25/06/2012 13:35, bearophile a écrit :
>> I've suggested to add a little piece of static analysis inside the D
>> compiler, able to catch all cases of a specific kind of bugs:
>>
>> http://d.puremagic.com/issues/show_bug.cgi?id=8293
>>
>>
>> A short thread in D.learn about a
>> core.exception.InvalidMemoryOperationError:
>> http://forum.dlang.org/thread/js649p$1707$1@digitalmars.com
>>
>> Caused by this code:
>>
>> class X {
>>      string[string] s;
>>      this() {
>>          s["s"] = "S";
>>      }
>>      ~this() {
>>          s.remove("s");
>>      }
>> }
>> void main() {
>>      X x = new X();
>> }
>>
>>
>>
>> Jonathan M Davis:
>>
>>> you should never do anything in a class' destructor/finalizer which
>>> could ever trigger the GC in any way.
>>
>> In past I have seen other similar bugs discussed in D.learn.
>>
>> I think a small amount of static analysis code added to the D front-end
>> can statically avoid every bug of this kind. This code has to look
>> inside ~this(){} and work recursively (like purity and nothrow
>> enforcement do), searching for functions that perform GC activity.
>>
>> (This is a bit different from the enforcement of the @noheap annotation
>> I have suggested in Issue 5219 because it's OK to manage C-heap memory
>> inside the destructor, while @noheap looks for C-heap activity too
>> (malloc/realloc/calloc)).
>>
>> Bye,
>> bearophile
>
> To me, it is a GC implementation issue. You should be able to allocate
> in destructors.

Yes, I don't understand why on earth this limitation is in place. There 
is no (good) technical reason it should be there.

-- 
Alex Rønne Petersen
alex@lycus.org
http://lycus.org
June 25, 2012
Re: An idea to avoid a narrow class of bugs
Alex Rønne Petersen , dans le message (digitalmars.D:170616), a écrit :
>>
>> To me, it is a GC implementation issue. You should be able to allocate
>> in destructors.
> 
> Yes, I don't understand why on earth this limitation is in place. There 
> is no (good) technical reason it should be there.

Allowing safe unwinding of the stack when throwing exceptions is not a 
'good' reason ?

Well, a destructor should rather be no_throw, and should be able to 
call no_throw (GC) functions.

-- 
Christophe
June 25, 2012
Re: An idea to avoid a narrow class of bugs
On 25-06-2012 15:27, Christophe Travert wrote:
> Alex Rønne Petersen , dans le message (digitalmars.D:170616), a écrit :
>>>
>>> To me, it is a GC implementation issue. You should be able to allocate
>>> in destructors.
>>
>> Yes, I don't understand why on earth this limitation is in place. There
>> is no (good) technical reason it should be there.
>
> Allowing safe unwinding of the stack when throwing exceptions is not a
> 'good' reason ?
>
> Well, a destructor should rather be no_throw, and should be able to
> call no_throw (GC) functions.
>

I fail to see how this is a problem. The GC (read: finalizer thread) 
simply has to catch the exception and Do Something Meaningful with it.

-- 
Alex Rønne Petersen
alex@lycus.org
http://lycus.org
June 25, 2012
Re: An idea to avoid a narrow class of bugs
The idea is nice, it would help newcomers a lot.
But please also think about the people that are using D _without_ a 
garbage collector. I wouldn't want the compiler to complain about 
something that isn't even true with my modified version of druntime.

Kind Regards
Benjamin Thaut

> I've suggested to add a little piece of static analysis inside the D
> compiler, able to catch all cases of a specific kind of bugs:
>
> http://d.puremagic.com/issues/show_bug.cgi?id=8293
>
>
> A short thread in D.learn about a
> core.exception.InvalidMemoryOperationError:
> http://forum.dlang.org/thread/js649p$1707$1@digitalmars.com
>
> Caused by this code:
>
> class X {
> string[string] s;
> this() {
> s["s"] = "S";
> }
> ~this() {
> s.remove("s");
> }
> }
> void main() {
> X x = new X();
> }
>
>
>
> Jonathan M Davis:
>
>> you should never do anything in a class' destructor/finalizer which
>> could ever trigger the GC in any way.
>
> In past I have seen other similar bugs discussed in D.learn.
>
> I think a small amount of static analysis code added to the D front-end
> can statically avoid every bug of this kind. This code has to look
> inside ~this(){} and work recursively (like purity and nothrow
> enforcement do), searching for functions that perform GC activity.
>
> (This is a bit different from the enforcement of the @noheap annotation
> I have suggested in Issue 5219 because it's OK to manage C-heap memory
> inside the destructor, while @noheap looks for C-heap activity too
> (malloc/realloc/calloc)).
>
> Bye,
> bearophile
June 25, 2012
Re: An idea to avoid a narrow class of bugs
Alex Rønne Petersen , dans le message (digitalmars.D:170622), a écrit :
> On 25-06-2012 15:27, Christophe Travert wrote:
>> Alex Rønne Petersen , dans le message (digitalmars.D:170616), a écrit :
>>>>
>>>> To me, it is a GC implementation issue. You should be able to allocate
>>>> in destructors.
>>>
>>> Yes, I don't understand why on earth this limitation is in place. There
>>> is no (good) technical reason it should be there.
>>
>> Allowing safe unwinding of the stack when throwing exceptions is not a
>> 'good' reason ?
>>
>> Well, a destructor should rather be no_throw, and should be able to
>> call no_throw (GC) functions.
>>
> 
> I fail to see how this is a problem. The GC (read: finalizer thread) 
> simply has to catch the exception and Do Something Meaningful with it.
> 


I confused two dinstict issues concerning destructors:

 - 1/ during GC, the GC may collect data in any order. References in a 
collected object may be invalid. This is not specific to GC usage in 
destructors however.

Example:
struct B
{
 string x;
 ~this()
 {
   writeln('Deleting ', x);
 }
}
struct A
{
 B* b;
 ~this()
 {
   writeln('About to Delete ', b.x); // error since b may have been 
                                     // deleted before this A instance.
   GC.free(b); // should be fine, GC.free has no effect on already 
               // deallocated block
 }
}

=> Using a maybe dead reference should be forbidden in destructor: 
Problem: it is in the general case impossible to tell if A.b has been 
allocated from the GC or not at compile time. However, somebody trying 
to collect GC data in a destructor is likely to ignore that the data may 
already have been collected. I reckon it is legitimate to collect GC 
data from a destructor (provided issue 2 is handled).

 - 2/ When an exception is thrown, destructors are called to unwind the 
stack until the exception is caught. If destructors start to trigger 
exceptions, things can get really messy.

=> It is a good idea to make destructors no_throw (and as stupid and 
simple as possible).

Maybe there is a third issue that motivate bearophile's post.

-- 
Christophe
June 25, 2012
Re: An idea to avoid a narrow class of bugs
On Monday, June 25, 2012 15:22:03 Alex Rønne Petersen wrote:
> > To me, it is a GC implementation issue. You should be able to allocate
> > in destructors.
> 
> Yes, I don't understand why on earth this limitation is in place. There
> is no (good) technical reason it should be there.

I believe that the main problem is that there's no guarantee that anything 
allocated on the GC heap will actually still exist when you the finalizer is 
run (since the order of destruction/finalization is undefined and in order to 
break circular references, the finalizer of a class could be run after some of 
its members' finalizers have been run), making it unsafe to access any stuff 
from the GC heap inside of the finalizer. But why that would prevent you from 
allocating inside of the finalizer, I don't know, since that would be newly 
allocated memory rather than memory that might have already been released as 
is the case if you reference member variables which are on the heap - though I 
don't know that it's really much of a restriction to not be able to allocate 
within the finilazire when allocating something on the GC heap without 
accessing anything else on the GC within the finalizer is unlikely to be useful 
very often (maybe it would be useful when creating an array or string to pass 
to a C function, but beyond that, I suspect that the fact that you can't 
access pre-existing stuff on the GC heap already prevents whatever you'd be 
trying to do in almost all cases).

- Jonathan MDavis
Top | Discussion index | About this forum | D home