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