Merge "Keystore 2.0: Make MonotonicRawTime use milliseconds."
diff --git a/keystore2/src/database.rs b/keystore2/src/database.rs
index d18223e..073799b 100644
--- a/keystore2/src/database.rs
+++ b/keystore2/src/database.rs
@@ -829,6 +829,20 @@
/// Name of the file that holds the cross-boot persistent database.
pub const PERSISTENT_DB_FILENAME: &'static str = &"persistent.sqlite";
+ /// Set write-ahead logging mode on the persistent database found in `db_root`.
+ pub fn set_wal_mode(db_root: &Path) -> Result<()> {
+ let path = Self::make_persistent_path(&db_root)?;
+ let conn =
+ Connection::open(path).context("In KeystoreDB::set_wal_mode: Failed to open DB")?;
+ let mode: String = conn
+ .pragma_update_and_check(None, "journal_mode", &"WAL", |row| row.get(0))
+ .context("In KeystoreDB::set_wal_mode: Failed to set journal_mode")?;
+ match mode.as_str() {
+ "wal" => Ok(()),
+ _ => Err(anyhow!("Unable to set WAL mode, db is still in {} mode.", mode)),
+ }
+ }
+
/// This will create a new database connection connecting the two
/// files persistent.sqlite and perboot.sqlite in the given directory.
/// It also attempts to initialize all of the tables.
@@ -837,18 +851,8 @@
pub fn new(db_root: &Path, gc: Option<Arc<Gc>>) -> Result<Self> {
let _wp = wd::watch_millis("KeystoreDB::new", 500);
- // Build the path to the sqlite file.
- let mut persistent_path = db_root.to_path_buf();
- persistent_path.push(Self::PERSISTENT_DB_FILENAME);
-
- // Now convert them to strings prefixed with "file:"
- let mut persistent_path_str = "file:".to_owned();
- persistent_path_str.push_str(&persistent_path.to_string_lossy());
-
- let conn = Self::make_connection(&persistent_path_str)?;
-
- // On busy fail Immediately. It is unlikely to succeed given a bug in sqlite.
- conn.busy_handler(None).context("In KeystoreDB::new: Failed to set busy handler.")?;
+ let persistent_path = Self::make_persistent_path(&db_root)?;
+ let conn = Self::make_connection(&persistent_path)?;
let mut db = Self { conn, gc, perboot: perboot::PERBOOT_DB.clone() };
db.with_transaction(TransactionBehavior::Immediate, |tx| {
@@ -967,6 +971,18 @@
Ok(())
}
+ fn make_persistent_path(db_root: &Path) -> Result<String> {
+ // Build the path to the sqlite file.
+ let mut persistent_path = db_root.to_path_buf();
+ persistent_path.push(Self::PERSISTENT_DB_FILENAME);
+
+ // Now convert them to strings prefixed with "file:"
+ let mut persistent_path_str = "file:".to_owned();
+ persistent_path_str.push_str(&persistent_path.to_string_lossy());
+
+ Ok(persistent_path_str)
+ }
+
fn make_connection(persistent_file: &str) -> Result<Connection> {
let conn =
Connection::open_in_memory().context("Failed to initialize SQLite connection.")?;
@@ -1409,18 +1425,6 @@
.context("In get_or_create_key_with.")
}
- /// SQLite3 seems to hold a shared mutex while running the busy handler when
- /// waiting for the database file to become available. This makes it
- /// impossible to successfully recover from a locked database when the
- /// transaction holding the device busy is in the same process on a
- /// different connection. As a result the busy handler has to time out and
- /// fail in order to make progress.
- ///
- /// Instead, we set the busy handler to None (return immediately). And catch
- /// Busy and Locked errors (the latter occur on in memory databases with
- /// shared cache, e.g., the per-boot database.) and restart the transaction
- /// after a grace period of half a millisecond.
- ///
/// Creates a transaction with the given behavior and executes f with the new transaction.
/// The transaction is committed only if f returns Ok and retried if DatabaseBusy
/// or DatabaseLocked is encountered.
@@ -3176,6 +3180,7 @@
use android_hardware_security_secureclock::aidl::android::hardware::security::secureclock::{
Timestamp::Timestamp,
};
+ use rusqlite::DatabaseName::Attached;
use rusqlite::NO_PARAMS;
use rusqlite::TransactionBehavior;
use std::cell::RefCell;
@@ -5484,4 +5489,26 @@
assert_eq!(db.find_auth_token_entry(|_| true).unwrap().0.auth_token.mac, b"mac2".to_vec());
Ok(())
}
+
+ #[test]
+ fn test_set_wal_mode() -> Result<()> {
+ let temp_dir = TempDir::new("test_set_wal_mode")?;
+ let mut db = KeystoreDB::new(temp_dir.path(), None)?;
+ let mode: String =
+ db.conn.pragma_query_value(Some(Attached("persistent")), "journal_mode", |row| {
+ row.get(0)
+ })?;
+ assert_eq!(mode, "delete");
+ db.conn.close().expect("Close didn't work");
+
+ KeystoreDB::set_wal_mode(temp_dir.path())?;
+
+ db = KeystoreDB::new(temp_dir.path(), None)?;
+ let mode: String =
+ db.conn.pragma_query_value(Some(Attached("persistent")), "journal_mode", |row| {
+ row.get(0)
+ })?;
+ assert_eq!(mode, "wal");
+ Ok(())
+ }
}
diff --git a/keystore2/src/globals.rs b/keystore2/src/globals.rs
index 8d39b22..89114a6 100644
--- a/keystore2/src/globals.rs
+++ b/keystore2/src/globals.rs
@@ -39,11 +39,12 @@
use binder::FromIBinder;
use keystore2_vintf::get_aidl_instances;
use lazy_static::lazy_static;
-use std::sync::{Arc, Mutex};
+use std::sync::{Arc, Mutex, RwLock};
use std::{cell::RefCell, sync::Once};
use std::{collections::HashMap, path::Path, path::PathBuf};
static DB_INIT: Once = Once::new();
+static DB_SET_WAL_MODE: Once = Once::new();
/// Open a connection to the Keystore 2.0 database. This is called during the initialization of
/// the thread local DB field. It should never be called directly. The first time this is called
@@ -54,11 +55,16 @@
/// is run only once, as long as the ASYNC_TASK instance is the same. So only one additional
/// database connection is created for the garbage collector worker.
pub fn create_thread_local_db() -> KeystoreDB {
- let mut db = KeystoreDB::new(
- &DB_PATH.lock().expect("Could not get the database directory."),
- Some(GC.clone()),
- )
- .expect("Failed to open database.");
+ let db_path = DB_PATH.read().expect("Could not get the database directory.");
+
+ DB_SET_WAL_MODE.call_once(|| {
+ log::info!("Setting Keystore 2.0 database to WAL mode first time since boot.");
+ KeystoreDB::set_wal_mode(&db_path)
+ .expect("In create_thread_local_db: Could not set WAL mode.");
+ });
+
+ let mut db = KeystoreDB::new(&db_path, Some(GC.clone())).expect("Failed to open database.");
+
DB_INIT.call_once(|| {
log::info!("Touching Keystore 2.0 database for this first time since boot.");
db.insert_last_off_body(MonotonicRawTime::now());
@@ -139,7 +145,7 @@
lazy_static! {
/// The path where keystore stores all its keys.
- pub static ref DB_PATH: Mutex<PathBuf> = Mutex::new(
+ pub static ref DB_PATH: RwLock<PathBuf> = RwLock::new(
Path::new("/data/misc/keystore").to_path_buf());
/// Runtime database of unwrapped super keys.
pub static ref SUPER_KEY: Arc<SuperKeyManager> = Default::default();
@@ -157,7 +163,7 @@
/// LegacyBlobLoader is initialized and exists globally.
/// The same directory used by the database is used by the LegacyBlobLoader as well.
pub static ref LEGACY_BLOB_LOADER: Arc<LegacyBlobLoader> = Arc::new(LegacyBlobLoader::new(
- &DB_PATH.lock().expect("Could not get the database path for legacy blob loader.")));
+ &DB_PATH.read().expect("Could not get the database path for legacy blob loader.")));
/// Legacy migrator. Atomically migrates legacy blobs to the database.
pub static ref LEGACY_MIGRATOR: Arc<LegacyMigrator> =
Arc::new(LegacyMigrator::new(Arc::new(Default::default())));
@@ -173,7 +179,7 @@
map_km_error(km_dev.deleteKey(&*blob))
.context("In invalidate key closure: Trying to invalidate key blob.")
}),
- KeystoreDB::new(&DB_PATH.lock().expect("Could not get the database directory."), None)
+ KeystoreDB::new(&DB_PATH.read().expect("Could not get the database directory."), None)
.expect("Failed to open database."),
SUPER_KEY.clone(),
)
diff --git a/keystore2/src/keystore2_main.rs b/keystore2/src/keystore2_main.rs
index 29330a6..53461da 100644
--- a/keystore2/src/keystore2_main.rs
+++ b/keystore2/src/keystore2_main.rs
@@ -56,7 +56,7 @@
// For the ground truth check the service startup rule for init (typically in keystore2.rc).
let id_rotation_state = if let Some(dir) = args.next() {
let db_path = Path::new(&dir);
- *keystore2::globals::DB_PATH.lock().expect("Could not lock DB_PATH.") =
+ *keystore2::globals::DB_PATH.write().expect("Could not lock DB_PATH.") =
db_path.to_path_buf();
IdRotationState::new(&db_path)
} else {
@@ -121,7 +121,7 @@
}
let vpnprofilestore = VpnProfileStore::new_native_binder(
- &keystore2::globals::DB_PATH.lock().expect("Could not get DB_PATH."),
+ &keystore2::globals::DB_PATH.read().expect("Could not get DB_PATH."),
);
binder::add_service(VPNPROFILESTORE_SERVICE_NAME, vpnprofilestore.as_binder()).unwrap_or_else(
|e| {
diff --git a/keystore2/vpnprofilestore/lib.rs b/keystore2/vpnprofilestore/lib.rs
index 8b3bc2b..df2731a 100644
--- a/keystore2/vpnprofilestore/lib.rs
+++ b/keystore2/vpnprofilestore/lib.rs
@@ -22,7 +22,7 @@
BinderFeatures, ExceptionCode, Result as BinderResult, Status as BinderStatus, Strong,
ThreadState,
};
-use anyhow::{Context, Result};
+use anyhow::{anyhow, Context, Result};
use keystore2::{async_task::AsyncTask, legacy_blob::LegacyBlobLoader, utils::watchdog as wd};
use rusqlite::{
params, Connection, OptionalExtension, Transaction, TransactionBehavior, NO_PARAMS,
@@ -30,25 +30,42 @@
use std::{
collections::HashSet,
path::{Path, PathBuf},
+ sync::Once,
};
+static DB_SET_WAL_MODE: Once = Once::new();
+
struct DB {
conn: Connection,
}
impl DB {
fn new(db_file: &Path) -> Result<Self> {
+ DB_SET_WAL_MODE.call_once(|| {
+ log::info!("Setting VpnProfileStore database to WAL mode first time since boot.");
+ Self::set_wal_mode(&db_file).expect("In vpnprofilestore: Could not set WAL mode.");
+ });
+
let mut db = Self {
conn: Connection::open(db_file).context("Failed to initialize SQLite connection.")?,
};
- // On busy fail Immediately. It is unlikely to succeed given a bug in sqlite.
- db.conn.busy_handler(None).context("Failed to set busy handler.")?;
-
db.init_tables().context("Trying to initialize vpnstore db.")?;
Ok(db)
}
+ fn set_wal_mode(db_file: &Path) -> Result<()> {
+ let conn = Connection::open(db_file)
+ .context("In VpnProfileStore set_wal_mode: Failed to open DB.")?;
+ let mode: String = conn
+ .pragma_update_and_check(None, "journal_mode", &"WAL", |row| row.get(0))
+ .context("In VpnProfileStore set_wal_mode: Failed to set journal_mode")?;
+ match mode.as_str() {
+ "wal" => Ok(()),
+ _ => Err(anyhow!("Unable to set WAL mode, db is still in {} mode.", mode)),
+ }
+ }
+
fn with_transaction<T, F>(&mut self, behavior: TransactionBehavior, f: F) -> Result<T>
where
F: Fn(&Transaction) -> Result<T>,
@@ -470,6 +487,9 @@
const PROFILE_COUNT: u32 = 5000u32;
const PROFILE_DB_COUNT: u32 = 5000u32;
+ let mode: String = db.conn.pragma_query_value(None, "journal_mode", |row| row.get(0))?;
+ assert_eq!(mode, "wal");
+
let mut actual_profile_count = PROFILE_COUNT;
// First insert PROFILE_COUNT profiles.
for count in 0..PROFILE_COUNT {