Closed Bug 334174 Opened 15 years ago Closed 14 years ago

Corrupted database files are not handled correctly

Categories

(Toolkit :: Storage, defect, P1)

1.8 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla1.8.1beta2

People

(Reporter: asqueella, Assigned: vlad)

Details

(Keywords: fixed1.8.1, Whiteboard: [has patch][needs approval])

Attachments

(3 files, 2 obsolete files)

If you have a corrupted database file for bookmarks or form data, the system is not able to recover from it. It should delete/move the corrupt file and create a new one. Instead current code does this:

1) history code notices the failure code from storage when calling TableExists and bails out, causing the createInstance call to fail.
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/browser/components/places/src/nsNavBookmarks.cpp&mark=258-259#253

2) form history code first does an unitialized memory read:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/toolkit/components/satchel/src/nsStorageFormHistory.cpp&rev=1.9&mark=400-402#400
then follows the steps from (1).

An example of a broken db file can be found at mozilla.doslash.org/stuff/formhistory.sqlite (it also triggers a bug in sqlite, which was filed as ticket 1773, but it's not related to this problem).
Flags: blocking-firefox2?
Assignee: nobody → brettw
Priority: -- → P2
Target Milestone: --- → Firefox 2 beta1
OS: Windows XP → All
Priority: P2 → P1
Hardware: PC → All
Version: Trunk → 2.0 Branch
Priority: P1 → P2
Target Milestone: Firefox 2 beta1 → Firefox 3 alpha2
Version: 2.0 Branch → Trunk
Flags: blocking-firefox2?
Note that this is not places-only. Formhistory still uses storage on 1.8.1 branch and still suffers from this problem.
I'm pretty sure we switched formhistory back to mork on the 1.8branch. supercookies still uses storage of course.
Nope, form history is bug 335558, waiting for a1.8.1 from mtschrep.
I wasn't aware of the plans to turn it storage formhistory off on the branch, sorry for the spam.
Component: Places → Storage
Product: Firefox → Toolkit
Target Milestone: Firefox 3 alpha2 → mozilla1.8.1beta1
Flags: blocking-firefox2+
QA Contact: places → storage
Whiteboard: swag: 2d
If formhistory (see bug 339861 and bug 335558) is going to use Storage again, we really need to fix this bug.
Priority: P2 → P1
Version: Trunk → 1.8 Branch
Whiteboard: swag: 2d → at risk
Target Milestone: mozilla1.8.1beta1 → mozilla1.8.1beta2
Joey/Vlad do we still need to bullet-proof the Search Engine and DOM storage code?
I'm working on a patch for this; I thought it would be faster than it was, but it'll probably not be until b2.  There are a number of problems that conspire against a simple solution:

- sqlite3_open doesn't return an error, esp. when we have async IO, since the open doesn't actually happen until the first db operation
- when a db operation occurs, and it returns SQLITE3_CORRUPT or some other such error code, I have code in there now that translates this to NS_ERROR_FILE_IS_CORRUPT or whatever is appropriate.  however...
- by the time we get this error (e.g. from TableExists()), we have the db file already opened.  On win32 at least, this means that we can't delete it until we close all open file handles.  We can't just close the db connection, since that's an async operation; we need to flush the entire async operation list to be able to close it.

So, this will be a bit of work; I'm thinking about changing mozStorage to perform a dummy operation right after open, so that it can immediately catch IO errors/corrupt errors/etc, and so that Open() can return those to the caller.
Vlad, did you see the latest version of test_async.c in the sqlite repository? I think it handles this stuff. We've diverged a lot and you would want to use Mozilla data structures instead of his custom ones, but you might be able to see how it's done.
This does a bit more error checking at db open time, and then form history/DOM storage deletes the db file if there's a failure.  Not sure if we can do any better here.
Assignee: brettw → vladimir
Status: NEW → ASSIGNED
Attachment #229567 - Flags: first-review?
Attachment #229567 - Flags: first-review? → first-review?(brettw)
Comment on attachment 229567 [details] [diff] [review]
delete corrupt databases if open fails

>+DO_OPENDB:
>   rv = mStorageService->OpenDatabase(formHistoryFile, getter_AddRefs(mDBConn));
>+  if (rv == NS_ERROR_FILE_CORRUPTED && !failedOnce) {
>+    // delete the db and try opening again
>+    mDBConn = nsnull; // must force-close the db
>+    failedOnce = PR_TRUE;
>+    rv = formHistoryFile->Remove(PR_FALSE);
>+    NS_ENSURE_SUCCESS(rv, rv);
>+    goto DO_OPENDB;
>+  }

and

>   rv = service->OpenDatabase(storageFile, getter_AddRefs(mConnection));
>+  if (rv == NS_ERROR_FILE_CORRUPTED) {
>+    // delete the db and try opening again
>+    rv = storageFile->Remove(PR_FALSE);
>+    NS_ENSURE_SUCCESS(rv, rv);
>+    rv = service->OpenDatabase(storageFile, getter_AddRefs(mConnection));
>+  }
>   NS_ENSURE_SUCCESS(rv, rv);

look like they do the same thing, but the bottom one looks much better. Can you change the top one to be like it to get rid of the goto?

Also, should these operations be NS_ENSURE_SUCCESS (both for Remove() , and OpenDatabase())? I thought that this was to be used more like an assertion, and you should just if() it if it is something that can always fail, like a file access. I guess this doesn't matter much, though.
Attachment #229567 - Flags: first-review?(brettw) → first-review+
Whoops, yeah, I removed the goto bit already in the top chunk.  I need to test one more thing and then I'll ask for a1.8.1+
Whiteboard: at risk → [has patch][needs testing and approval]
Attached patch new trunk patch (obsolete) — Splinter Review
New patch.  The original patch had problems due to the async IO -- close is also async, so the remove would often fail if the async thread hadn't caught up to the close yet.

This patch adds a way to flush the async queue, and we call that method every time we close a database -- this isn't that often (normally only on shutdown, or in the case of a corrupt db), so this shouldn't be expensive.

I also originally missed a few places where a corrupt-db condition needed to be handled; I've caught (I think) most of those here.

Note that for the branch, the only two storage-using pieces are the search service and the anti-phishing service.
Attachment #229567 - Attachment is obsolete: true
Attachment #230653 - Flags: first-review?(brettw)
Comment on attachment 230653 [details] [diff] [review]
new trunk patch

The problem is that you've created the possibility of starvation by requiring that the queue be empty before signaling that the flush is complete. This means that another connection on a different thread could be writing into the queue constantly and prevent your flush from completing.

What you really want is to wait until everything in the queue at the point the flush is called is flushed. Darin and I came up with this plan:

Create a new event type to go along with ASYNC_WRITE, etc. called ASYNC_FLUSH. In your flush function, create a new condition variable, put it into this event, and post it. This should not require acquiring the lock since AppendNewAsyncMessage will do all the necessary locking for you.

Then just wait on your created condition (without holding the queue lock). ProcessOneMessage signals the condition when it gets that message. Your thread then deletes the condition and continues.

I think that this will be easier to implement and read than your current implementation.
Attachment #230653 - Flags: first-review?(brettw) → first-review-
Attached patch updated patchSplinter Review
Ok, good idea; updated patch with that implementation.  The only thing that's changed is the stuff in AsyncIO (the first diff in the file).
Attachment #230653 - Attachment is obsolete: true
Attachment #230686 - Flags: first-review?(brettw)
Comment on attachment 230686 [details] [diff] [review]
updated patch

Why don't you use AppendNewAsyncMessage instead? It handles all the memory allocation stuff for you. Then you can get rid of AsyncBarrier and in FlushAsyncIO. Also, you should handle out of memory or we're going to have a deadlock because the lock will never get signaled.

AsyncMessageBarrierData data;
data.mLock = PR_NewLock()
data.mCondVar = PR_NewCondVar()
rv = AppendNewAsyncMessage(nsnull, ASYNC_BARRIER, 0, sizeof(data),
  (char*)&data);
if (rv == SQLITE_OK) {
  lock
  wait
  unlock
}
destroy locks
return rv;



>Index: storage/src/mozStorageAsyncIO.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/storage/src/mozStorageAsyncIO.cpp,v
>retrieving revision 1.12
>diff -u -8 -p -r1.12 mozStorageAsyncIO.cpp
>--- storage/src/mozStorageAsyncIO.cpp	19 May 2006 16:25:40 -0000	1.12
>+++ storage/src/mozStorageAsyncIO.cpp	26 Jul 2006 02:55:13 -0000
>@@ -343,26 +343,33 @@ struct AsyncMessage
>   // a single buffer with mBuf pointing to the memory immediately following
>   // this structure. Freeing this structure will also free mBuf.
>   char *mBuf;
> 
>   // Next write operation (to any file) in the linked list
>   AsyncMessage* mNext;
> };
> 
>+struct AsyncMessageBarrierData
>+{
>+  PRLock *mLock;
>+  PRCondVar *mCondVar;
>+};
>+
> // Possible values of AsyncMessage.mOp
> #define ASYNC_WRITE         1
> #define ASYNC_SYNC          2
> #define ASYNC_TRUNCATE      3
> #define ASYNC_CLOSE         4
> #define ASYNC_OPENDIRECTORY 5
> #define ASYNC_SETFULLSYNC   6
> #define ASYNC_DELETE        7
> #define ASYNC_OPENEXCLUSIVE 8
> #define ASYNC_SYNCDIRECTORY 9
>+#define ASYNC_BARRIER       10
> 
> // replacements for the sqlite OS routines
> static int AsyncOpenReadWrite(const char *aName, OsFile **aFile, int *aReadOnly);
> static int AsyncOpenExclusive(const char *aName, OsFile **aFile, int aDelFlag);
> static int AsyncOpenReadOnly(const char *aName, OsFile **aFile);
> static int AsyncDelete(const char* aName);
> static int AsyncSyncDirectory(const char* aName);
> static int AsyncFileExists(const char *aName);
>@@ -376,16 +383,18 @@ static int AsyncRead(OsFile* aFile, void
> static int AsyncSeek(OsFile* aFile, sqlite_int64 aOffset);
> static int AsyncFileSize(OsFile* aFile, sqlite_int64* aSize);
> static int AsyncFileHandle(OsFile* aFile);
> static int AsyncLock(OsFile* aFile, int aLockType);
> static int AsyncUnlock(OsFile* aFile, int aLockType);
> static int AsyncCheckReservedLock(OsFile* aFile);
> static int AsyncLockState(OsFile* aFile);
> 
>+static void AsyncBarrier(PRLock* aLock, PRCondVar* aCondVar);
>+
> // backend for all the open functions
> static int AsyncOpenFile(const char *aName, AsyncOsFile **aFile,
>                      OsFile *aBaseRead, PRBool aOpenForWriting);
> 
> // message queue
> static AsyncMessage* AsyncQueueFirst = nsnull;
> static AsyncMessage* AsyncQueueLast = nsnull;
> #ifdef SINGLE_THREADED
>@@ -516,16 +525,50 @@ mozStorageService::InitStorageAsyncIO()
>     AsyncWriteThreadInstance = nsnull;
>     return rv;
>   }
> #endif
> 
>   return NS_OK;
> }
> 
>+// mozstorageService::FlushAsyncIO
>+//
>+//    This function will grab the async lock and process all
>+//    remaining async operations that are in the queue on the current
>+//    thread.  Call this when you need to make sure that an operation
>+//    has taken place, e.g. that a file has been closed.
>+
>+nsresult
>+mozStorageService::FlushAsyncIO()
>+{
>+  AsyncMessage *message = 0;
>+  int rc = SQLITE_OK;
>+
>+  // single threaded? nothing to do.
>+  if (!AsyncWriteThreadInstance)
>+    return NS_OK;
>+
>+  PRLock *flushLock = PR_NewLock();
>+  PRCondVar *flushCond = PR_NewCondVar(flushLock);
>+
>+  PR_Lock(flushLock);
>+  AsyncBarrier(flushLock, flushCond);
>+
>+  // the async thread will notify us once it reaches
>+  // the ASYNC_BARRIER operation
>+  PR_WaitCondVar(flushCond, PR_INTERVAL_NO_TIMEOUT);
>+  PR_Unlock(flushLock);
>+
>+  PR_DestroyCondVar(flushCond);
>+  PR_DestroyLock(flushLock);
>+
>+  return NS_OK;
>+}
>+
> 
> // mozStorageService::FinishAsyncIO
> //
> //    Call this function on shutdown to ensure that all buffered writes have
> //    been comitted to disk. This then puts us into sychronous write mode. Any
> //    subsequent database operations will be blocking. This way, we don't care
> //    about the shutdown order of components. Other components can still
> //    continue to use the database as we shut down, it just won't be buffered.
>@@ -1253,16 +1295,46 @@ int // static
> AsyncLockState(OsFile* aFile)
> {
>   if (AsyncWriteError != SQLITE_OK)
>     return AsyncWriteError;
>   NS_NOTREACHED("Don't call LockState in async mode");
>   return SQLITE_OK;
> }
> 
>+// AsyncBarrier
>+//
>+//    This is used to notify a waiting thread that this point in the async
>+//    queue has been reached.  Note that this is not a sqlite redirected IO
>+//    function
>+
>+void // static
>+AsyncBarrier(PRLock* aLock, PRCondVar* aCondVar)
>+{
>+  AsyncMessage* p = NS_STATIC_CAST(AsyncMessage*,
>+      nsMemory::Alloc(sizeof(AsyncMessage) + sizeof(AsyncMessageBarrierData)));
>+  if (!p) {
>+    NS_NOTREACHED("ALLOC ERROR");
>+    return;
>+  }
>+
>+  AsyncMessageBarrierData* bd = (AsyncMessageBarrierData*) (((char*)p) + sizeof(AsyncMessage));
>+
>+  bd->mLock = aLock;
>+  bd->mCondVar = aCondVar;
>+
>+  p->mOp = ASYNC_BARRIER;
>+  p->mOffset = 0;
>+  p->mBytes = 0;
>+  p->mFile = nsnull;
>+  p->mNext = nsnull;
>+  p->mBuf = (char*) bd;
>+
>+  AppendAsyncMessage(p);
>+}
> 
> // ProcessOneMessage
> //
> //    When called, this thread is holding the mutex on the write-op queue.  In
> //    the general case, we hold on to the mutex for the entire processing of
> //    the message.
> //
> //    However in the potentially slower cases enumerated below, we relinquish
>@@ -1360,16 +1432,24 @@ ProcessOneMessage(AsyncMessage* aMessage
>       // doesn't try to use it to do synchronous reading.
>       PR_Lock(AsyncQueueLock);
>       regainMutex = PR_FALSE;
>       if (rc == SQLITE_OK)
>         pFile->mBaseRead = pBase;
>       break;
>     }
> 
>+    case ASYNC_BARRIER: {
>+      AsyncMessageBarrierData *bd = (AsyncMessageBarrierData*) aMessage->mBuf;
>+      PR_Lock(bd->mLock);
>+      PR_NotifyCondVar(bd->mCondVar);
>+      PR_Unlock(bd->mLock);
>+      break;
>+    }
>+
>     default:
>       NS_NOTREACHED("Illegal value for AsyncMessage.mOp");
>   }
> 
>   if (regainMutex) {
>     PR_Lock(AsyncQueueLock);
>   }
>   return rc;
>Index: storage/src/mozStorageConnection.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/storage/src/mozStorageConnection.cpp,v
>retrieving revision 1.15
>diff -u -8 -p -r1.15 mozStorageConnection.cpp
>--- storage/src/mozStorageConnection.cpp	12 Apr 2006 15:39:42 -0000	1.15
>+++ storage/src/mozStorageConnection.cpp	25 Jul 2006 22:48:38 -0000
>@@ -68,19 +68,43 @@ mozStorageConnection::mozStorageConnecti
> 
> mozStorageConnection::~mozStorageConnection()
> {
>     if (mDBConn) {
>         int srv = sqlite3_close (mDBConn);
>         if (srv != SQLITE_OK) {
>             NS_WARNING("sqlite3_close failed.  There are probably outstanding statements!");
>         }
>+
>+        // make sure it really got closed
>+        ((mozStorageService*)(mStorageService.get()))->FlushAsyncIO();
>     }
> }
> 
>+// convert a sqlite srv to an nsresult
>+static nsresult
>+ConvertResultCode (int srv)
>+{
>+    switch (srv) {
>+        case SQLITE_OK:
>+            return NS_OK;
>+        case SQLITE_CORRUPT:
>+        case SQLITE_NOTADB:
>+            return NS_ERROR_FILE_CORRUPTED;
>+        case SQLITE_PERM:
>+        case SQLITE_CANTOPEN:
>+            return NS_ERROR_FILE_ACCESS_DENIED;
>+        case SQLITE_BUSY:
>+            return NS_ERROR_FILE_IS_LOCKED;
>+    }
>+
>+    // generic error
>+    return NS_ERROR_FAILURE;
>+}
>+
> #ifdef PR_LOGGING
> void tracefunc (void *closure, const char *stmt)
> {
>     PR_LOG(gStorageLog, PR_LOG_DEBUG, ("%s", stmt));
> }
> #endif
> 
> /**
>@@ -104,26 +128,55 @@ mozStorageConnection::Initialize(nsIFile
> 
>         srv = sqlite3_open (NS_ConvertUTF16toUTF8(path).get(), &mDBConn);
>     } else {
>         // in memory database requested, sqlite uses a magic file name
>         srv = sqlite3_open (":memory:", &mDBConn);
>     }
>     if (srv != SQLITE_OK) {
>         mDBConn = nsnull;
>-        return NS_ERROR_FAILURE; // XXX error code
>+        return ConvertResultCode(srv);
>     }
> 
> #ifdef PR_LOGGING
>     if (! gStorageLog)
>         gStorageLog = PR_NewLogModule("mozStorage");
> 
>     sqlite3_trace (mDBConn, tracefunc, nsnull);
> #endif
> 
>+    /* Execute a dummy statement to force the db open, and to verify
>+     * whether it's valid or not
>+     */
>+    sqlite3_stmt *stmt = nsnull;
>+    nsCString query("SELECT * FROM sqlite_master");
>+    srv = sqlite3_prepare (mDBConn, query.get(), query.Length(), &stmt, nsnull);
>+ 
>+    if (srv == SQLITE_OK) {
>+        srv = sqlite3_step(stmt);
>+ 
>+        if (srv == SQLITE_DONE || srv == SQLITE_ROW)
>+            srv = SQLITE_OK;
>+    } else {
>+        stmt = nsnull;
>+    }
>+
>+    if (stmt != nsnull)
>+        sqlite3_finalize (stmt);
>+ 
>+    if (srv != SQLITE_OK) {
>+        sqlite3_close (mDBConn);
>+        mDBConn = nsnull;
>+
>+        // make sure it really got closed
>+        ((mozStorageService*)(mStorageService.get()))->FlushAsyncIO();
>+
>+        return ConvertResultCode(srv);
>+    }
>+
>     mFunctions = do_CreateInstance(NS_ARRAY_CONTRACTID, &rv);
>     if (NS_FAILED(rv)) return rv;
> 
>     return NS_OK;
> }
> 
> /*****************************************************************************
>  ** mozIStorageConnection interface
>@@ -194,33 +247,33 @@ mozStorageConnection::CreateStatement(co
>     NS_ASSERTION(mDBConn, "connection not initialized");
> 
>     mozStorageStatement *statement = new mozStorageStatement();
>     NS_ADDREF(statement);
> 
>     nsresult rv = statement->Initialize (this, aSQLStatement);
>     if (NS_FAILED(rv)) {
>         NS_RELEASE(statement);
>-        return NS_ERROR_FAILURE; // XXX error code
>+        return rv;
>     }
> 
>     *_retval = statement;
>     return NS_OK;
> }
> 
> NS_IMETHODIMP
> mozStorageConnection::ExecuteSimpleSQL(const nsACString& aSQLStatement)
> {
>     NS_ENSURE_ARG_POINTER(mDBConn);
> 
>     int srv = sqlite3_exec (mDBConn, PromiseFlatCString(aSQLStatement).get(),
>                             NULL, NULL, NULL);
>     if (srv != SQLITE_OK) {
>         HandleSqliteError(nsPromiseFlatCString(aSQLStatement).get());
>-        return NS_ERROR_FAILURE; // XXX error code
>+        return ConvertResultCode(srv);
>     }
> 
>     return NS_OK;
> }
> 
> NS_IMETHODIMP
> mozStorageConnection::TableExists(const nsACString& aSQLStatement, PRBool *_retval)
> {
>@@ -229,17 +282,17 @@ mozStorageConnection::TableExists(const 
>     nsCString query("SELECT name FROM sqlite_master WHERE type = 'table' AND name ='");
>     query.Append(aSQLStatement);
>     query.AppendLiteral("'");
> 
>     sqlite3_stmt *stmt = nsnull;
>     int srv = sqlite3_prepare (mDBConn, query.get(), query.Length(), &stmt, nsnull);
>     if (srv != SQLITE_OK) {
>         HandleSqliteError(query.get());
>-        return NS_ERROR_FAILURE; // XXX error code
>+        return ConvertResultCode(srv);
>     }
> 
>     PRBool exists = PR_FALSE;
> 
>     srv = sqlite3_step(stmt);
>     // we just care about the return value from step
>     sqlite3_finalize(stmt);
> 
>@@ -264,17 +317,17 @@ mozStorageConnection::IndexExists(const 
>     nsCString query("SELECT name FROM sqlite_master WHERE type = 'index' AND name ='");
>     query.Append(aIndexName);
>     query.AppendLiteral("'");
> 
>     sqlite3_stmt *stmt = nsnull;
>     int srv = sqlite3_prepare(mDBConn, query.get(), query.Length(), &stmt, nsnull);
>     if (srv != SQLITE_OK) {
>         HandleSqliteError(query.get());
>-        return NS_ERROR_FAILURE; // XXX error code
>+        return ConvertResultCode(srv);
>     }
> 
>     PRBool exists = PR_FALSE;
> 
>     srv = sqlite3_step(stmt);
>     // we just care about the return value from step
>     sqlite3_finalize(stmt);
> 
>@@ -376,20 +429,17 @@ mozStorageConnection::CreateTable(/*cons
>     if (!buf)
>         return NS_ERROR_OUT_OF_MEMORY;
> 
>     srv = sqlite3_exec (mDBConn, buf,
>                         NULL, NULL, NULL);
> 
>     PR_smprintf_free(buf);
> 
>-    if (srv != SQLITE_OK) {
>-        return NS_ERROR_FAILURE; // XXX SQL_ERROR_TABLE_EXISTS
>-    }
>-    return NS_OK;
>+    return ConvertResultCode(srv);
> }
> 
> /**
>  ** Functions
>  **/
> 
> static void
> mozStorageSqlFuncHelper (sqlite3_context *ctx,
>@@ -428,36 +478,34 @@ mozStorageConnection::CreateFunction(con
>                                        aNumArguments,
>                                        SQLITE_ANY,
>                                        aFunction,
>                                        mozStorageSqlFuncHelper,
>                                        nsnull,
>                                        nsnull);
>     if (srv != SQLITE_OK) {
>         HandleSqliteError(nsnull);
>-        return NS_ERROR_FAILURE;
>+        return ConvertResultCode(srv);
>     }
> 
>     rv = mFunctions->AppendElement (aFunction, PR_FALSE);
>     if (NS_FAILED(rv)) return rv;
> 
>     return NS_OK;
> }
> 
> /**
>  * Mozilla-specific sqlite function to preload the DB into the cache. See the
>  * IDL and sqlite3.h
>  */
> nsresult
> mozStorageConnection::Preload()
> {
>   int srv = sqlite3Preload(mDBConn);
>-  if (srv != SQLITE_OK)
>-    return NS_ERROR_FAILURE;
>-  return NS_OK;
>+  return ConvertResultCode(srv);
> }
> 
> /**
>  ** Other bits
>  **/
> void
> mozStorageConnection::HandleSqliteError(const char *aSqlStatement)
> {
>Index: storage/src/mozStorageService.h
>===================================================================
>RCS file: /cvsroot/mozilla/storage/src/mozStorageService.h,v
>retrieving revision 1.3
>diff -u -8 -p -r1.3 mozStorageService.h
>--- storage/src/mozStorageService.h	17 Feb 2006 00:36:24 -0000	1.3
>+++ storage/src/mozStorageService.h	25 Jul 2006 22:48:38 -0000
>@@ -43,19 +43,23 @@
> 
> #include "nsCOMPtr.h"
> #include "nsIFile.h"
> #include "nsIObserver.h"
> #include "nsIObserverService.h"
> 
> #include "mozIStorageService.h"
> 
>+class mozStorageConnection;
>+
> class mozStorageService : public mozIStorageService,
>                           public nsIObserver
> {
>+    friend class mozStorageConnection;
>+
> public:
>     mozStorageService();
> 
>     // two-phase init, must call before using service
>     nsresult Init();
> 
>     // nsISupports
>     NS_DECL_ISUPPORTS
>@@ -66,13 +70,14 @@ public:
>     NS_DECL_NSIOBSERVER
> 
> private:
>     ~mozStorageService();
> protected:
>     nsCOMPtr<nsIFile> mProfileStorageFile;
> 
>     nsresult InitStorageAsyncIO();
>+    nsresult FlushAsyncIO();
>     nsresult FinishAsyncIO();
>     void FreeLocks();
> };
> 
> #endif /* _MOZSTORAGESERVICE_H_ */
>Index: browser/components/places/src/nsNavHistory.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/browser/components/places/src/Attic/nsNavHistory.cpp,v
>retrieving revision 1.101
>diff -u -8 -p -r1.101 nsNavHistory.cpp
>--- browser/components/places/src/nsNavHistory.cpp	3 Jun 2006 23:35:53 -0000	1.101
>+++ browser/components/places/src/nsNavHistory.cpp	25 Jul 2006 22:46:41 -0000
>@@ -411,16 +411,22 @@ nsNavHistory::InitDB(PRBool *aDoImport)
>                               getter_AddRefs(dbFile));
>   NS_ENSURE_SUCCESS(rv, rv);
>   rv = dbFile->Append(NS_LITERAL_STRING("bookmarks_history.sqlite"));
>   NS_ENSURE_SUCCESS(rv, rv);
> 
>   mDBService = do_GetService(MOZ_STORAGE_SERVICE_CONTRACTID, &rv);
>   NS_ENSURE_SUCCESS(rv, rv);
>   rv = mDBService->OpenDatabase(dbFile, getter_AddRefs(mDBConn));
>+  if (rv == NS_ERROR_FILE_CORRUPTED) {
>+    // delete the db and try opening again
>+    rv = dbFile->Remove(PR_FALSE);
>+    NS_ENSURE_SUCCESS(rv, rv);
>+    rv = mDBService->OpenDatabase(dbFile, getter_AddRefs(mDBConn));
>+  }
>   NS_ENSURE_SUCCESS(rv, rv);
> 
>   // Set the database page size. This will only have any effect on empty files,
>   // so must be done before anything else. If the file already exists, we'll
>   // get that file's page size and this will have no effect.
>   nsCAutoString pageSizePragma("PRAGMA page_size=");
>   pageSizePragma.AppendInt(DEFAULT_DB_PAGE_SIZE);
>   rv = mDBConn->ExecuteSimpleSQL(pageSizePragma);
>Index: browser/components/search/nsSearchService.js
>===================================================================
>RCS file: /cvsroot/mozilla/browser/components/search/nsSearchService.js,v
>retrieving revision 1.52
>diff -u -8 -p -r1.52 nsSearchService.js
>--- browser/components/search/nsSearchService.js	14 Jul 2006 16:31:27 -0000	1.52
>+++ browser/components/search/nsSearchService.js	25 Jul 2006 22:46:48 -0000
>@@ -2559,17 +2559,27 @@ var engineMetadataService = {
>   newTable: false,
> 
>   init: function epsInit() {
>     var engineDataTable = "id INTEGER PRIMARY KEY, engineid STRING, name STRING, value STRING";
>     var file = getDir(NS_APP_USER_PROFILE_50_DIR);
>     file.append("search.sqlite");
>     var dbService = Cc["@mozilla.org/storage/service;1"].
>                     getService(Ci.mozIStorageService);
>-    this.mDB = dbService.openDatabase(file);
>+    try {
>+        this.mDB = dbService.openDatabase(file);
>+    } catch (ex) {
>+        if (ex.result == 0x8052000b) { /* NS_ERROR_FILE_CORRUPTED */
>+            // delete and try again
>+            file.remove(false);
>+            this.mDB = dbService.openDatabase(file);
>+        } else {
>+            throw ex;
>+        }
>+    }
> 
>     try {
>       this.mDB.createTable("engine_data", engineDataTable);
>       this.newTable = true;
>     } catch (ex) {
>       this.newTable = false;
>       // Fails if the table already exists, which is fine
>     }
>Index: dom/src/storage/nsDOMStorageDB.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/dom/src/storage/nsDOMStorageDB.cpp,v
>retrieving revision 1.3
>diff -u -8 -p -r1.3 nsDOMStorageDB.cpp
>--- dom/src/storage/nsDOMStorageDB.cpp	19 May 2006 15:33:12 -0000	1.3
>+++ dom/src/storage/nsDOMStorageDB.cpp	25 Jul 2006 22:47:16 -0000
>@@ -59,16 +59,22 @@ nsDOMStorageDB::Init()
>   rv = storageFile->Append(NS_LITERAL_STRING("webappsstore.sqlite"));
>   NS_ENSURE_SUCCESS(rv, rv);
> 
>   nsCOMPtr<mozIStorageService> service;
> 
>   service = do_GetService(MOZ_STORAGE_SERVICE_CONTRACTID, &rv);
>   NS_ENSURE_SUCCESS(rv, rv);
>   rv = service->OpenDatabase(storageFile, getter_AddRefs(mConnection));
>+  if (rv == NS_ERROR_FILE_CORRUPTED) {
>+    // delete the db and try opening again
>+    rv = storageFile->Remove(PR_FALSE);
>+    NS_ENSURE_SUCCESS(rv, rv);
>+    rv = service->OpenDatabase(storageFile, getter_AddRefs(mConnection));
>+  }
>   NS_ENSURE_SUCCESS(rv, rv);
> 
>   PRBool exists;
>   rv = mConnection->TableExists(NS_LITERAL_CSTRING("moz_webappsstore"), &exists);
>   NS_ENSURE_SUCCESS(rv, rv);
>   if (! exists) {
>     rv = mConnection->ExecuteSimpleSQL(
>            NS_LITERAL_CSTRING("CREATE TABLE moz_webappsstore ("
>Index: toolkit/components/satchel/src/nsStorageFormHistory.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/toolkit/components/satchel/src/nsStorageFormHistory.cpp,v
>retrieving revision 1.10
>diff -u -8 -p -r1.10 nsStorageFormHistory.cpp
>--- toolkit/components/satchel/src/nsStorageFormHistory.cpp	18 May 2006 23:14:10 -0000	1.10
>+++ toolkit/components/satchel/src/nsStorageFormHistory.cpp	25 Jul 2006 22:48:46 -0000
>@@ -383,16 +383,22 @@ nsFormHistory::OpenDatabase()
>   nsresult rv;
>   mStorageService = do_GetService(MOZ_STORAGE_SERVICE_CONTRACTID, &rv);
>   NS_ENSURE_SUCCESS(rv, rv);
> 
>   nsCOMPtr<nsIFile> formHistoryFile;
>   rv = GetDatabaseFile(getter_AddRefs(formHistoryFile));
>   NS_ENSURE_SUCCESS(rv, rv);
>   rv = mStorageService->OpenDatabase(formHistoryFile, getter_AddRefs(mDBConn));
>+  if (rv == NS_ERROR_FILE_CORRUPTED) {
>+    // delete the db and try opening again
>+    rv = formHistoryFile->Remove(PR_FALSE);
>+    NS_ENSURE_SUCCESS(rv, rv);
>+    rv = mStorageService->OpenDatabase(formHistoryFile, getter_AddRefs(mDBConn));
>+  }
>   NS_ENSURE_SUCCESS(rv, rv);
> 
>   // We execute many statements before the database cache is started to create
>   // the tables (which can not be done which the cache is locked in memory by
>   // the dummy statement--see StartCache). This transaction will keep the cache
>   // between these statements, which should improve startup performance because
>   // we won't have to keep requesting pages from the OS.
>   mozStorageTransaction transaction(mDBConn, PR_FALSE);
>Index: toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp,v
>retrieving revision 1.14
>diff -u -8 -p -r1.14 nsUrlClassifierDBService.cpp
>--- toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp	15 Jun 2006 19:10:04 -0000	1.14
>+++ toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp	25 Jul 2006 22:48:46 -0000
>@@ -521,17 +521,24 @@ nsUrlClassifierDBServiceWorker::OpenDb()
>   NS_ENSURE_SUCCESS(rv, rv);
>   rv = dbFile->Append(NS_LITERAL_STRING(DATABASE_FILENAME));
>   NS_ENSURE_SUCCESS(rv, rv);
> 
>   // open the connection
>   nsCOMPtr<mozIStorageService> storageService =
>     do_GetService(MOZ_STORAGE_SERVICE_CONTRACTID, &rv);
>   NS_ENSURE_SUCCESS(rv, rv);
>-  return storageService->OpenDatabase(dbFile, &mConnection);
>+  rv = storageService->OpenDatabase(dbFile, &mConnection);
>+  if (rv == NS_ERROR_FILE_CORRUPTED) {
>+    // delete the db and try opening again
>+    rv = dbFile->Remove(PR_FALSE);
>+    NS_ENSURE_SUCCESS(rv, rv);
>+    rv = storageService->OpenDatabase(dbFile, &mConnection);
>+  }
>+  return rv;
> }
> 
> nsresult
> nsUrlClassifierDBServiceWorker::MaybeCreateTable(const nsCString& aTableName)
> {
>   LOG(("MaybeCreateTable %s\n", aTableName.get()));
> 
>   nsCOMPtr<mozIStorageStatement> createStatement;
Attachment #230686 - Flags: first-review?(brettw) → first-review-
Fixed out-of-memory issues and uses AppendNewAsyncMessage.  I kept AsyncBarrier() in as a function because it could be used for other things as well (arbitrary notification of when the async thread has reached a certain point).
Attachment #230763 - Flags: first-review?(brettw)
Comment on attachment 230763 [details] [diff] [review]
just the asyncio bit

Looks good.
Attachment #230763 - Flags: first-review?(brettw) → first-review+
Checked in on trunk.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: [has patch][needs testing and approval] → [has patch][needs approval]
Comment on attachment 230763 [details] [diff] [review]
just the asyncio bit

Requesting approval1.8.1 on this; tomorrow's trunk builds will have this, and it's straightforward to verify this (just echo "abc" to the *.sqlite files in the profile dir).  Needs this patch + parts of the previous patch, since this is just the asyncio bits.
Attachment #230763 - Flags: approval1.8.1?
Attachment #230763 - Flags: approval1.8.1? → approval1.8.1+
Attaching patch that landed on branch for posterity (some of the actual db-users moved around on trunk)
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.