scope(exit) & stack => double free or corruption (fasttop) ... help?

[Ben Davis]

On 26/02/2013 06:21, Charles Hixson wrote:
> On 02/24/2013 05:39 PM, H. S. Teoh wrote:
>> On Sun, Feb 24, 2013 at 03:14:01PM -0800, Charles Hixson wrote:
>>> Given a struct with:
>>>
>>> ~this()
>>> {  close();    }
>>>
>>> void    close()
>>> {  if    (currentKey !is null)    currentKey    =    null;
>>>     if    (cursor is null)    return;
>>>     tcbdbcurdel(cursor);
>>> }
>>>
>>> and:
>>>
>>>    scope (exit)    if (bdb !is null)  tcbdbclose(bdb);
>>>    //scope(exit)    cur.close;       //<<- cur is the struct noted above
>> [...]
>>
>> The struct dtor is automatically called upon exiting the scope, so your
>> scope(exit) here is redundant, and is the cause of the double free.
>>
>>
>> T
>>
>
> Sorry, but that wasn't the answer...which was trivial, when I saw it.  I
> needed to change the close() routine to:
> void    close()
> {  if    (currentKey !is null)    currentKey    =    null;
>     if    (cursor is null)    return;
>     tcbdbcurdel(cursor);
>     cursor  = null;
> }
>
> Apparently the library didn't null the cursor after it closed (deleted) it.

You're both right.

Your struct is presumably declared as a simple local variable, like this:

someFunction() {
   YOUR_STRUCT cur;
   ...
}

What we're saying is that cur's destructor is called automatically as 
soon as execution reaches the }, even if an exception is thrown or a 
break/continue/return/goto jumps out.

So when you write "scope (exit) cur.close();", you're queueing the 
close() to happen under the same circumstances. This doesn't stop the 
destructor call happening. Your destructor calls close() too, so it gets 
called twice.

You should also know that the library you're using *can't* set 'cursor' 
to null for you, because you're in control of the memory location where 
'cursor' is stored (it's inside your struct), and when you pass it to 
tcbdbcurdel(), you're only passing a copy of the value. In order for it 
to set it to null for you, you would have pass a pointer to the value, 
by writing '&cursor'. (This is not strictly true for D or C++ functions 
since they can take 'references' which are implicit pointers, but you 
said it's a C library - and in any case, 'ref' parameters will usually 
be much more obviously 'ref' parameters than this one, if the API is 
well designed.)

So - setting cursor to null is a good safe fix, as it makes it safe to 
call close() more than once - but you also don't need the 'scope 
(exit)', as you can rely on the destructor being called.

(But further to that - you can only rely on the destructor being called 
if it's a struct (and not a pointer to one), or a scope class (a class 
with the 'scope' attribute). Normal classes will be destroyed eventually 
by the GC, but not at a well-defined time, and there's (probably?) no 
guarantee it'll be called before the program exits.)

I guess D isn't as simple as it wanted to be! But it is powerful :)