Bug 1638927 - Replace dedicated `XULStore` persistence thread with background tasks. r=KrisWright

Differential Revision: https://phabricator.services.mozilla.com/D75864

toolkit/components/xulstore/src/ffi.rs

@@ -3,7 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use crate as XULStore;
-use crate::{iter::XULStoreIterator, persist::clear_on_shutdown, statics::update_profile_dir};
+use crate::{iter::XULStoreIterator, statics::update_profile_dir};
 use libc::{c_char, c_void};
 use nserror::{nsresult, NS_ERROR_NOT_IMPLEMENTED, NS_OK};
 use nsstring::{nsAString, nsString};
@@ -212,27 +212,6 @@ impl ProfileChangeObserver {
     }
 }
 
-#[derive(xpcom)]
-#[xpimplements(nsIObserver)]
-#[refcnt = "nonatomic"]
-pub(crate) struct InitXpcomShutdownObserver {}
-impl XpcomShutdownObserver {
-    #[allow(non_snake_case)]
-    unsafe fn Observe(
-        &self,
-        _subject: *const nsISupports,
-        _topic: *const c_char,
-        _data: *const i16,
-    ) -> nsresult {
-        clear_on_shutdown();
-        NS_OK
-    }
-
-    pub(crate) fn new() -> RefPtr<XpcomShutdownObserver> {
-        XpcomShutdownObserver::allocate(InitXpcomShutdownObserver {})
-    }
-}
-
 #[no_mangle]
 pub unsafe extern "C" fn xulstore_set_value(
     doc: &nsAString,

toolkit/components/xulstore/src/lib.rs

@@ -223,4 +223,4 @@ pub(crate) fn get_attrs(doc: &nsAString, id: &nsAString) -> XULStoreResult<XULSt
 
 pub(crate) fn shutdown() -> XULStoreResult<()> {
     flush_writes()
-}
+}

toolkit/components/xulstore/src/persist.rs

@@ -16,16 +16,15 @@
 
 use crate::{
     error::{XULStoreError, XULStoreResult},
-    ffi::XpcomShutdownObserver,
     statics::get_database,
 };
 use crossbeam_utils::atomic::AtomicCell;
 use lmdb::Error as LmdbError;
-use moz_task::{create_thread, Task, TaskRunnable};
+use moz_task::{dispatch_background_task_with_options, DispatchOptions, Task, TaskRunnable};
 use nserror::nsresult;
 use rkv::{StoreError as RkvStoreError, Value};
 use std::{collections::HashMap, sync::Mutex, thread::sleep, time::Duration};
-use xpcom::{interfaces::nsIThread, RefPtr, ThreadBoundRefPtr};
+use xpcom::RefPtr;
 
 lazy_static! {
     /// A map of key/value pairs to persist.  Values are Options so we can
@@ -45,36 +44,6 @@ lazy_static! {
     /// since each task opens the database, and we need to ensure there is only
     /// one open database handle for the database at any given time.
     static ref PERSIST: Mutex<()> = { Mutex::new(()) };
-
-    static ref THREAD: Mutex<Option<ThreadBoundRefPtr<nsIThread>>> = {
-        let thread: RefPtr<nsIThread> = match create_thread("XULStore") {
-            Ok(thread) => thread,
-            Err(err) => {
-                error!("error creating XULStore thread: {}", err);
-                return Mutex::new(None);
-            }
-        };
-
-        // Observe XPCOM shutdown so we can clear the thread and thus not
-        // "leak" it (from the perspective of the leak checker).
-        observe_xpcom_shutdown();
-
-        Mutex::new(Some(ThreadBoundRefPtr::new(thread)))
-    };
-}
-
-fn observe_xpcom_shutdown() {
-    (|| -> XULStoreResult<()> {
-        let obs_svc = xpcom::services::get_ObserverService().ok_or(XULStoreError::Unavailable)?;
-        let observer = XpcomShutdownObserver::new();
-        unsafe {
-            obs_svc
-                .AddObserver(observer.coerce(), cstr!("xpcom-shutdown").as_ptr(), false)
-                .to_result()?
-        };
-        Ok(())
-    })()
-    .unwrap_or_else(|err| error!("error observing XPCOM shutdown: {}", err));
 }
 
 /// Synchronously persists changes recorded in memory to disk. Typically
@@ -121,7 +90,7 @@ fn sync_persist() -> XULStoreResult<()> {
     Ok(())
 }
 
-pub(crate) fn flush_writes() ->XULStoreResult<()> {
+pub(crate) fn flush_writes() -> XULStoreResult<()> {
     // One of three things will happen here (barring unexpected errors):
     // - There are no writes queued and the background thread is idle. In which
     //   case, we will get the lock, see that there's nothing to write, and
@@ -148,19 +117,11 @@ pub(crate) fn flush_writes() ->XULStoreResult<()> {
             info!("Unable to persist xulstore");
         }
 
-        Err(err) => return Err(err.into())
+        Err(err) => return Err(err.into()),
     }
     Ok(())
 }
 
-pub(crate) fn clear_on_shutdown() {
-    (|| -> XULStoreResult<()> {
-        THREAD.lock()?.take();
-        Ok(())
-    })()
-    .unwrap_or_else(|err| error!("error clearing thread: {}", err));
-}
-
 pub(crate) fn persist(key: String, value: Option<String>) -> XULStoreResult<()> {
     let mut changes = CHANGES.lock()?;
 
@@ -170,13 +131,10 @@ pub(crate) fn persist(key: String, value: Option<String>) -> XULStoreResult<()>
         // If *changes* was `None`, then this is the first change since
         // the last time we persisted, so dispatch a new PersistTask.
         let task = Box::new(PersistTask::new());
-        let thread_guard = THREAD.lock()?;
-        let thread = thread_guard
-            .as_ref()
-            .ok_or(XULStoreError::Unavailable)?
-            .get_ref()
-            .ok_or(XULStoreError::Unavailable)?;
-        TaskRunnable::dispatch(TaskRunnable::new("XULStore::Persist", task)?, thread)?;
+        dispatch_background_task_with_options(
+            RefPtr::new(TaskRunnable::new("XULStore::Persist", task)?.coerce()),
+            DispatchOptions::default().may_block(true),
+        )?;
     }
 
     // Now insert the key/value pair into the map.  The unwrap() call here