www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - GC-proof resource classes

reply "ponce" <contact gam3sfrommars.fr> writes:
As a library writer I've struggled a bit with to provide easy 
resource clean-up despite using class objects.

Is there reasons to use class objects that hold resources in the 
first place vs Unique!T or RefCounted!T structs?
I think yes:
- classes have reference semantics without naming or runtime 
overhead: you read Resource and not RefCounted!Resource
- no need to disable the postblit or have a valid .init is 
another thing
That's about it from the top of my head and it may not be good 
reasons!

As you probably know GC-called destructors enjoy a variety of 
limitations:
- can't allocate,
- can't access members,
- aren't guaranteed to happen at all.

This makes GC-called destructors mostly useless for non-memory 
resource release. IIRC Andrei suggested once that destructors 
shouldn't be called at all by the GC, something that I agree with.

As such, some of us started providing a 
release()/dispose()/close() method, and have the destructor call 
it to support both scoped ownership and manual release.

But that introduce accidentally correct design when the 
destructor is called by GC, and avoids a leak. This is arguably 
worse than the initial problem.

It turns out separating calls of ~this() that are called by the 
GC from those that are called for a deterministic reason is 
enough, and support all cases I can think of: 
Unique!T/scoped!T/.destroy/RefCounted!T/manual/GC-called

It works like this:

----------------

class MyResource
{
     void* handle;

     this()
     {
         handle = create_handle();
     }

     ~this()
     {
         if (handle != null) // must support repeated calls for 
the case (called by .destroy + called by GC later)
         {
             ensureNotInGC("MyResource");
             free_handle(handle);
         }
     }
}

----------------

ensureNotInGC() is implemented like this:

----------------

void ensureNotInGC(string resourceName) nothrow
{
     debug
     {
         import core.exception;
         try
         {
             import core.memory;
             void* p = GC.malloc(1); // not ideal since it 
allocates
             return;
         }
         catch(InvalidMemoryOperationError e)
         {
             import core.stdc.stdio;
             fprintf(stderr, "Error: clean-up of %s incorrectly 
depends on destructors called by the GC.\n", resourceName.ptr);
             assert(false); // crash
         }
     }
}

--------------

Looks ugly? Yes, but it makes the GC acts as a cheap leak 
detector, giving accurate messages for still opened resources.
Aug 29 2015
next sibling parent reply "cym13" <cpicard openmailbox.org> writes:
On Saturday, 29 August 2015 at 13:14:26 UTC, ponce wrote:

 This makes GC-called destructors mostly useless for non-memory 
 resource release. IIRC Andrei suggested once that destructors 
 shouldn't be called at all by the GC, something that I agree 
 with.

 As such, some of us started providing a 
 release()/dispose()/close() method, and have the destructor 
 call it to support both scoped ownership and manual release.


I'm not sure it is the best way to do things... In Python for 
example we have a GC that calls the default destructor (__del__ 
method). As a consequence, if you need some resource to be freed 
you have to do it explicitely by writting a close()/whatever() 
method. But nobody's linking the destructor to it because of the 
separation of concerns principle: we release what has to be 
released and only that: freeing the object is the realm of the 
GC. This mixed style is something I have yet to encounter in D 
where it could be way more powerful than in Python: free what you 
must, not what you can.


 But that introduce accidentally correct design when the 
 destructor is called by GC, and avoids a leak. This is arguably 
 worse than the initial problem.


I'd like to see a concrete example of this, it seems I'm missing 
something...


 void ensureNotInGC(string resourceName) nothrow
 {
     debug
     {
         import core.exception;
         try
         {
             import core.memory;
             void* p = GC.malloc(1); // not ideal since it 
 allocates
             return;
         }
         catch(InvalidMemoryOperationError e)
         {
             import core.stdc.stdio;
             fprintf(stderr, "Error: clean-up of %s incorrectly 
 depends on destructors called by the GC.\n", resourceName.ptr);
             assert(false); // crash
         }
     }
 }

 --------------

 Looks ugly? Yes, but it makes the GC acts as a cheap leak 
 detector, giving accurate messages for still opened resources.


As I said before, I'm not sure preventing the GC from doing its 
collection job is a good idea, but I find the concept of having 
such a leak detector really interesting!
Aug 29 2015
next sibling parent reply "ponce" <contact gam3sfrommars.fr> writes:
On Saturday, 29 August 2015 at 13:43:33 UTC, cym13 wrote:

 But that introduce accidentally correct design when the 
 destructor is called by GC, and avoids a leak. This is 
 arguably worse than the initial problem.


 I'd like to see a concrete example of this, it seems I'm 
 missing something...


Example 1:

You forget to release Resource A. The GC happen to call A 
destructor that releases it. But GC destructors are not 
guaranteed to happen.
See http://dlang.org/class.html ("The garbage collector is not 
guaranteed to run the destructor for all unreferenced objects").


Example 2:

Resource A depends on Resource B. You forget to release either. 
The GC happens to call A and B destructors in the right order, by 
chance. A new D release changes this order later.
Aug 29 2015
parent "cym13" <cpicard openmailbox.org> writes:
On Saturday, 29 August 2015 at 13:58:07 UTC, ponce wrote:

 Example 1:

 You forget to release Resource A. The GC happen to call A 
 destructor that releases it. But GC destructors are not 
 guaranteed to happen.
 See http://dlang.org/class.html ("The garbage collector is not 
 guaranteed to run the destructor for all unreferenced objects").


This, ok, it is the common GC flaw that it shouldn't memleak but 
might. To me it isn't either an "accidentally correct design" nor 
an "avoided leak" but ok. If something _has_ to be freed then it 
should be done explicitely either way hence crashing the GC for 
that particular ressource, I follow you here.


 Example 2:

 Resource A depends on Resource B. You forget to release either. 
 The GC happens to call A and B destructors in the right order, 
 by chance. A new D release changes this order later.


Here comes the accidentally correct design. Ok, I'm with you on 
that one.

I think however that this should really be limited on ressources 
that _must_ be freed in time. It shouldn't become a standard way 
to deal with the GC.
Aug 29 2015
prev sibling parent reply "ponce" <contact gam3sfrommars.fr> writes:
On Saturday, 29 August 2015 at 13:43:33 UTC, cym13 wrote:

 But nobody's linking the destructor to it because of the 
 separation of concerns principle: we release what has to be 
 released and only that: freeing the object is the realm of the 
 GC.


I see what you mean, but Unique!T, RefCounted!T and scoped!T call 
the destructor, not the release() function you just defined. So 
separating concerns break those.
Aug 29 2015
parent "cym13" <cpicard openmailbox.org> writes:
On Saturday, 29 August 2015 at 14:00:59 UTC, ponce wrote:

 On Saturday, 29 August 2015 at 13:43:33 UTC, cym13 wrote:

 But nobody's linking the destructor to it because of the 
 separation of concerns principle: we release what has to be 
 released and only that: freeing the object is the realm of the 
 GC.


 I see what you mean, but Unique!T, RefCounted!T and scoped!T 
 call the destructor, not the release() function you just 
 defined. So separating concerns break those.


Yes, and I think it is kind of cumbersome actually. Being able to 
pass a method to scoped!T for example would be really great (with 
the destructor as default of course).
Aug 29 2015
prev sibling next sibling parent reply "rsw0x" <anonymous anonymous.com> writes:
On Saturday, 29 August 2015 at 13:14:26 UTC, ponce wrote:

 ...


All of this could be fixed by not letting the GC call 
destructors. It's a bad, error-prone design to begin with and I 
guarantee any semi-large D program is probably abusing undefined 
behavior due to it.
Aug 29 2015
next sibling parent reply "cym13" <cpicard openmailbox.org> writes:
On Saturday, 29 August 2015 at 14:17:10 UTC, rsw0x wrote:

 On Saturday, 29 August 2015 at 13:14:26 UTC, ponce wrote:

 ...


 All of this could be fixed by not letting the GC call 
 destructors. It's a bad, error-prone design to begin with and I 
 guarantee any semi-large D program is probably abusing 
 undefined behavior due to it.


After reading all that, I too am convinced that the GC shouldn't 
call the destructor.
Aug 29 2015
parent reply Timon Gehr <timon.gehr gmx.ch> writes:
On 08/29/2015 04:20 PM, cym13 wrote:

 On Saturday, 29 August 2015 at 14:17:10 UTC, rsw0x wrote:

 On Saturday, 29 August 2015 at 13:14:26 UTC, ponce wrote:

 ...


 All of this could be fixed by not letting the GC call destructors.
 It's a bad, error-prone design to begin with and I guarantee any
 semi-large D program is probably abusing undefined behavior due to it.


 After reading all that, I too am convinced that the GC shouldn't call
 the destructor.


But then classes with destructors shouldn't be allowed to be allocated 
on the GC heap in the first place, which is a PITA as well. (Note that 
classes/arrays can have destructors because some component structs have 
them; structs generally assume that their destructors will be called.)
Aug 29 2015
next sibling parent reply "rsw0x" <anonymous anonymous.com> writes:
On Saturday, 29 August 2015 at 14:32:27 UTC, Timon Gehr wrote:

 On 08/29/2015 04:20 PM, cym13 wrote:

 On Saturday, 29 August 2015 at 14:17:10 UTC, rsw0x wrote:

 On Saturday, 29 August 2015 at 13:14:26 UTC, ponce wrote:

 ...


 All of this could be fixed by not letting the GC call 
 destructors.
 It's a bad, error-prone design to begin with and I guarantee 
 any
 semi-large D program is probably abusing undefined behavior 
 due to it.


 After reading all that, I too am convinced that the GC 
 shouldn't call
 the destructor.


 But then classes with destructors shouldn't be allowed to be 
 allocated on the GC heap in the first place, which is a PITA as 
 well. (Note that classes/arrays can have destructors because 
 some component structs have them; structs generally assume that 
 their destructors will be called.)


make classes with destructors(and structs allocated via new) have 
RC semantics.
Aug 29 2015
parent reply Timon Gehr <timon.gehr gmx.ch> writes:
On 08/29/2015 04:45 PM, rsw0x wrote:

 On Saturday, 29 August 2015 at 14:32:27 UTC, Timon Gehr wrote:

 On 08/29/2015 04:20 PM, cym13 wrote:

 On Saturday, 29 August 2015 at 14:17:10 UTC, rsw0x wrote:

 On Saturday, 29 August 2015 at 13:14:26 UTC, ponce wrote:

 ...


 All of this could be fixed by not letting the GC call destructors.
 It's a bad, error-prone design to begin with and I guarantee any
 semi-large D program is probably abusing undefined behavior due to it.


 After reading all that, I too am convinced that the GC shouldn't call
 the destructor.


 But then classes with destructors shouldn't be allowed to be allocated
 on the GC heap in the first place, which is a PITA as well. (Note that
 classes/arrays can have destructors because some component structs
 have them; structs generally assume that their destructors will be
 called.)


 make classes with destructors(and structs allocated via new) have RC
 semantics.


RC is an especially eager form of GC, that does not deal with cycles by 
default. Why does it help?
Aug 29 2015
parent reply "rsw0x" <anonymous anonymous.com> writes:
On Saturday, 29 August 2015 at 23:08:45 UTC, Timon Gehr wrote:

 On 08/29/2015 04:45 PM, rsw0x wrote:

 On Saturday, 29 August 2015 at 14:32:27 UTC, Timon Gehr wrote:

 On 08/29/2015 04:20 PM, cym13 wrote:

 On Saturday, 29 August 2015 at 14:17:10 UTC, rsw0x wrote:

 [...]


 After reading all that, I too am convinced that the GC 
 shouldn't call
 the destructor.


 But then classes with destructors shouldn't be allowed to be 
 allocated
 on the GC heap in the first place, which is a PITA as well. 
 (Note that
 classes/arrays can have destructors because some component 
 structs
 have them; structs generally assume that their destructors 
 will be
 called.)


 make classes with destructors(and structs allocated via new) 
 have RC
 semantics.


 RC is an especially eager form of GC, that does not deal with 
 cycles by default. Why does it help?


The problem is that there's no guarantee the destructor will run 
with the GC, no guarantee what thread it will run on, and no 
guarantee on when it will run. RC guarantees all three of these 
outside of cycles.
Aug 30 2015
parent Timon Gehr <timon.gehr gmx.ch> writes:
On 08/30/2015 12:10 PM, rsw0x wrote:

 On Saturday, 29 August 2015 at 23:08:45 UTC, Timon Gehr wrote:

 On 08/29/2015 04:45 PM, rsw0x wrote:

 On Saturday, 29 August 2015 at 14:32:27 UTC, Timon Gehr wrote:

 On 08/29/2015 04:20 PM, cym13 wrote:

 On Saturday, 29 August 2015 at 14:17:10 UTC, rsw0x wrote:

 [...]


 After reading all that, I too am convinced that the GC shouldn't call
 the destructor.


 But then classes with destructors shouldn't be allowed to be allocated
 on the GC heap in the first place, which is a PITA as well. (Note that
 classes/arrays can have destructors because some component structs
 have them; structs generally assume that their destructors will be
 called.)


 make classes with destructors(and structs allocated via new) have RC
 semantics.


 RC is an especially eager form of GC, that does not deal with cycles
 by default. Why does it help?


 The problem is that there's no guarantee the destructor will run with
 the GC,


This is something the GC can in principle guarantee though, and it might 
be good enough to deallocate memory.


 no guarantee what thread it will run on, and no guarantee on
 when it will run. RC guarantees all three of these


Those are good points, but why do this implicitly with the same 
allocation syntax?


 outside of cycles.


One might not have no cycles. Each reference counted reference has a 
destructor. I.e. as soon as some class in an object graph has a 
destructor, a lot of them do. I think it wouldn't necessarily be great 
language design to have memory leaks caused by e.g. adding 
std.container.Array to some class.
Aug 30 2015
prev sibling parent reply "cym13" <cpicard openmailbox.org> writes:
On Saturday, 29 August 2015 at 14:32:27 UTC, Timon Gehr wrote:

 But then classes with destructors shouldn't be allowed to be 
 allocated on the GC heap in the first place, which is a PITA as 
 well. (Note that classes/arrays can have destructors because 
 some component structs have them; structs generally assume that 
 their destructors will be called.)


I don't quite follow the reasonning here. If GC doesn't call the 
destructor then this same destructor is no more than a normal 
method (with restrictions... would those still stand?) that is 
the default destruction method to be called by things like 
scoped!T or destroy if explicit destruction is needed.

I think there should be a separation of concerns that isn't 
possible right now. Freeing ressources and freeing memory isn't 
the same thing and they should be decoupled. I think a destructor 
is there to free ressources, and the GC is there to free memory. 
If the GC doesn't call the destructor then why should having a 
destructor have anything to do with the GC?

Or do you fear for classes whose memory would be freed before 
freeing its ressources? That could be a problem... In that case I 
think the best option would be to allow them to be allocated by 
the GC but GC-ing it if the destructor hasn't been called should 
spawn an error (or something like that, haven't taken the time to 
think through it). Or maybe it shouldn't be marked as garbage if 
the destructor hasn't been called.

I think of it as a simple switch hasDestructorBeenCalled  that 
would be set to true if no destructor exists or if it has been 
called, and false otherwise, and would prevent GC-ing (or 
crash... I don't know what's best) of the object if false.

That way simple classes stay simple, complex classes can live on 
the heap happily without fearing collection while being able to 
reliably free ressources.
Aug 29 2015
next sibling parent reply Timon Gehr <timon.gehr gmx.ch> writes:
On 08/29/2015 05:20 PM, cym13 wrote:

 On Saturday, 29 August 2015 at 14:32:27 UTC, Timon Gehr wrote:

 But then classes with destructors shouldn't be allowed to be allocated
 on the GC heap in the first place, which is a PITA as well. (Note that
 classes/arrays can have destructors because some component structs
 have them; structs generally assume that their destructors will be
 called.)


 I don't quite follow the reasonning here. If GC doesn't call the
 destructor then this same destructor is no more than a normal method


If it is no more than a normal method:
- Why have special syntax for it?
- Why should Object have it?


 (with restrictions... would those still stand?) that is the default
 destruction method to be called by things like scoped!T or destroy if
 explicit destruction is needed.
 ...


If there is a destructor, this (usually) means that explicit destruction 
is needed.

Again, note that if I have

import std.collection: Array;
class C{ Array arr; ... }

then now C implicitly has a destructor (that does nothing but call arr's 
destructor which may in turn free memory on the C heap).

Constructor and destructor are supposed to frame the lifetime of an 
instance. Destructors are in the language so that the language can help 
with enforcing this. If there's a built-in and expected way to violate 
this property, the syntactic similarity of constructors and destructors 
is misleading, and the features are less useful.


 I think there should be a separation of concerns that isn't possible
 right now. Freeing ressources and freeing memory isn't the same thing
 and they should be decoupled.


Memory is a resource, and not all memory is allocated by the GC. (c.f. 
http://erdani.com/d/phobos-prerelease/std_experimental_allocator.html)


 I think a destructor is there to free
 ressources, and the GC is there to free memory. If the GC doesn't call
 the destructor then why should having a destructor have anything to do
 with the GC?

 Or do you fear for classes whose memory would be freed before freeing
 its ressources?


For example. The general idea is that there is no point in having 
language features to deal with complex issues if they actually don't.


 That could be a problem...  In that case I think the best
 option would be to allow them to be allocated by the GC


I assume this means allow 'new Class()' even if Class has a destructor.


 but GC-ing it if
 the destructor hasn't been called should spawn an error (or something
 like that, haven't taken the time to think through it).


Why is it sensible to have the same syntax for allocation if
deallocation/destruction needs to be handled differently?


 Or maybe it shouldn't be marked as garbage if the destructor hasn't been
called.
 ...


I.e. leak memory.


 I think of it as a simple switch hasDestructorBeenCalled  that would be
 set to true if no destructor exists or if it has been called, and false
 otherwise, and would prevent GC-ing (or crash... I don't know what's
 best) of the object if false.

 That way simple classes stay simple,


Simple classes get an additional hidden field. Even the monitor field is 
too much.


 complex classes can live on the
 heap happily without fearing collection while being able to reliably
 free ressources.


This does not make a lot of sense. If there is no live reference to a 
class managing a resource and it would then need to "fear" collection, 
this means that the resource has been leaked.
Aug 29 2015
parent "cym13" <cpicard openmailbox.org> writes:
On Saturday, 29 August 2015 at 22:50:44 UTC, Timon Gehr wrote:

 If it is no more than a normal method:
 - Why have special syntax for it?
 - Why should Object have it?


I see it a bit like popFront() and other range methods that are 
normal methods but take part in common process so their name was 
standardized and they are usually used through a special syntax. 
I have no answer for the Object comment, I don't see why it 
should have it if it doesn't do anything.


 If there is a destructor, this (usually) means that explicit 
 destruction is needed.

 Again, note that if I have

 import std.collection: Array;
 class C{ Array arr; ... }

 then now C implicitly has a destructor (that does nothing but 
 call arr's destructor which may in turn free memory on the C 
 heap).

 Constructor and destructor are supposed to frame the lifetime 
 of an instance. Destructors are in the language so that the 
 language can help with enforcing this. If there's a built-in 
 and expected way to violate this property, the syntactic 
 similarity of constructors and destructors is misleading, and 
 the features are less useful.



 Memory is a resource, and not all memory is allocated by the 
 GC. (c.f. 
 http://erdani.com/d/phobos-prerelease/std_experimental_allocator.html)


It isn't quite the same kind of resource as, say, a File handle 
that would cause more problem if not handed closed in time. 
Letting the GC collect when it estimates that memory is low is 
often more than good enough. Also, whoever allocates/opens a 
resource is the one that should be freeing/closing it. That goes 
for memory as well.


 I assume this means allow 'new Class()' even if Class has a 
 destructor.


It means allow this class to be managed by the GC



 Why is it sensible to have the same syntax for allocation if
 deallocation/destruction needs to be handled differently?


Because memory management is the same here, internal resource 
management isn't


 I.e. leak memory.


or being incorrectly GC-ed, that seems more dangerous to me... 
Well it shouldn't happen anyway so crashing should actually be ok.


 Simple classes get an additional hidden field. Even the monitor 
 field is too much.


They're using the GC and you're worrying about 1 bit of overhead? 
Ok, not having it would be better but still...


 This does not make a lot of sense. If there is no live 
 reference to a class managing a resource and it would then need 
 to "fear" collection, this means that the resource has been 
 leaked.


Yeah, after thinking about it I agree with that.

---

Reading this made me think about why I'd want all classes to be 
managed by the GC. I think what I fear most is inconsistence in 
usage. Also, I always feel uneasy from this exodus away from the 
GC. Having a GC is great, I don't want to be forced to always 
manage memory by hand. Not being able to use a class (or struct, 
because I think it would apply too) because it has a destructor 
would be a disruptive hard to explain change.

The thing is I still think all resources aren't equal and having 
a mixed style when you do explicitely only what's necessary could 
prove powerful in D. I need to mature this idea though.

Letting that aside, I can't find a good reason for thoses objects 
to be used by the GC if their memory must be explicitely freed, I 
now think you're right about that. The more I think about it the 
more that solution makes sense to me... But it would be one hell 
of a breaking change so it probably won't happen... too bad 
because it is a better idea than not calling the destructor. Not 
calling it would produce leaks, calling ponce's leak detector 
would produce runtime errors... In that case having compile-time 
errors sounds better. And if one need it to fit in the garbage 
collector anyway but wants to free some resource he sure can 
afford a .close() method that would make it explicit instead of 
providing a destructor.

Yeah, that sounds right as it is.
Aug 29 2015
prev sibling parent reply Jim Hewes <jimhewes gmail.com> writes:
On 8/29/2015 8:20 AM, cym13 wrote:

 I think there should be a separation of concerns that isn't possible
 right now. Freeing ressources and freeing memory isn't the same thing
 and they should be decoupled. I think a destructor is there to free
 ressources, and the GC is there to free memory. If the GC doesn't call
 the destructor then why should having a destructor have anything to do
 with the GC?


This.

I think when language designers consider reference counting vs. garbage 
collection in the beginning, they decide that reference counting is too 
costly, particularly in cases where you may need to create many small 
objects. So garbage collection seems to make more sense if you have to 
choose one or the other. But the places I want deterministic destruction 
always tend to be when there are only one or a few objects because they 
are handles or wrappers for resources which aren't very numerous. So the 
cost of reference counting is negligible and is worth it in these cases. 
But unfortunately this problem got swept under the rug with garbage 
collection.

I confess I haven't thought about this in depth and surely other people 
have; so there may be reasons why this is a more difficult problem than 
I think. But I agree there should be a separation of concerns between 
memory and other resources. But there may need to be some sort of 
transitivity with it, similar to const/immutable. That is, if I have an 
object which needs reference counting---for which the resources need to 
be cleaned up---and it is a member of another object, then the owning 
object also needs to be reference counted automatically or made to be a 
compiler error. Whether this is implicit just by having a destructor 
defined or explicit I'm not sure. I just don't know if this could be 
handled at all with collection classes. (How would they know?)

Does it make sense that this needs to be tied to structs and not 
classes? I'm not a language designer so I don't know the reasons. But 
ideally I'd think the whole concern of releasing resources would also be 
separate from that as well.

Regarding the garbage collector, if the destructor isn't guaranteed to 
be called, then I have to assume the worst case that it won't be called. 
That means I should not use destructors in these cases. It seems to me 
that should be enough argument.
Aug 30 2015
parent "rsw0x" <anonymous anonymous.com> writes:
On Sunday, 30 August 2015 at 18:21:17 UTC, Jim Hewes wrote:

 Regarding the garbage collector, if the destructor isn't 
 guaranteed to be called, then I have to assume the worst case 
 that it won't be called. That means I should not use 
 destructors in these cases. It seems to me that should be 
 enough argument.


I've made this argument before, nobody seemed that interested.
A perfectly standards compliant garbage collector could 
completely ignore destructors.
Aug 30 2015
prev sibling parent "ponce" <contact gam3sfrommars.fr> writes:
On Saturday, 29 August 2015 at 14:17:10 UTC, rsw0x wrote:

 All of this could be fixed by not letting the GC call 
 destructors. It's a bad, error-prone design to begin with and I 
 guarantee any semi-large D program is probably abusing 
 undefined behavior due to it.


Indeed.
Aug 29 2015
prev sibling next sibling parent reply "skoppe" <mail skoppe.eu> writes:
On Saturday, 29 August 2015 at 13:14:26 UTC, ponce wrote:

 class MyResource
 {
     void* handle;

     this()
     {
         handle = create_handle();
     }

     ~this()
     {
         if (handle != null) // must support repeated calls for 
 the case (called by .destroy + called by GC later)
         {
             ensureNotInGC("MyResource");
             free_handle(handle);
         }
     }
 }


I don't think it is a good idea to call create_handle() in the 
constructor. Why not just pass a handle into the Resource?
Aug 29 2015
parent reply "ponce" <contact gam3sfrommars.fr> writes:
On Saturday, 29 August 2015 at 16:12:52 UTC, skoppe wrote:

 I don't think it is a good idea to call create_handle() in the 
 constructor. Why not just pass a handle into the Resource?


This isn't related to the topic.
Aug 30 2015
parent reply "skoppe" <mail skoppe.eu> writes:
On Sunday, 30 August 2015 at 09:54:31 UTC, ponce wrote:

 On Saturday, 29 August 2015 at 16:12:52 UTC, skoppe wrote:

 I don't think it is a good idea to call create_handle() in the 
 constructor. Why not just pass a handle into the Resource?


 This isn't related to the topic.


By putting create_handle in the constructor, you inevitably end 
up putting free_handle in some (destructor) function. The 
problems you are having might be different/easier when you make 
something else do the (de)construction. Like, say, a 
ResourceManager.
Aug 30 2015
parent "ponce" <contact gam3sfrommars.fr> writes:
On Sunday, 30 August 2015 at 11:45:36 UTC, skoppe wrote:

 On Sunday, 30 August 2015 at 09:54:31 UTC, ponce wrote:

 On Saturday, 29 August 2015 at 16:12:52 UTC, skoppe wrote:

 I don't think it is a good idea to call create_handle() in 
 the constructor. Why not just pass a handle into the Resource?


 This isn't related to the topic.


 By putting create_handle in the constructor, you inevitably end 
 up putting free_handle in some (destructor) function. The 
 problems you are having might be different/easier when you make 
 something else do the (de)construction. Like, say, a 
 ResourceManager.


The handle is not important, the idiom applies equally to any 
class with a non-trivial destructor.
Aug 30 2015
prev sibling next sibling parent reply "ZombineDev" <valid_email he.re> writes:
On Saturday, 29 August 2015 at 13:14:26 UTC, ponce wrote:

 ----------------

 ensureNotInGC() is implemented like this:

 ----------------

 void ensureNotInGC(string resourceName) nothrow
 {
     debug
     {
         import core.exception;
         try
         {
             import core.memory;
             void* p = GC.malloc(1); // not ideal since it 
 allocates
             return;
         }
         catch(InvalidMemoryOperationError e)
         {
             import core.stdc.stdio;
             fprintf(stderr, "Error: clean-up of %s incorrectly 
 depends on destructors called by the GC.\n", resourceName.ptr);
             assert(false); // crash
         }
     }
 }

 --------------

...


BTW, you can use GC.free, instead of GC.malloc, with any non-zero 
invalid memory address, for example: 
http://dpaste.dzfl.pl/af0dc9aaa29d

A more robust solution would be to check if the gcx.running flag 
is raised, or if the GC lock is taken, like this: 
https://gist.github.com/ZombineDev/14076777dff7d879d659, however 
this is not currently possible, because the _gc variable in 
gc.proxy is private and by default gc.proxy is not part of the 
include files.

Since it's really straightforward to expose the information to 
the user, I think this would an easy enhancement.
Aug 30 2015
parent reply "ponce" <contact gam3sfrommars.fr> writes:
On Sunday, 30 August 2015 at 17:00:23 UTC, ZombineDev wrote:

 On Saturday, 29 August 2015 at 13:14:26 UTC, ponce wrote:

 ----------------

 ensureNotInGC() is implemented like this:

 ----------------

 void ensureNotInGC(string resourceName) nothrow
 {
     debug
     {
         import core.exception;
         try
         {
             import core.memory;
             void* p = GC.malloc(1); // not ideal since it 
 allocates
             return;
         }
         catch(InvalidMemoryOperationError e)
         {
             import core.stdc.stdio;
             fprintf(stderr, "Error: clean-up of %s incorrectly 
 depends on destructors called by the GC.\n", resourceName.ptr);
             assert(false); // crash
         }
     }
 }

 --------------

...


 BTW, you can use GC.free, instead of GC.malloc, with any 
 non-zero invalid memory address, for example: 
 http://dpaste.dzfl.pl/af0dc9aaa29d


In the case the destructor isn't called by the GC, the call must 
succeed.
GC.malloc(1) fits the bill but it's a waste of time and memory 
indeed. GC.free(<invalid-adress>) would fail in both cases if I 
understand correctly.


 A more robust solution would be to check if the gcx.running 
 flag is raised, or if the GC lock is taken, like this: 
 https://gist.github.com/ZombineDev/14076777dff7d879d659, 
 however this is not currently possible, because the _gc 
 variable in gc.proxy is private and by default gc.proxy is not 
 part of the include files.


Those two would work better than GC.malloc indeed. A nice thing 
is that it seems we don't need synchronization so _gc.gcx.running 
would be ideal.


 Since it's really straightforward to expose the information to 
 the user, I think this would an easy enhancement.


Perhaps as a getter like GC.isRunning()?
Aug 30 2015
parent reply "ZombineDev" <valid_email he.re> writes:
On Sunday, 30 August 2015 at 20:44:17 UTC, ponce wrote:

 In the case the destructor isn't called by the GC, the call 
 must succeed.
 GC.malloc(1) fits the bill but it's a waste of time and memory 
 indeed. GC.free(<invalid-adress>) would fail in both cases if I 
 understand correctly.


As you can see from the output 
(http://dpaste.dzfl.pl/aa004554034a), when GC.free(cast(void*)1) 
is called in main() it doesn't throw. It only throws in the 
destructor of A, because a GC collection is taking place.

If you look at the implementation, it is more or less guaranteed 
that:
a) GC.free() during collection -> InvalidMemoryOperationError [1]
b) GC.free() with invalid pointer -> no-op [2]


 Perhaps as a getter like GC.isRunning()?


Yeah, that's what I had in mind. But maybe it also makes sense to 
provide a way to take the GC lock? Of course, such method should 
definitely be marked as  system.

[1]: 
https://github.com/D-Programming-Language/druntime/blob/master/src/gc/gc.d#L879
[2]: 
https://github.com/D-Programming-Language/druntime/blob/master/src/gc/gc.d#L888
Aug 30 2015
parent reply "ponce" <contact gam3sfrommars.fr> writes:
On Sunday, 30 August 2015 at 21:52:37 UTC, ZombineDev wrote:

 On Sunday, 30 August 2015 at 20:44:17 UTC, ponce wrote:

 In the case the destructor isn't called by the GC, the call 
 must succeed.
 GC.malloc(1) fits the bill but it's a waste of time and memory 
 indeed. GC.free(<invalid-adress>) would fail in both cases if 
 I understand correctly.


 As you can see from the output 
 (http://dpaste.dzfl.pl/aa004554034a), when 
 GC.free(cast(void*)1) is called in main() it doesn't throw. It 
 only throws in the destructor of A, because a GC collection is 
 taking place.

 If you look at the implementation, it is more or less 
 guaranteed that:
 a) GC.free() during collection -> InvalidMemoryOperationError 
 [1]
 b) GC.free() with invalid pointer -> no-op [2]


Cool thing.


 Perhaps as a getter like GC.isRunning()?


 Yeah, that's what I had in mind. But maybe it also makes sense 
 to provide a way to take the GC lock? Of course, such method 
 should definitely be marked as  system.

 [1]: 
 https://github.com/D-Programming-Language/druntime/blob/master/src/gc/gc.d#L879
 [2]: 
 https://github.com/D-Programming-Language/druntime/blob/master/src/gc/gc.d#L888


I'm not sure there is even a need for synchronization since other 
threads that wan't to allocate try to take the GC lock while the 
GC-hijacked thread calls destructors.

And if the destructor isn't called by the GC, I don't see a 
problem either.tre
Aug 30 2015
parent "Brian Schott" <briancschott gmail.com> writes:
On Sunday, 30 August 2015 at 21:59:59 UTC, ponce wrote:

 I'm not sure there is even a need for synchronization since 
 other threads that wan't to allocate try to take the GC lock 
 while the GC-hijacked thread calls destructors.

 And if the destructor isn't called by the GC, I don't see a 
 problem either.tre


I'd like to point out that this problem is showing up in the 
design of std.experimental.allocator. Any allocator that 
deallocates its memory in a destructor cannot a) use GCAllocator 
(directly or indirectly) and b) be used by CAllocatorImpl
at the same time.

As of right now there's no compile-time check that disallows this 
and there is no way for the allocators to know that they 
shouldn't do things like call deallocateAll() on an allocator 
when the GC is running.
Aug 30 2015
prev sibling parent reply "ponce" <contact gam3sfrommars.fr> writes:
On Saturday, 29 August 2015 at 13:14:26 UTC, ponce wrote:

 Looks ugly? Yes, but it makes the GC acts as a cheap leak 
 detector, giving accurate messages for still opened resources.


So, let me tell a little success story while using the "GC-proof" 
resource classes.
I tested the above idiom on some code I was a bit liberal with 
and with no thought gone into clean-up.
I found all resource leaks in ~1 hour, and there was several of 
them.

The open question that remains is: "can ~this() really be called 
multiple times?", that would made the idiom less ugly (had to add 
boolean flags for most resources).
Aug 31 2015
parent reply "Sebastiaan Koppe" <mail skoppe.eu> writes:
On Monday, 31 August 2015 at 11:29:21 UTC, ponce wrote:

 On Saturday, 29 August 2015 at 13:14:26 UTC, ponce wrote:

 Looks ugly? Yes, but it makes the GC acts as a cheap leak 
 detector, giving accurate messages for still opened resources.


 So, let me tell a little success story while using the 
 "GC-proof" resource classes.
 I tested the above idiom on some code I was a bit liberal with 
 and with no thought gone into clean-up.
 I found all resource leaks in ~1 hour, and there was several of 
 them.

 The open question that remains is: "can ~this() really be 
 called multiple times?", that would made the idiom less ugly 
 (had to add boolean flags for most resources).


It feels like calling ~this() twice is UB. What about:

```
class MyResource
{
     void* handle;
     this()
     {
         handle = create_handle();
     }
     close()
     {
         if (handle)
             free_handle(handle)
         handle = null;
     }
     ~this()
     {
         enforce(!handle,"Resource leak");
     }
}
```

Nice pattern either way.
Aug 31 2015
parent reply "ponce" <contact gam3sfrommars.fr> writes:
On Monday, 31 August 2015 at 12:54:05 UTC, Sebastiaan Koppe wrote:

 What about:

 ```
 class MyResource
 {
     void* handle;
     this()
     {
         handle = create_handle();
     }
     close()
     {
         if (handle)
             free_handle(handle)
         handle = null;
     }
     ~this()
     {
         enforce(!handle,"Resource leak");
     }
 }
 ```




Unique!T destructor calls delete which calls ~this() not close()
RefCounted!T and scoped!T call .destroy which calls ~this() not 
close()
We really want one thing there.
Aug 31 2015
parent reply "byron" <byron.heads gmail.com> writes:
On Monday, 31 August 2015 at 13:35:54 UTC, ponce wrote:

 On Monday, 31 August 2015 at 12:54:05 UTC, Sebastiaan Koppe 
 wrote:

 What about:

 ```
 class MyResource
 {
     void* handle;
     this()
     {
         handle = create_handle();
     }
     close()
     {
         if (handle)
             free_handle(handle)
         handle = null;
     }
     ~this()
     {
         enforce(!handle,"Resource leak");
     }
 }
 ```




 Unique!T destructor calls delete which calls ~this() not close()
 RefCounted!T and scoped!T call .destroy which calls ~this() not 
 close()
 We really want one thing there.


I normally stick with this pattern when dealing with resource, 
though I would only uses a class if I needed an interface or 
inheritance..


```
class MyResource
{
     void* handle;
     this()
     {
         handle = create_handle();
     }

     close()
     {
         if (handle !is null)
         {
             synchronized {
                 if (handle !is null) {
                     free_handle(handle);
                 }
             }
             handle = null;
         }
     }

     ~this()
     {
         close();
     }
}
```
Aug 31 2015
parent reply "ponce" <contact gam3sfrommars.fr> writes:
On Monday, 31 August 2015 at 14:18:43 UTC, byron wrote:

 ```
 class MyResource
 {
     void* handle;
     this()
     {
         handle = create_handle();
     }

     close()
     {
         if (handle !is null)
         {
             synchronized {
                 if (handle !is null) {
                     free_handle(handle);
                 }
             }
             handle = null;
         }
     }

     ~this()
     {
         close();
     }
 }
 ```


Used to do like that modulo the synchronized (which makes sense 
considering destructors are run after all threads are unpaused). 
The problem is that relying on GC destructors tends to come bite 
later imho.
Aug 31 2015
parent reply "cym13" <cpicard openmailbox.org> writes:
On Monday, 31 August 2015 at 14:56:36 UTC, ponce wrote:

 On Monday, 31 August 2015 at 14:18:43 UTC, byron wrote:

 ```
 class MyResource
 {
     void* handle;
     this()
     {
         handle = create_handle();
     }

     close()
     {
         if (handle !is null)
         {
             synchronized {
                 if (handle !is null) {
                     free_handle(handle);
                 }
             }
             handle = null;
         }
     }

     ~this()
     {
         close();
     }
 }
 ```


 Used to do like that modulo the synchronized (which makes sense 
 considering destructors are run after all threads are 
 unpaused). The problem is that relying on GC destructors tends 
 to come bite later imho.


By the way, why not add this to your idiom page? I think it would 
be valuable information.
Sep 01 2015
parent "ponce" <contact gam3sfrommars.fr> writes:
On Tuesday, 1 September 2015 at 08:26:02 UTC, cym13 wrote:

 By the way, why not add this to your idiom page? I think it 
 would be valuable information.


Already there! 
http://p0nce.github.io/d-idioms/#GC-proof-resource-class
Sep 01 2015