www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - An idea to avoid a narrow class of bugs

reply "bearophile" <bearophileHUGS lycos.com> writes:
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
Jun 25 2012
next sibling parent "Rene Zwanenburg" <renezwanenburg gmail.com> writes:
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.
Jun 25 2012
prev sibling next sibling parent reply deadalnix <deadalnix gmail.com> writes:
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.
Jun 25 2012
parent reply =?UTF-8?B?QWxleCBSw7hubmUgUGV0ZXJzZW4=?= <alex lycus.org> writes:
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
Jun 25 2012
next sibling parent reply travert phare.normalesup.org (Christophe Travert) writes:
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
Jun 25 2012
parent reply =?UTF-8?B?QWxleCBSw7hubmUgUGV0ZXJzZW4=?= <alex lycus.org> writes:
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
Jun 25 2012
parent travert phare.normalesup.org (Christophe Travert) writes:
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
Jun 25 2012
prev sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Monday, June 25, 2012 15:22:03 Alex R=C3=B8nne Petersen wrote:

 To me, it is a GC implementation issue. You should be able to alloc=




ate

 in destructors.


=20
 Yes, I don't understand why on earth this limitation is in place. The=


re

 is no (good) technical reason it should be there.


I believe that the main problem is that there's no guarantee that anyth=
ing=20
allocated on the GC heap will actually still exist when you the finaliz=
er is=20
run (since the order of destruction/finalization is undefined and in or=
der to=20
break circular references, the finalizer of a class could be run after =
some of=20
its members' finalizers have been run), making it unsafe to access any =
stuff=20
from the GC heap inside of the finalizer. But why that would prevent yo=
u from=20
allocating inside of the finalizer, I don't know, since that would be n=
ewly=20
allocated memory rather than memory that might have already been releas=
ed as=20
is the case if you reference member variables which are on the heap - t=
hough I=20
don't know that it's really much of a restriction to not be able to all=
ocate=20
within the finilazire when allocating something on the GC heap without=20=

accessing anything else on the GC within the finalizer is unlikely to b=
e useful=20
very often (maybe it would be useful when creating an array or string t=
o pass=20
to a C function, but beyond that, I suspect that the fact that you can'=
t=20
access pre-existing stuff on the GC heap already prevents whatever you'=
d be=20
trying to do in almost all cases).

- Jonathan MDavis
Jun 25 2012
prev sibling parent Benjamin Thaut <code benjamin-thaut.de> writes:
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
Jun 25 2012