Fixing crash in SafeBrowsingDatabaseBloom:

When SafeBrowsingDatabaseBloom is going out of scope (see stack trace below) Close() is called.
When insert_transaction_ is non NULL, it holds a pointer to the database, |db_|, which is owned by SafeBrowsingDatabaseBloom.
Close() closes the database but did not free |insert_transaction_| which causes |insert_transaction_| to attempt to rollback the transaction with an invalid database pointer.

The fix is simply to reset the transaction before closing the database.

Stack:
0:009> kP
ChildEBP RetAddr  
0579fa14 027e73e0 ntdll!RtlEnterCriticalSection+0xb
0579fa24 027958e6 chrome_12d0000!winMutexEnter(
			struct sqlite3_mutex * p = 0xdddddddd)+0x10 [c:\chromium\src\third_party\sqlite\src\mutex_w32.c @ 185]
0579fa34 0278d76c chrome_12d0000!sqlite3_mutex_enter(
			struct sqlite3_mutex * p = 0xdddddddd)+0x16 [c:\chromium\src\third_party\sqlite\src\mutex.c @ 111]
0579fa80 029655f8 chrome_12d0000!sqlite3_exec(
			struct sqlite3 * db = 0x06df6618, 
			char * zSql = 0x043feec0 "ROLLBACK", 
			<function> * xCallback = 0x00000000, 
			void * pArg = 0x00000000, 
			char ** pzErrMsg = 0x00000000)+0x4c [c:\chromium\src\third_party\sqlite\src\legacy.c @ 50]
0579faa4 02965523 chrome_12d0000!SQLTransaction::EndCommand(
			char * command = 0x043feec0 "ROLLBACK")+0x48 [c:\chromium\src\chrome\common\sqlite_utils.cc @ 96]
0579fab8 029654ea chrome_12d0000!SQLTransaction::Rollback(void)+0x23 [c:\chromium\src\chrome\common\sqlite_utils.h @ 57]
0579fac4 02965496 chrome_12d0000!SQLTransaction::~SQLTransaction(void)+0x2a [c:\chromium\src\chrome\common\sqlite_utils.cc @ 82]
0579fad0 01ad0cdf chrome_12d0000!SQLTransaction::`scalar deleting destructor'(void)+0x16
0579faf0 01ac8c49 chrome_12d0000!scoped_ptr<SQLTransaction>::~scoped_ptr<SQLTransaction>(void)+0x3f [c:\chromium\src\base\scoped_ptr.h @ 72]
0579fafc 01ac8a86 chrome_12d0000!SafeBrowsingDatabaseBloom::~SafeBrowsingDatabaseBloom(void)+0x49 [c:\chromium\src\chrome\browser\safe_browsing\safe_browsing_database_bloom.cc @ 61]
0579fb08 017bb3a0 chrome_12d0000!SafeBrowsingDatabaseBloom::`scalar deleting destructor'(void)+0x16
0579fb28 01c66a99 chrome_12d0000!DeleteTask<SafeBrowsingDatabase>::Run(void)+0x40 [c:\chromium\src\base\task.h @ 227]
0579fbd8 01c66b45 chrome_12d0000!MessageLoop::RunTask(
			class Task * task = 0x0800df10)+0xb9 [c:\chromium\src\base\message_loop.cc @ 308]
0579fbe8 01c67029 chrome_12d0000!MessageLoop::DeferOrRunPendingTask(
			struct MessageLoop::PendingTask * pending_task = 0x0579fc04)+0x35 [c:\chromium\src\base\message_loop.cc @ 319]
0579fc24 01cea44c chrome_12d0000!MessageLoop::DoWork(void)+0xe9 [c:\chromium\src\base\message_loop.cc @ 408]
0579fd04 01c663bb chrome_12d0000!base::MessagePumpDefault::Run(
			class base::MessagePump::Delegate * delegate = 0x0579feb4)+0xbc [c:\chromium\src\base\message_pump_default.cc @ 23]
0579fdb0 01c66220 chrome_12d0000!MessageLoop::RunInternal(void)+0xfb [c:\chromium\src\base\message_loop.cc @ 197]
0579fde8 01c660aa chrome_12d0000!MessageLoop::RunHandler(void)+0x90 [c:\chromium\src\base\message_loop.cc @ 181]
0579fe10 01c86d38 chrome_12d0000!MessageLoop::Run(void)+0x3a [c:\chromium\src\base\message_loop.cc @ 155]
0579ffa4 01c86071 chrome_12d0000!base::Thread::ThreadMain(void)+0xb8 [c:\chromium\src\base\thread.cc @ 156]

Review URL: http://codereview.chromium.org/17617

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@7891 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 1 insertions, 4 deletions

diff --git a/chrome/browser/safe_browsing/safe_browsing_database_bloom.cc b/chrome/browser/safe_browsing/safe_browsing_database_bloom.cc
index ef188d7..44636f3 100644
--- a/chrome/browser/safe_browsing/safe_browsing_database_bloom.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_database_bloom.cc
@@ -141,6 +141,7 @@ bool SafeBrowsingDatabaseBloom::Close() {
   if (!db_)
     return true;
 
+  insert_transaction_.reset();
   statement_cache_.reset();  // Must free statements before closing DB.
   bool result = sqlite3_close(db_) == SQLITE_OK;
   db_ = NULL;
@@ -237,8 +238,6 @@ bool SafeBrowsingDatabaseBloom::ResetDatabase() {
   hash_cache_->clear();
   ClearUpdateCaches();
 
-  insert_transaction_.reset();
-
   bool rv = Close();
   DCHECK(rv);
 
@@ -420,7 +419,6 @@ bool SafeBrowsingDatabaseBloom::UpdateStarted() {
   insert_transaction_.reset(new SQLTransaction(db_));
   if (insert_transaction_->Begin() != SQLITE_OK) {
     DCHECK(false) << "Safe browsing database couldn't start transaction";
-    insert_transaction_.reset();
     Close();
     return false;
   }
@@ -431,7 +429,6 @@ void SafeBrowsingDatabaseBloom::UpdateFinished(bool update_succeeded) {
   if (update_succeeded)
     BuildBloomFilter();
 
-  insert_transaction_.reset();
   Close();
 
   // We won't need the chunk caches until the next update (which will read them