Skip to content
This repository was archived by the owner on Jul 9, 2025. It is now read-only.

Commit ae4301e

Browse files
committed
Bug 1285041 - ignore locking when trying to read chrome DB file, r=mak
MozReview-Commit-ID: 89f0YCxxgC8 --HG-- extra : rebase_source : 53efa0f0258728df54fc17a773cbf74c712b51e4
1 parent 5be6837 commit ae4301e

6 files changed

Lines changed: 156 additions & 35 deletions

File tree

browser/components/migration/ChromeProfileMigrator.js

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -311,19 +311,59 @@ function GetHistoryResource(aProfileFolder) {
311311
if (!historyFile.exists())
312312
return null;
313313

314+
function getRows(dbOptions) {
315+
const RETRYLIMIT = 10;
316+
const RETRYINTERVAL = 100;
317+
return Task.spawn(function* innerGetRows() {
318+
let rows = null;
319+
for (let retryCount = RETRYLIMIT; retryCount && !rows; retryCount--) {
320+
// Attempt to get the rows. If this succeeds, we will bail out of the loop,
321+
// close the database in a failsafe way, and pass the rows back.
322+
// If fetching the rows throws, we will wait RETRYINTERVAL ms
323+
// and try again. This will repeat a maximum of RETRYLIMIT times.
324+
let db;
325+
let didOpen = false;
326+
let exceptionSeen;
327+
try {
328+
db = yield Sqlite.openConnection(dbOptions);
329+
didOpen = true;
330+
rows = yield db.execute(`SELECT url, title, last_visit_time, typed_count
331+
FROM urls WHERE hidden = 0`);
332+
} catch (ex) {
333+
if (!exceptionSeen) {
334+
Cu.reportError(ex);
335+
}
336+
exceptionSeen = ex;
337+
} finally {
338+
try {
339+
if (didOpen) {
340+
yield db.close();
341+
}
342+
} catch (ex) {}
343+
}
344+
if (exceptionSeen) {
345+
yield new Promise(resolve => setTimeout(resolve, RETRYINTERVAL));
346+
}
347+
}
348+
if (!rows) {
349+
throw new Error("Couldn't get rows from the Chrome history database.");
350+
}
351+
return rows;
352+
});
353+
}
354+
314355
return {
315356
type: MigrationUtils.resourceTypes.HISTORY,
316357

317358
migrate(aCallback) {
318359
Task.spawn(function* () {
319-
let db = yield Sqlite.openConnection({
360+
let dbOptions = {
361+
readOnly: true,
362+
ignoreLockingMode: true,
320363
path: historyFile.path
321-
});
322-
323-
let rows = yield db.execute(`SELECT url, title, last_visit_time, typed_count
324-
FROM urls WHERE hidden = 0`);
325-
yield db.close();
364+
};
326365

366+
let rows = yield getRows(dbOptions);
327367
let places = [];
328368
for (let row of rows) {
329369
try {

storage/mozStorageConnection.cpp

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -472,7 +472,8 @@ class AsyncInitializeClone final: public Runnable
472472

473473
Connection::Connection(Service *aService,
474474
int aFlags,
475-
bool aAsyncOnly)
475+
bool aAsyncOnly,
476+
bool aIgnoreLockingMode)
476477
: sharedAsyncExecutionMutex("Connection::sharedAsyncExecutionMutex")
477478
, sharedDBMutex("Connection::sharedDBMutex")
478479
, threadOpenedOn(do_GetCurrentThread())
@@ -485,9 +486,12 @@ Connection::Connection(Service *aService,
485486
, mTransactionInProgress(false)
486487
, mProgressHandler(nullptr)
487488
, mFlags(aFlags)
489+
, mIgnoreLockingMode(aIgnoreLockingMode)
488490
, mStorageService(aService)
489491
, mAsyncOnly(aAsyncOnly)
490492
{
493+
MOZ_ASSERT(!mIgnoreLockingMode || mFlags & SQLITE_OPEN_READONLY,
494+
"Can't ignore locking for a non-readonly connection!");
491495
mStorageService->registerConnection(this);
492496
}
493497

@@ -577,6 +581,7 @@ nsresult
577581
Connection::initialize()
578582
{
579583
NS_ASSERTION (!mDBConn, "Initialize called on already opened database!");
584+
MOZ_ASSERT(!mIgnoreLockingMode, "Can't ignore locking on an in-memory db.");
580585
PROFILER_LABEL("mozStorageConnection", "initialize",
581586
js::ProfileEntry::Category::STORAGE);
582587

@@ -610,8 +615,15 @@ Connection::initialize(nsIFile *aDatabaseFile)
610615
nsresult rv = aDatabaseFile->GetPath(path);
611616
NS_ENSURE_SUCCESS(rv, rv);
612617

618+
#ifdef XP_WIN
619+
static const char* sIgnoreLockingVFS = "win32-none";
620+
#else
621+
static const char* sIgnoreLockingVFS = "unix-none";
622+
#endif
623+
const char* vfs = mIgnoreLockingMode ? sIgnoreLockingVFS : nullptr;
624+
613625
int srv = ::sqlite3_open_v2(NS_ConvertUTF16toUTF8(path).get(), &mDBConn,
614-
mFlags, nullptr);
626+
mFlags, vfs);
615627
if (srv != SQLITE_OK) {
616628
mDBConn = nullptr;
617629
return convertResultCode(srv);

storage/mozStorageConnection.h

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,16 @@ class Connection final : public mozIStorageConnection
6969
* - |mozIStorageAsyncConnection|;
7070
* If |false|, the result also implements synchronous interface:
7171
* - |mozIStorageConnection|.
72+
* @param aIgnoreLockingMode
73+
* If |true|, ignore locks in force on the file. Only usable with
74+
* read-only connections. Defaults to false.
75+
* Use with extreme caution. If sqlite ignores locks, reads may fail
76+
* indicating database corruption (the database won't actually be
77+
* corrupt) or produce wrong results without any indication that has
78+
* happened.
7279
*/
73-
Connection(Service *aService, int aFlags, bool aAsyncOnly);
80+
Connection(Service *aService, int aFlags, bool aAsyncOnly,
81+
bool aIgnoreLockingMode = false);
7482

7583
/**
7684
* Creates the connection to an in-memory database.
@@ -356,6 +364,11 @@ class Connection final : public mozIStorageConnection
356364
*/
357365
const int mFlags;
358366

367+
/**
368+
* Stores whether we should ask sqlite3_open_v2 to ignore locking.
369+
*/
370+
const bool mIgnoreLockingMode;
371+
359372
// This is here for two reasons: 1) It's used to make sure that the
360373
// connections do not outlive the service. 2) Our custom collating functions
361374
// call its localeCompareStrings() method.

storage/mozStorageService.cpp

Lines changed: 44 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -748,11 +748,43 @@ Service::OpenAsyncDatabase(nsIVariant *aDatabaseStore,
748748
NS_ENSURE_ARG(aDatabaseStore);
749749
NS_ENSURE_ARG(aCallback);
750750

751-
nsCOMPtr<nsIFile> storageFile;
752-
int flags = SQLITE_OPEN_READWRITE;
751+
nsresult rv;
752+
bool shared = false;
753+
bool readOnly = false;
754+
bool ignoreLockingMode = false;
755+
int32_t growthIncrement = -1;
756+
757+
#define FAIL_IF_SET_BUT_INVALID(rv)\
758+
if (NS_FAILED(rv) && rv != NS_ERROR_NOT_AVAILABLE) { \
759+
return NS_ERROR_INVALID_ARG; \
760+
}
761+
762+
// Deal with options first:
763+
if (aOptions) {
764+
rv = aOptions->GetPropertyAsBool(NS_LITERAL_STRING("readOnly"), &readOnly);
765+
FAIL_IF_SET_BUT_INVALID(rv);
766+
767+
rv = aOptions->GetPropertyAsBool(NS_LITERAL_STRING("ignoreLockingMode"),
768+
&ignoreLockingMode);
769+
FAIL_IF_SET_BUT_INVALID(rv);
770+
// Specifying ignoreLockingMode will force use of the readOnly flag:
771+
if (ignoreLockingMode) {
772+
readOnly = true;
773+
}
774+
775+
rv = aOptions->GetPropertyAsBool(NS_LITERAL_STRING("shared"), &shared);
776+
FAIL_IF_SET_BUT_INVALID(rv);
777+
778+
// NB: we re-set to -1 if we don't have a storage file later on.
779+
rv = aOptions->GetPropertyAsInt32(NS_LITERAL_STRING("growthIncrement"),
780+
&growthIncrement);
781+
FAIL_IF_SET_BUT_INVALID(rv);
782+
}
783+
int flags = readOnly ? SQLITE_OPEN_READONLY : SQLITE_OPEN_READWRITE;
753784

785+
nsCOMPtr<nsIFile> storageFile;
754786
nsCOMPtr<nsISupports> dbStore;
755-
nsresult rv = aDatabaseStore->GetAsISupports(getter_AddRefs(dbStore));
787+
rv = aDatabaseStore->GetAsISupports(getter_AddRefs(dbStore));
756788
if (NS_SUCCEEDED(rv)) {
757789
// Generally, aDatabaseStore holds the database nsIFile.
758790
storageFile = do_QueryInterface(dbStore, &rv);
@@ -763,17 +795,12 @@ Service::OpenAsyncDatabase(nsIVariant *aDatabaseStore,
763795
rv = storageFile->Clone(getter_AddRefs(storageFile));
764796
MOZ_ASSERT(NS_SUCCEEDED(rv));
765797

766-
// Ensure that SQLITE_OPEN_CREATE is passed in for compatibility reasons.
767-
flags |= SQLITE_OPEN_CREATE;
768-
769-
// Extract and apply the shared-cache option.
770-
bool shared = false;
771-
if (aOptions) {
772-
rv = aOptions->GetPropertyAsBool(NS_LITERAL_STRING("shared"), &shared);
773-
if (NS_FAILED(rv) && rv != NS_ERROR_NOT_AVAILABLE) {
774-
return NS_ERROR_INVALID_ARG;
775-
}
798+
if (!readOnly) {
799+
// Ensure that SQLITE_OPEN_CREATE is passed in for compatibility reasons.
800+
flags |= SQLITE_OPEN_CREATE;
776801
}
802+
803+
// Apply the shared-cache option.
777804
flags |= shared ? SQLITE_OPEN_SHAREDCACHE : SQLITE_OPEN_PRIVATECACHE;
778805
} else {
779806
// Sometimes, however, it's a special database name.
@@ -787,17 +814,13 @@ Service::OpenAsyncDatabase(nsIVariant *aDatabaseStore,
787814
// connection to use a memory DB.
788815
}
789816

790-
int32_t growthIncrement = -1;
791-
if (aOptions && storageFile) {
792-
rv = aOptions->GetPropertyAsInt32(NS_LITERAL_STRING("growthIncrement"),
793-
&growthIncrement);
794-
if (NS_FAILED(rv) && rv != NS_ERROR_NOT_AVAILABLE) {
795-
return NS_ERROR_INVALID_ARG;
796-
}
817+
if (!storageFile && growthIncrement >= 0) {
818+
return NS_ERROR_INVALID_ARG;
797819
}
798820

799821
// Create connection on this thread, but initialize it on its helper thread.
800-
RefPtr<Connection> msc = new Connection(this, flags, true);
822+
RefPtr<Connection> msc = new Connection(this, flags, true,
823+
ignoreLockingMode);
801824
nsCOMPtr<nsIEventTarget> target = msc->getAsyncExecutionTarget();
802825
MOZ_ASSERT(target, "Cannot initialize a connection that has been closed already");
803826

storage/test/unit/test_storage_connection.js

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -355,12 +355,27 @@ add_task(function* test_open_async() {
355355
yield standardAsyncTest(openAsyncDatabase("memory"),
356356
"in-memory database", true);
357357
yield standardAsyncTest(openAsyncDatabase("memory",
358-
{shared: false, growthIncrement: 54}),
358+
{shared: false}),
359359
"in-memory database and options", true);
360360

361-
do_print("Testing async opening with bogus options 1");
361+
do_print("Testing async opening with bogus options 0");
362362
let raised = false;
363363
let adb = null;
364+
365+
try {
366+
adb = yield openAsyncDatabase("memory", {shared: false, growthIncrement: 54});
367+
} catch (ex) {
368+
raised = true;
369+
} finally {
370+
if (adb) {
371+
yield asyncClose(adb);
372+
}
373+
}
374+
do_check_true(raised);
375+
376+
do_print("Testing async opening with bogus options 1");
377+
raised = false;
378+
adb = null;
364379
try {
365380
adb = yield openAsyncDatabase(getTestDB(), {shared: "forty-two"});
366381
} catch (ex) {

toolkit/modules/Sqlite.jsm

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -868,6 +868,15 @@ ConnectionData.prototype = Object.freeze({
868868
* *not* a timer on the idle service and this could fire while the
869869
* application is active.
870870
*
871+
* readOnly -- (bool) Whether to open the database with SQLITE_OPEN_READONLY
872+
* set. If used, writing to the database will fail. Defaults to false.
873+
*
874+
* ignoreLockingMode -- (bool) Whether to ignore locks on the database held
875+
* by other connections. If used, implies readOnly. Defaults to false.
876+
* USE WITH EXTREME CAUTION. This mode WILL produce incorrect results or
877+
* return "false positive" corruption errors if other connections write
878+
* to the DB at the same time.
879+
*
871880
* FUTURE options to control:
872881
*
873882
* special named databases
@@ -915,12 +924,21 @@ function openConnection(options) {
915924
log.info("Opening database: " + path + " (" + identifier + ")");
916925

917926
return new Promise((resolve, reject) => {
918-
let dbOptions = null;
927+
let dbOptions = Cc["@mozilla.org/hash-property-bag;1"].
928+
createInstance(Ci.nsIWritablePropertyBag);
919929
if (!sharedMemoryCache) {
920-
dbOptions = Cc["@mozilla.org/hash-property-bag;1"].
921-
createInstance(Ci.nsIWritablePropertyBag);
922930
dbOptions.setProperty("shared", false);
923931
}
932+
if (options.readOnly) {
933+
dbOptions.setProperty("readOnly", true);
934+
}
935+
if (options.ignoreLockingMode) {
936+
dbOptions.setProperty("ignoreLockingMode", true);
937+
dbOptions.setProperty("readOnly", true);
938+
}
939+
940+
dbOptions = dbOptions.enumerator.hasMoreElements() ? dbOptions : null;
941+
924942
Services.storage.openAsyncDatabase(file, dbOptions, (status, connection) => {
925943
if (!connection) {
926944
log.warn(`Could not open connection to ${path}: ${status}`);

0 commit comments

Comments
 (0)