Bug 1638920 - use a background task queue in cert_storage rather than a dedicated thread r=lina

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

Cargo.lock

@@ -529,6 +529,7 @@ dependencies = [
  "base64 0.10.1",
  "byteorder",
  "crossbeam-utils 0.6.5",
+ "cstr",
  "log",
  "memmap",
  "moz_task",

security/manager/ssl/cert_storage/Cargo.toml

@@ -7,6 +7,7 @@ authors = ["Dana Keeler <dkeeler@mozilla.com>", "Mark Goodwin <mgoodwin@mozilla.
 base64 = "0.10"
 byteorder = "1.2.7"
 crossbeam-utils = "0.6.3"
+cstr = "0.1"
 log = "0.4"
 memmap = "0.7"
 moz_task = { path = "../../../../xpcom/rust/moz_task" }

security/manager/ssl/cert_storage/src/lib.rs

@@ -6,6 +6,8 @@ extern crate base64;
 extern crate byteorder;
 extern crate crossbeam_utils;
 #[macro_use]
+extern crate cstr;
+#[macro_use]
 extern crate log;
 extern crate memmap;
 extern crate moz_task;
@@ -26,7 +28,7 @@ extern crate tempfile;
 use byteorder::{LittleEndian, NetworkEndian, ReadBytesExt, WriteBytesExt};
 use crossbeam_utils::atomic::AtomicCell;
 use memmap::Mmap;
-use moz_task::{create_thread, is_main_thread, Task, TaskRunnable};
+use moz_task::{create_background_task_queue, is_main_thread, Task, TaskRunnable};
 use nserror::{
     nsresult, NS_ERROR_FAILURE, NS_ERROR_NOT_SAME_THREAD, NS_ERROR_NO_AGGREGATION,
     NS_ERROR_NULL_POINTER, NS_ERROR_UNEXPECTED, NS_OK,
@@ -46,14 +48,14 @@ use std::os::raw::c_char;
 use std::path::{Path, PathBuf};
 use std::slice;
 use std::str;
-use std::sync::{Arc, Mutex, RwLock};
+use std::sync::{Arc, RwLock};
 use std::time::{Duration, SystemTime};
 use storage_variant::VariantType;
 use thin_vec::ThinVec;
 use xpcom::interfaces::{
     nsICRLiteState, nsICertInfo, nsICertStorage, nsICertStorageCallback, nsIFile,
     nsIIssuerAndSerialRevocationState, nsIObserver, nsIPrefBranch, nsIRevocationState,
-    nsISubjectAndPubKeyRevocationState, nsISupports, nsIThread,
+    nsISerialEventTarget, nsISubjectAndPubKeyRevocationState, nsISupports,
 };
 use xpcom::{nsIID, GetterAddrefs, RefPtr, ThreadBoundRefPtr, XpCom};
 
@@ -1014,7 +1016,7 @@ fn do_construct_cert_storage(
 
     let cert_storage = CertStorage::allocate(InitCertStorage {
         security_state: Arc::new(RwLock::new(SecurityState::new(path_buf)?)),
-        thread: Mutex::new(create_thread("cert_storage")?),
+        queue: create_background_task_queue(cstr!("cert_storage"))?,
     });
 
     unsafe {
@@ -1192,7 +1194,7 @@ macro_rules! get_security_state {
 #[refcnt = "atomic"]
 struct InitCertStorage {
     security_state: Arc<RwLock<SecurityState>>,
-    thread: Mutex<RefPtr<nsIThread>>,
+    queue: RefPtr<nsISerialEventTarget>,
 }
 
 /// CertStorage implements the nsICertStorage interface. The actual work is done by the
@@ -1200,7 +1202,7 @@ struct InitCertStorage {
 /// the one and only SecurityState. So, only one thread can use SecurityState's &mut self functions
 /// at a time, while multiple threads can use &self functions simultaneously (as long as there are
 /// no threads using an &mut self function). The Arc is to allow for the creation of background
-/// tasks that use the SecurityState on the thread owned by CertStorage. This allows us to not block
+/// tasks that use the SecurityState on the queue owned by CertStorage. This allows us to not block
 /// the main thread.
 #[allow(non_snake_case)]
 impl CertStorage {
@@ -1251,9 +1253,8 @@ impl CertStorage {
             &self.security_state,
             move |ss| ss.get_has_prior_data(data_type),
         ));
-        let thread = try_ns!(self.thread.lock());
         let runnable = try_ns!(TaskRunnable::new("HasPriorData", task));
-        try_ns!(TaskRunnable::dispatch(runnable, &*thread));
+        try_ns!(TaskRunnable::dispatch(runnable, self.queue.coerce()));
         NS_OK
     }
 
@@ -1313,9 +1314,8 @@ impl CertStorage {
             &self.security_state,
             move |ss| ss.set_batch_state(&entries, nsICertStorage::DATA_TYPE_REVOCATION as u8),
         ));
-        let thread = try_ns!(self.thread.lock());
         let runnable = try_ns!(TaskRunnable::new("SetRevocations", task));
-        try_ns!(TaskRunnable::dispatch(runnable, &*thread));
+        try_ns!(TaskRunnable::dispatch(runnable, self.queue.coerce()));
         NS_OK
     }
 
@@ -1392,9 +1392,8 @@ impl CertStorage {
             &self.security_state,
             move |ss| ss.set_batch_state(&crlite_entries, nsICertStorage::DATA_TYPE_CRLITE as u8),
         ));
-        let thread = try_ns!(self.thread.lock());
         let runnable = try_ns!(TaskRunnable::new("SetCRLiteState", task));
-        try_ns!(TaskRunnable::dispatch(runnable, &*thread));
+        try_ns!(TaskRunnable::dispatch(runnable, self.queue.coerce()));
         NS_OK
     }
 
@@ -1438,9 +1437,8 @@ impl CertStorage {
             &self.security_state,
             move |ss| ss.set_full_crlite_filter(filter_owned, timestamp),
         ));
-        let thread = try_ns!(self.thread.lock());
         let runnable = try_ns!(TaskRunnable::new("SetFullCRLiteFilter", task));
-        try_ns!(TaskRunnable::dispatch(runnable, &*thread));
+        try_ns!(TaskRunnable::dispatch(runnable, self.queue.coerce()));
         NS_OK
     }
 
@@ -1461,9 +1459,8 @@ impl CertStorage {
             &self.security_state,
             move |ss| ss.add_crlite_stash(stash_owned),
         ));
-        let thread = try_ns!(self.thread.lock());
         let runnable = try_ns!(TaskRunnable::new("AddCRLiteStash", task));
-        try_ns!(TaskRunnable::dispatch(runnable, &*thread));
+        try_ns!(TaskRunnable::dispatch(runnable, self.queue.coerce()));
         NS_OK
     }
 
@@ -1544,9 +1541,8 @@ impl CertStorage {
             &self.security_state,
             move |ss| ss.add_certs(&cert_entries),
         ));
-        let thread = try_ns!(self.thread.lock());
         let runnable = try_ns!(TaskRunnable::new("AddCerts", task));
-        try_ns!(TaskRunnable::dispatch(runnable, &*thread));
+        try_ns!(TaskRunnable::dispatch(runnable, self.queue.coerce()));
         NS_OK
     }
 
@@ -1572,9 +1568,8 @@ impl CertStorage {
             &self.security_state,
             move |ss| ss.remove_certs_by_hashes(&hash_entries),
         ));
-        let thread = try_ns!(self.thread.lock());
         let runnable = try_ns!(TaskRunnable::new("RemoveCertsByHashes", task));
-        try_ns!(TaskRunnable::dispatch(runnable, &*thread));
+        try_ns!(TaskRunnable::dispatch(runnable, self.queue.coerce()));
         NS_OK
     }