D2 weak references

[Jason House]

I'm finally looking at the gc source code.  From what I can tell, memory is 
allocated in large chunks (pools), and divided into 16 byte chunks.  
Attributes are not all packed together like the BlkInfo of the spec implies, 
so some so access to that stuff is safer than I would have thought.

Here's a weak ref implementation that I just whipped together under the 
assumption it goes into druntime/src/gc/basic/gcx.d.  I think it is sane.  I 
haven't scoured the gc's code to ensure everything is legit.  I declared it 
as a class mostly because value semantics don't make much sense and I'm too 
lazy/tired to do the sample code as a 100% correct struct definition.

My two biggest concerns: 

1. Is the gc lock held while running a constructor? If not and it's needed 
to do what it needs, where can the hook be added to make the construction of 
a weak reference special?

2. Will the assignment to noscan get overwritten / hit a race condition?  It 
is possible to hide the pointer inside a size_t instead, but given the 
intimate tying to the GC such tricks just make the code tougher to read.

class weakRef(T) if (is(T==class))
{
    this(T _t){
        // set up tracking for the weakly referenced object
        t = _t;
        pool = findPool(t);
        rt_attachDisposeEvent(&t, &sweep);

        // tell the gc to not scan the weak reference
        weakRefPool = findPool(&this);
        weakRefPool.noscan[index(weakRefPool,&this)] = true;
    }

    T opCall(){
        T tmp = t; // force future conservative GC scan to find the pointer
        if ( (tmp is null) || (pool.mark[index] == false) ){
            t = null;
            return null;
        }
        tmp = t; // avoid race where t was replaced with new data
        return tmp;
    }

private:
    T t;
    Pool* pool;
  
    size_t index(Pool *p, byte *element){ return (element-p)/16; }
    size_t index(){ return index(pool,&t); }

    void sweep( Object o ){
        t = null;
        pool = null;
    }
}


Comments and criticism are welcome.







Leandro Lucarella wrote:

> Frits van Bommel, el 22 de abril a las 23:39 me escribiste:
>> >>>I think this approach is both simple and efficient, it doesn't have
>> >>>races at all (unless I'm missing something, of course). The only
>> >>>problem is it's too tied to the current GC implementation. But I think
>> >>>any weak ref implementation will, so it looks like weak ref really
>> >>>belong to the GC.
>> >>
>> >>I think you're missing something. Consider this sequence of events:
>> >>- Object A is collected by the GC. (And its mark bit M is cleared)
>> >>- Object B is allocated, and gets the memory formerly used by object A.
>> >>Mark bit M is set. - A weak reference to A gets used; it checks mark
>> >>bit M, which is set...
>> >>
>> >>In other words, the "this object was collected" state needs to be
>> >>stored in the weak reference, not in the object that got collected (or
>> >>anything tied to its memory location) because that memory may get
>> >>reused.
>> >
>> >That's where finalizers come in. They can mark the weak ref as invalid
>> >so that you don't care what was returned. In my (recent?) pseudo code,
>> >the weak ref was decoded, the mark bit was checked, and then the weak
>> >ref was decoded again. The final decoding is to solve the race with the
>> >finalizer.
>> 
>> Will this work if B was allocated in another thread, right after it gets
>> resumed (i.e. possibly before the finalizer has a chance to run)?
>> 
>> ... oh, maybe the GC lock gets held even after other threads get
>> resumed, so the allocation will wait for finalizers to run? (just
>> thought of that)
> 
> That's exactly what happens. So I'm not seeing any flaws so far with my
> proposal.
> 
>> >Even with such precautions, a lock free implementation has one more
>> >problem: what happens if the memory block was released to the os?
>> >That's a segfault :( if other data is checked to avoid such situations,
>> >that's likely where gc locks come into the picture... I'm pretty sure
>> >Sean said querying the attribute bits requires a lock. It may be for
>> >this reason. I still have to look at the code though...
>> 
>> You mean it would segfault on checking the mark bit? Can't the GC just
>> say the mark bit was not set for any pointer not pointing into
>> a currently-allocated pool?  But yeah, checking if that address is part
>> of an allocated pool may require holding the GC lock...
> 
> In the patch I posted this is already happening. The mark bit is returned
> as part of the BlkAttr belonging to a memory block. If the pointer passed
> to gc_getAttr() (or gc_query()) don't point to a GC allocated memory
> block, 0 is returned.
> 
> I any case, I can't see how that could happen if there are still
> notifications before freeing the memory block in the sweep phase.
> 
> The sequence is like this:
> 
> 0) All live cells has the mark bit set
> 1) Acquire the GC lock
> 2) Stop the world
> 3) Clear mark bits
> 4) Mark
> 5) Start the world
> 6) Sweep (calling finalizing notification callbacks)
> 7) Release the GC lock
> 
> Then, you are guaranteed to access to valid memory (mark bit or even
> the entire memory block) until the notification comes. When the weak
> reference receive the notification it invalidates the reference and don't
> rely anymore in the mark bit.
> 
> But I just found a flaw in my patch (not the proposal), right now, as you
> say, the gc_query() and gc_getAttr() functions are acquiring the GC lock.
> Some non-locking interface should be exposed that calls to queryNoSync()
> and getAttrNoSync(). It should be safe for this limited usage as long as
> there are not concurrent calls to setAttr()/clrAttr() to that memory
> block (I think).
>