Re: memory corruption inside connector/c++: whose bug?

[Jean-Denis Muys]

On Oct 31, 2009, at 21:28 , Jean-Denis Muys wrote:

> On Oct 31, 2009, at 17:55 , Jean-Denis Muys wrote:
>
>>
>> So could it be an obscure bug in connector/C++? Under which  
>> conditions could client code lead Connector/C++ to write into a  
>> block it has just freed?
>>

With some help from Jens Alfke, I have managed to find one answer to  
this question (among how many?), and to me, Connector/C++ is buggy.  
Consider a call to:

sql::ResultSet * MySQL_Prepared_Statement::executeQuery()


It constructs a MySQL_Prepared_ResultSet through this constructor:


MySQL_Prepared_ResultSet::MySQL_Prepared_ResultSet(
			MYSQL_STMT * s,
			MySQL_ResultBind * r_bind,
			sql::ResultSet::enum_type rset_type,
			MySQL_Prepared_Statement * par,
			sql::mysql::util::my_shared_ptr< MySQL_DebugLogger > * l
		)


Now pay attention to second argument to that constructor:  
MySQL_ResultBind * r_bind.

The constructor doesn't know that this pointer is *not* his to manage,  
because the caller has already put it into an auto_ptr:


sql::ResultSet * MySQL_Prepared_Statement::executeQuery()
{
[...]
	std::auto_ptr< MySQL_ResultBind > result_bind(new MySQL_ResultBind 
(stmt, logger));
	
	sql::ResultSet * tmp = new MySQL_Prepared_ResultSet(stmt,  
result_bind.get(), tmp_type, this, logger);

[...]
}

Yet the constructor *also* puts it into an auto_ptr:


MySQL_Prepared_ResultSet::MySQL_Prepared_ResultSet(
	[...]
			MySQL_ResultBind * r_bind,
	[...]

		)
	: result_bind(r_bind)
[...]


So now at this point, the *same* pointer is inside *two* auto_ptr.  
This is a recipe for disaster. And disaster occurs, because, still in  
the constructor, an exception can occur. For me it once occurred on  
this call:

MySQL_Prepared_ResultSet::MySQL_Prepared_ResultSet([...])
	: [...]
{
[...]
	result_bind->bindResult();
[...]
}

So now, an exception in a constructor means the partially constructed  
object is destructed (thanks Jens). So our nice result_bind auto_ptr  
is destructed, which means the r_bind object is destructed too.

Now the caller has a *bad* auto_ptr pointing to a destructed object.  
That auto_ptr being an automatic variable, it is destructed too, and  
boom, r_bind's destructor is called twice on the same object.

Then all bets are off. Memory corruption follows.

Note that without an exception, the caller handles the situation:


sql::ResultSet * MySQL_Prepared_Statement::executeQuery()
{
[...]
	std::auto_ptr< MySQL_ResultBind > result_bind(new MySQL_ResultBind 
(stmt, logger));
	
	sql::ResultSet * tmp = new MySQL_Prepared_ResultSet(stmt,  
result_bind.get(), tmp_type, this, logger);
	result_bind.release();
[...]
}

But that call to release is too late. In the short time while the  
*same* pointer is in *two* auto_ptr, hell can break loose, and Murphy  
ensures that it will.

My discovery of that bad code pattern in Connector/C++ makes me very  
wary of using it.

I really hope my ringing this alarm call will be answered.

Jean-Denis