Opening database in invalid location causes memory leak

I am using qt 4.5.3 to access the sqlite database, like this :

class db : private boost::noncopyable
{
 public:
  db( QString file ) : filename( file ),
                       realdb( NULL ),
                       theConnectionEstablished( false )
  {
  }
  ~db()
  {
    if ( NULL != realdb.get() )
    {
      realdb.reset( NULL );
    }
    if ( theConnectionEstablished )
    {
      QSqlDatabase::removeDatabase( "ConnName" );
    }
  }

  void open()
  {
    realdb.reset( new QSqlDatabase( QSqlDatabase::addDatabase( "QSQLITE", "ConnName" ) ) );
    theConnectionEstablished = true;

    // open the db
    realdb->setDatabaseName( filename );
    if ( ! realdb->open() )
    {
        const QSqlError dbError = realdb->lastError();
        const QString errorDesc = "Error opening the database : " + filename +
                                  "\nDatabase error : " + dbError.databaseText() +
                                  "\nDatabase driver error : " + dbError.driverText();

        // DatabaseError is a class type which accepts the std::string for logging purposes
        throw DatabaseError( errorDesc.toStdString() );
    }
  }

  const QString filename;
  std::auto_ptr<QSqlDatabase> realdb;
  bool theConnectionEstablished;
};

Now if I try to test this case like this (I am using cxxtest) :

void test_failed_connection()
{
  db obj( "/" );
  TS_ASSERT_THROWS( obj.open(), DatabaseError );
}

I get a memory leak reported by valgrind :

<error>
  <unique>0x5b</unique>
  <tid>1</tid>
  <kind>Leak_DefinitelyLost</kind>
  <what>986 (384 direct, 602 indirect) bytes in 1 blocks are definitely lost in loss record 23 of 23</what>
  <leakedbytes>986</leakedbytes>
  <leakedblocks>1</leakedblocks>
  <stack>
    <frame>
      <ip>0x4006D3E</ip>
      <obj>/opt/valgrind341/lib/valgrind/x86-linux/vgpreload_memcheck.so</obj>
      <fn>malloc</fn>
      <dir>/home/slawomir/valgrind-3.4.1/build/valgrind-3.4.1/coregrind/m_replacemalloc</dir>
      <file>vg_replace_malloc.c</file>
      <line>207</line>
    </frame>
    <frame>
      <ip>0x67FADC4</ip>
      <obj>/usr/lib/libsqlite3.so.0.8.6</obj>
      <fn>sqlite3_malloc</fn>
    </frame>
    <frame>
      <ip>0x67FAF13</ip>
      <obj>/usr/lib/libsqlite3.so.0.8.6</obj>
    </frame>
    <frame>
      <ip>0x6816DA3</ip>
      <obj>/usr/lib/libsqlite3.so.0.8.6</obj>
    </frame>
    <frame>
      <ip>0x68175FD</ip>
      <obj>/usr/lib/libsqlite3.so.0.8.6<开发者_StackOverflow中文版;/obj>
      <fn>sqlite3_open16</fn>
    </frame>
    <frame>
      <ip>0x40DDEF9</ip>
      <obj>/usr/lib/qt4/plugins/sqldrivers/libqsqlite.so</obj>
    </frame>
    <frame>
      <ip>0x7F34AE0</ip>
      <obj>/usr/lib/libQtSql.so.4.5.2</obj>
      <fn>QSqlDatabase::open()</fn>
    </frame>
    </frame>
  </stack>
</error>

Does anyone know how to fix this leak?

---

Had a browse through the Qt and sqlite sources ... interesting.

Reading the manual for sqlite3_open16(), http://www.sqlite.org/c3ref/open.html, one finds the following quote:

Whether or not an error occurs when it is opened, resources associated with the database connection handle should be released by passing it to sqlite3_close() when it is no longer required.

QSQLiteDriver::close() seems to be calling that, http://qt.gitorious.org/qt/qt/blobs/4.7/src/sql/drivers/sqlite/qsql_sqlite.cpp, only in case of the open having been successful. SQLite's documentation might indicate that sqlite3_close() should be called either case.

On the other hand, http://www.sqlite.org/c3ref/close.html claims to be a no-op if NULL is passed for a handle (which would be if the open failed). A look into the SQLite sourcecode (DIY - I don't know a web source browser interface for it) confirms that, it just returns if called with NULL.

Well - now for the nitty-gritty fun of the issue...

Naively, one assumes that a failure of sqlite3_open*() would imply a NULL db handle. But according to SQLite's sources, read openDatabase() in main.c, that's not true - the call can fail but still return you a non-NULL db handle.

Qt looks like it assumes a failure to open the DB connection implies receiving a NULL db handle. But that's not what SQLite does. The documentation could be clearer, though.

Try adding that close into QSQLiteDriver::open() and see if it fixes the leak. If so, file a bug with the Qt guys, and another one with the SQLite folks to have the documentation clarified ;-)

---

Your code

realdb.reset( new QSqlDatabase( QSqlDatabase::addDatabase( "QSQLITE", "ConnName" ) ) );

looks rather strange, not sure why it compiles. I don't see a constructor of QSqlDatabase that takes QSqlDatabase* as a parameter.

You called QSqlDatabase::addDatabase which returns QSqlDatabase * then constructed another QSqlDatabase using new and passing that as the parameter.

Instead of auto_ptr you could use boost::shared_ptr then reset with

realdb.reset(QSqlDatabase::addDatabase( "QSQLITE", "ConnName" ), QSqlDatabase::removeDatabase);

Beware that removeDatabase can leave a resource leak if there are open queries on it.