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.

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.

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.

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.

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()?

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

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

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.

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).

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.