diff --git a/components/webext-storage/src/db.rs b/components/webext-storage/src/db.rs index e24c8ea97c..b8b8a42eaa 100644 --- a/components/webext-storage/src/db.rs +++ b/components/webext-storage/src/db.rs @@ -11,7 +11,8 @@ use rusqlite::Connection; use rusqlite::OpenFlags; use sql_support::open_database::open_database_with_flags; use sql_support::ConnExt; -use std::ops::{Deref, DerefMut}; +use std::ops::Deref; +// use std::ops::{Deref, DerefMut}; use std::path::{Path, PathBuf}; use std::sync::Arc; use url::Url; @@ -23,10 +24,16 @@ use url::Url; /// /// We only support a single writer connection - so that's the only thing we /// store. It's still a bit overkill, but there's only so many yaks in a day. +pub enum WebExtStorageDb { + Open(Connection), + Closed, +} + pub struct StorageDb { - writer: Connection, + pub writer: WebExtStorageDb, interrupt_handle: Arc, } + impl StorageDb { /// Create a new, or fetch an already open, StorageDb backed by a file on disk. pub fn new(db_path: impl AsRef) -> Result { @@ -54,7 +61,7 @@ impl StorageDb { let conn = open_database_with_flags(db_path, flags, &schema::WebExtMigrationLogin)?; Ok(Self { interrupt_handle: Arc::new(SqlInterruptHandle::new(&conn)), - writer: conn, + writer: WebExtStorageDb::Open(conn), }) } @@ -73,31 +80,28 @@ impl StorageDb { /// underlying connection so the caller can retry but (a) that's very tricky /// in an Arc> world and (b) we never actually took advantage of /// that retry capability. - pub fn close(self) -> Result<()> { - self.writer.close().map_err(|(writer, err)| { - // In rusqlite 0.28.0 and earlier, if we just let `writer` drop, - // the close would panic on failure. - // Later rusqlite versions will not panic, but this behavior doesn't - // hurt there. - std::mem::forget(writer); - err.into() - }) + pub fn close(&mut self) -> Result<()> { + let conn = match std::mem::replace(&mut self.writer, WebExtStorageDb::Closed) { + WebExtStorageDb::Open(conn) => conn, + WebExtStorageDb::Closed => return Ok(()), + }; + conn.close().map_err(|(_,y)| Error::SqlError(y)) } } -impl Deref for StorageDb { - type Target = Connection; +// impl Deref for StorageDb { +// type Target = Connection; - fn deref(&self) -> &Self::Target { - &self.writer - } -} +// fn deref(&self) -> &Self::Target { +// &self.writer +// } +// } -impl DerefMut for StorageDb { - fn deref_mut(&mut self) -> &mut Self::Target { - &mut self.writer - } -} +// impl DerefMut for StorageDb { +// fn deref_mut(&mut self) -> &mut Self::Target { +// &mut self.writer +// } +// } // We almost exclusively use this ThreadSafeStorageDb pub struct ThreadSafeStorageDb { diff --git a/components/webext-storage/src/store.rs b/components/webext-storage/src/store.rs index d9c3281dd4..dc05be503c 100644 --- a/components/webext-storage/src/store.rs +++ b/components/webext-storage/src/store.rs @@ -3,7 +3,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use crate::api::{self, StorageChanges}; -use crate::db::{StorageDb, ThreadSafeStorageDb}; +use crate::db::{StorageDb, ThreadSafeStorageDb, WebExtStorageDb}; use crate::error::*; use crate::migration::{migrate, MigrationInfo}; use crate::sync; @@ -157,7 +157,7 @@ impl WebExtStorageStore { } }; // consume the mutex and get back the inner. - let db = shared.into_inner(); + let mut db = shared.into_inner(); db.close() } @@ -177,12 +177,17 @@ impl WebExtStorageStore { /// /// Note that `filename` isn't normalized or canonicalized. pub fn migrate(&self, filename: impl AsRef) -> Result<()> { - let db = self.db.lock(); - let tx = db.unchecked_transaction()?; + let db = &self.db.lock().writer; + let conn = match db { + WebExtStorageDb::Open(y) => y, + WebExtStorageDb::Closed => return Err(Error::DatabaseConnectionClosed), + }; + + let tx = conn.unchecked_transaction()?; let result = migrate(&tx, filename.as_ref())?; tx.commit()?; // Failing to store this information should not cause migration failure. - if let Err(e) = result.store(&db) { + if let Err(e) = result.store(&conn) { debug_assert!(false, "Migration error: {:?}", e); log::warn!("Failed to record migration telmetry: {}", e); }