Description:
I have been having some nasty memory corruption issues in my code using Connector/C++.
While I can't exclude bugs in my code, I found one code example in Connector/C++ that was the direct cause of one of my memory corruption crashes:
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
How to repeat:
Call executeQuery on a prepared statement that has no result, for example an UPDATE.
Suggested fix:
don't put a pointer into two auto_ptr. As a rule, don't store the result of auto_ptr::get() anywhere: neither in an object, nor pass it as an argument. Also possibly, avoid using auto_ptr as class members.
This implies a heavy code review of Connector/C++