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

[Charles Hixson]

On 02/26/2013 04:24 PM, Ben Davis wrote:
> 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 :)

I want the design to work whether the struct is allocated on the stack 
or on the heap, so while in this particular instance it was on the 
stack, I don't want a design that assumes that.  After I get it working 
right I intend to wrap everything up into probably two structs. So I 
*can't* rely on the destructor being called...not in a timely way.  I'm 
using scope(exit) because try{}finally{} would be a nuisance...but the 
database must ensure that the cursor is properly closed.  (At least I'm 
assuming so.)

Still, you were right that the library couldn't set my local pointer to 
null.  So I made *another* mistake.  *grrr*