On 7/28/23 11:15 AM, IchorDev wrote:

It seems like it's not __gshared at all, but misusing malloc with GC pointers.

this(long n){
     (){
         cache = new typeof(cache);
         assert(cache);
         import core.memory;
         core.memory.GC.addRoot(cast(void*)cache);
     }();
     // ...

This is the wrong approach, it's the allocating call that should add the root (actually a range).

For instance, the mutex is not added, that might be collected. Or if you add more GC-pointing things into the class, that could be a problem.

What I'd do is:

T alloc(T, A...)(auto ref A args){
    enum classSize = __traits(classInstanceSize, T);
    void* mem = core.stdc.stdlib.malloc(classSize);
    assert(mem !is null, "Out of memory");
    core.memory.GC.addRange(mem[0 .. classSize]);
    scope(failure) {
       core.memory.GC.removeRange(mem[0 .. classSize]);
       core.stdc.stdlib.free(mem);
    }
    T inst = cast(T)mem;
    inst.emplace(__traits(parameters));
    return inst;
}

And of course, a dealloc that removes the range should also be added.

-Steve

On Friday, 28 July 2023 at 21:22:01 UTC, Steven Schveighoffer wrote:

T alloc(T, A...)(auto ref A args){
    enum classSize = __traits(classInstanceSize, T);
    void* mem = core.stdc.stdlib.malloc(classSize);
    assert(mem !is null, "Out of memory");
    core.memory.GC.addRange(mem[0 .. classSize]);
    scope(failure) {
       core.memory.GC.removeRange(mem[0 .. classSize]);
       core.stdc.stdlib.free(mem);
    }
    T inst = cast(T)mem;
    inst.emplace(__traits(parameters));
    return inst;
}

This also works unless I do a foreach over the AA's keys & values (foreach(k,v; aa)), in which case it still segfaults at random. Deconstructing the foreach into a for works though, for whatever reason.

On 8/10/23 1:59 AM, IchorDev wrote:

That shouldn't matter.

The key thing is, if you are going to use malloc, you need to register the memory with the GC. The GC doesn't scan memory it doesn't know about.

But my main point was the class itself shouldn't be registering itself -- it did not make the decision to malloc. You should register where you allocate.

I also highly recommend using emplace to handle all the sticky issues with lifetime/construction.

-Steve

On Thursday, 10 August 2023 at 15:20:28 UTC, Steven Schveighoffer wrote:

Well, it does here. The AA is mutated during the loop, so perhaps this is an optimisation quirk where it works with for but segfaults in foreach? I've pretty thoroughly abused the for version and I haven't gotten it to segfault yet.

Have not run into the aforementioned sticky issues yet, but I can't even find emplace's docs anywhere now. I recall it being incompatible with classes that have @nogc/nothrow constructors though, which made it pretty useless to me, and it wouldn't work with BetterC, which was a requirement for the allocation wrapper I was writing at the time.

On 8/12/23 5:55 AM, IchorDev wrote:

oh yeah. That is not allowed. Any mutation of the AA during iteration can invalidate existing foreach or ranges over the AA.

Basically, a rehash can cause the buckets to jumble up, and in that case, the "current index" can be changed to point at a null bucket.

More here: https://dlang.org/spec/statement.html#foreach_restrictions

In fact, that statement is way too broad. Invalidation of iteration should be based on the type's requirements.

We really should put a note in the AA spec page.

https://dlang.org/phobos/core_lifetime.html#.emplace

It probably won't work with betterC, but that's probably just because of linker errors.

Any attribute requirements would be inferred based on the attributes of your constructor, because emplace is a template.

-Steve

On Tuesday, 15 August 2023 at 17:36:13 UTC, Steven Schveighoffer wrote:

And yet this apparently doesn’t apply to an equivalent for somehow. Perhaps foreach shouldn’t have this arbitrary problem in the first place…

Probably yeah.

Well the docs don’t say that at all, and the code is an unreadable mess. I dunno how anymore is meant to figure that out?