A bug in my code

[Sergey Gromov]

bearophile <bearophileHUGS at lycos.com> wrote:
> Sergey Gromov, thank you for your explanations, you are quite clear, I 
> have understood everything you have explained :-) And your code works.

You're welcome. :)

> > addRange(&new_block.next, &new_block.data+1);
> 
> The trick of adding a range of pointers to pointers is cute :-)
> I presume that +1 is necessary because it's an interval open on the 
> right, even if the Phobos docs don't tell it:
> void addRange(void* pbot, void* ptop);

Well, I assumed that.  Void doesn't have size after all, so I thought 
pbot and ptop were specifying byte boundaries, not bytes or words 
themselves.

> >Both require Block.~this() to remove any unneeded roots or ranges<
> 
> Okay, I understand now. Jarrett Billingsley has told me that those 
> ranges are inefficient, so I hope the roots are more efficient.
> 
> Using your second solution:
> addRange(&new_block.next, &new_block.data+1);
> 
> I have added this destructor:
> 
> ~this() {
>     while (this.list_head != null) {
>         Block* next_ptr = this.list_head.next;
>         std.gc.removeRange(&this.list_head.next);
>         std.gc.realloc(this.list_head.data, 0);
>         std.gc.realloc(this.list_head, 0);
>         this.list_head = next_ptr;
>     }
> }

I think that
>         std.gc.realloc(this.list_head.data, 0);
>         std.gc.realloc(this.list_head, 0);
is dangerous, not to mention redundant.  If there are references to 
elements in data other than in Blocks, reallocing data would silently 
invalidate those references eventually causing GPF (segfault).  The 
correct code is

> ~this() {
>     while (this.list_head != null) {
>         Block* next_ptr = this.list_head.next;
>         std.gc.removeRange(&this.list_head.next);
>         this.list_head = next_ptr;
>     }
> }

What it does is:
1. next_ptr makes a reference to list_head.next on the stack so that it 
isn't GC'd immediately.
2. removeRange() prevents GC from scanning list_head.next and 
list_head.data for pointers.  The next is still referenced from stack so 
it lives yet.  The data is not referenced from list_head anymore, and if 
there are no other references to it, it is GC'd eventually.  If there 
are references though, it lives while those references are alive, which 
is expected and correct.
3. list_head = next_ptr removes the last reference to the former 
list_head so it gets GC'd automatically later.

> Now I think I understand the Phobos GC API a bit better. But I'll have 
> to be really careful, because doing mistakes with such things looks very 
> easy still for me.