Bug 1472722 - Use the unix-excl Sqlite VFS by default. r=nalexander,asuth
authorMarco Bonardo <mbonardo@mozilla.com>
Fri, 13 Jul 2018 16:54:57 +0000
changeset 818534 b3494bf0c3d31048eb0af0324f107208d7e5ecc6
parent 818533 04dd259d71db60341016eccf53ced43742319631
child 818535 587bc807f560fa912e04a2176b6a1a794d009432
push id116284
push userhikezoe@mozilla.com
push dateSun, 15 Jul 2018 11:20:01 +0000
reviewersnalexander, asuth
bugs1472722
milestone63.0a1
Bug 1472722 - Use the unix-excl Sqlite VFS by default. r=nalexander,asuth Use the exclusive VFS on unix systems, so that: 1. we can avoid the memory mapped -shm files in wal mode 2. we gain more compatibility with nfs shares 3. we gain some protection from third parties touching open dbs On the other side it won't be possible anymore to use an open database from a different process (like the Sqlite command line), for which we provide an hidden pref: storage.multiProcessAccess.enabled Differential Revision: https://phabricator.services.mozilla.com/D1964
mobile/android/app/mobile.js
storage/TelemetryVFS.cpp
--- a/mobile/android/app/mobile.js
+++ b/mobile/android/app/mobile.js
@@ -44,16 +44,20 @@ pref("browser.tabs.useCache", false);
 // From libpref/src/init/all.js, extended to allow a slightly wider zoom range.
 pref("zoom.minPercent", 20);
 pref("zoom.maxPercent", 400);
 pref("toolkit.zoomManager.zoomValues", ".2,.3,.5,.67,.8,.9,1,1.1,1.2,1.33,1.5,1.7,2,2.4,3,4");
 
 // Mobile will use faster, less durable mode.
 pref("toolkit.storage.synchronous", 0);
 
+// Android needs concurrent access to the same database from multiple processes,
+// thus we can't use exclusive locking on it.
+pref("storage.multiProcessAccess.enabled", true);
+
 pref("browser.viewport.desktopWidth", 980);
 // The default fallback zoom level to render pages at. Set to -1 to fit page; otherwise
 // the value is divided by 1000 and clamped to hard-coded min/max scale values.
 pref("browser.viewport.defaultZoom", -1);
 
 // Show/Hide scrollbars when active/inactive
 pref("ui.showHideScrollbars", 1);
 pref("ui.useOverlayScrollbars", 1);
--- a/storage/TelemetryVFS.cpp
+++ b/storage/TelemetryVFS.cpp
@@ -17,24 +17,31 @@
 
 // The last VFS version for which this file has been updated.
 #define LAST_KNOWN_VFS_VERSION 3
 
 // The last io_methods version for which this file has been updated.
 #define LAST_KNOWN_IOMETHODS_VERSION 3
 
 /**
- * This preference is a workaround to allow users/sysadmins to identify
- * that the profile exists on an NFS share whose implementation
- * is incompatible with SQLite's default locking implementation.
- * Bug 433129 attempted to automatically identify such file-systems,
- * but a reliable way was not found and it was determined that the fallback
- * locking is slower than POSIX locking, so we do not want to do it by default.
-*/
-#define PREF_NFS_FILESYSTEM   "storage.nfs_filesystem"
+ * By default use the unix-excl VFS, for the following reasons:
+ * 1. It improves compatibility with NFS shares, whose implementation
+ *    is incompatible with SQLite's locking requirements.
+ *    Bug 433129 attempted to automatically identify such file-systems,
+ *    but a reliable way was not found and the fallback locking is slower than
+ *    POSIX locking, so we do not want to do it by default.
+ * 2. It allows wal mode to avoid the memory mapped -shm file, reducing the
+ *    likelihood of SIGBUS failures when disk space is exhausted.
+ * 3. It provides some protection from third party database tampering while a
+ *    connection is open.
+ * This preference allows to revert to the "unix" VFS, that is not exclusive,
+ * thus it can be used by developers to query a database through the Sqlite
+ * command line while it's already in use.
+ */
+#define PREF_MULTI_PROCESS_ACCESS "storage.multiProcessAccess.enabled"
 
 namespace {
 
 using namespace mozilla;
 using namespace mozilla::dom::quota;
 using namespace mozilla::net;
 
 struct Histograms {
@@ -860,32 +867,32 @@ namespace storage {
 const char *GetVFSName()
 {
   return "telemetry-vfs";
 }
 
 sqlite3_vfs* ConstructTelemetryVFS()
 {
 #if defined(XP_WIN)
-#define EXPECTED_VFS     "win32"
-#define EXPECTED_VFS_NFS "win32"
+#define EXPECTED_VFS      "win32"
+#define EXPECTED_VFS_EXCL "win32"
 #else
-#define EXPECTED_VFS     "unix"
-#define EXPECTED_VFS_NFS "unix-excl"
+#define EXPECTED_VFS      "unix"
+#define EXPECTED_VFS_EXCL "unix-excl"
 #endif
 
   bool expected_vfs;
   sqlite3_vfs *vfs;
-  if (Preferences::GetBool(PREF_NFS_FILESYSTEM)) {
-    vfs = sqlite3_vfs_find(EXPECTED_VFS_NFS);
-    expected_vfs = (vfs != nullptr);
-  }
-  else {
+  if (Preferences::GetBool(PREF_MULTI_PROCESS_ACCESS, false)) {
+    // Use the non-exclusive VFS.
     vfs = sqlite3_vfs_find(nullptr);
     expected_vfs = vfs->zName && !strcmp(vfs->zName, EXPECTED_VFS);
+  } else {
+    vfs = sqlite3_vfs_find(EXPECTED_VFS_EXCL);
+    expected_vfs = (vfs != nullptr);
   }
   if (!expected_vfs) {
     return nullptr;
   }
 
   sqlite3_vfs *tvfs = new ::sqlite3_vfs;
   memset(tvfs, 0, sizeof(::sqlite3_vfs));
   // If the VFS version is higher than the last known one, you should update