www.digitalmars.com         C & C++   DMDScript  

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

reply Charles Hixson <charleshixsn earthlink.net> writes:
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

Why does the second scope need to be commented out to avoid:
*** glibc detected *** ./tctest: double free or corruption (fasttop): 
0x00000000015a5980 ***

I thought that scope(exit) statements were executed in the reverse of 
the order that they were executed.

FWIW, it's *probably* safe to leave it commented out, but I'm not really 
sure because it depends on the internals of a C library that I've been 
considering opaque.
N.B.:  If I manually insert the cursor.close method at the end of the 
routine, it works as expected.  The problem only occurs if I expect 
scope(exit) to issue the close.  (I suppose I could just add another 
level of parens to force the order, but should I need to?)
Feb 24 2013
parent reply "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
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 -- Written on the window of a clothing store: No shirt, no shoes, no service.
Feb 24 2013
parent reply Charles Hixson <charleshixsn earthlink.net> writes:
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.
Feb 25 2013
parent reply Ben Davis <entheh cantab.net> writes:
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 :)
Feb 26 2013
parent Charles Hixson <charleshixsn earthlink.net> writes:
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*
Feb 27 2013