Bug 1783299 - Add support for threadsafe mirrored StaticPref strings. r=KrisWright

Prior to this patch, one would need to manually instantiate a copy of a
string from a preference on the main thread in order to access it in a
threadsafe manner on another thread.

This patch adds support for a `DataMutexString` threadsafe type for
mirror: always type StaticPrefs, and works similarly to the existing
atomic types.

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

modules/libpref/Preferences.cpp

@@ -132,6 +132,18 @@ using ipc::FileDescriptor;
 
 #endif  // DEBUG
 
+// Forward declarations.
+namespace mozilla::StaticPrefs {
+
+static void InitAll();
+static void StartObservingAlwaysPrefs();
+static void InitOncePrefs();
+static void InitStaticPrefsFromShared();
+static void RegisterOncePrefs(SharedPrefMapBuilder& aBuilder);
+static void ShutdownAlwaysPrefs();
+
+}  // namespace mozilla::StaticPrefs
+
 //===========================================================================
 // Low-level types and operations
 //===========================================================================
@@ -1129,6 +1141,10 @@ class MOZ_STACK_CLASS PrefWrapper : public PrefWrapperBase {
     return NS_OK;
   }
 
+  nsresult GetValue(PrefValueKind aKind, nsACString* aResult) const {
+    return GetValue(aKind, *aResult);
+  }
+
   // Returns false if this pref doesn't have a user value worth saving.
   bool UserValueToStringForSaving(nsCString& aStr) {
     // Should we save the user value, if present? Only if it does not match the
@@ -3616,6 +3632,7 @@ void Preferences::Shutdown() {
   if (!sShutdown) {
     sShutdown = true;  // Don't create the singleton instance after here.
     sPreferences = nullptr;
+    StaticPrefs::ShutdownAlwaysPrefs();
   }
 }
 
@@ -3697,17 +3714,6 @@ void Preferences::DeserializePreferences(char* aStr, size_t aPrefsLen) {
   gContentProcessPrefsAreInited = true;
 }
 
-// Forward declarations.
-namespace StaticPrefs {
-
-static void InitAll();
-static void StartObservingAlwaysPrefs();
-static void InitOncePrefs();
-static void InitStaticPrefsFromShared();
-static void RegisterOncePrefs(SharedPrefMapBuilder& aBuilder);
-
-}  // namespace StaticPrefs
-
 /* static */
 FileDescriptor Preferences::EnsureSnapshot(size_t* aSize) {
   MOZ_ASSERT(XRE_IsParentProcess());
@@ -4529,6 +4535,9 @@ static nsCString PrefValueToString(const uint32_t* u) {
 static nsCString PrefValueToString(const float* f) {
   return nsPrintfCString("%f", *f);
 }
+static nsCString PrefValueToString(const nsACString* s) {
+  return nsCString(*s);
+}
 static nsCString PrefValueToString(const nsACString& s) { return nsCString(s); }
 
 // These preference getter wrappers allow us to look up the value for static
@@ -4637,13 +4646,36 @@ struct Internals {
     return result;
   }
 
+  template <typename T, typename V>
+  static void MOZ_NEVER_INLINE AssignMirror(T& aMirror, V aValue) {
+    aMirror = aValue;
+  }
+
+  static void MOZ_NEVER_INLINE AssignMirror(DataMutexString& aMirror,
+                                            nsCString&& aValue) {
+    auto lock = aMirror.Lock();
+    lock->Assign(std::move(aValue));
+  }
+
+  static void MOZ_NEVER_INLINE AssignMirror(DataMutexString& aMirror,
+                                            const nsLiteralCString& aValue) {
+    auto lock = aMirror.Lock();
+    lock->Assign(aValue);
+  }
+
+  static void ClearMirror(DataMutexString& aMirror) {
+    auto lock = aMirror.Lock();
+    lock->Assign(nsCString());
+  }
+
   template <typename T>
   static void UpdateMirror(const char* aPref, void* aMirror) {
     StripAtomic<T> value;
 
     nsresult rv = GetPrefValue(aPref, &value, PrefValueKind::User);
     if (NS_SUCCEEDED(rv)) {
-      *static_cast<T*>(aMirror) = value;
+      AssignMirror(*static_cast<T*>(aMirror),
+                   std::forward<StripAtomic<T>>(value));
     } else {
       // GetPrefValue() can fail if the update is caused by the pref being
       // deleted or if it fails to make a cast. This assertion is the only place
@@ -5433,6 +5465,16 @@ static MOZ_NEVER_INLINE void AddMirror(T* aMirror, const nsACString& aPref,
   AddMirrorCallback(aMirror, aPref);
 }
 
+static MOZ_NEVER_INLINE void AddMirror(DataMutexString& aMirror,
+                                       const nsACString& aPref) {
+  auto lock = aMirror.Lock();
+  nsCString result(*lock);
+  Internals::GetPrefValue(PromiseFlatCString(aPref).get(), result,
+                          PrefValueKind::User);
+  lock->Assign(std::move(result));
+  AddMirrorCallback(&aMirror, aPref);
+}
+
 // The InitPref_*() functions below end in a `_<type>` suffix because they are
 // used by the PREF macro definition in InitAll() below.
 
@@ -5511,6 +5553,15 @@ static void InitAlwaysPref(const nsCString& aName, T* aCache,
   *aCache = aDefaultValue;
 }
 
+static void InitAlwaysPref(const nsCString& aName, DataMutexString& aCache,
+                           const nsLiteralCString& aDefaultValue) {
+  // Only called in the parent process. Set/reset the pref value and the
+  // `always` mirror to the default value.
+  // `once` mirrors will be initialized lazily in InitOncePrefs().
+  InitPref_String(aName, aDefaultValue.get());
+  Internals::AssignMirror(aCache, aDefaultValue);
+}
+
 static Atomic<bool> sOncePrefRead(false);
 static StaticMutex sOncePrefMutex MOZ_UNANNOTATED;
 
@@ -5538,11 +5589,14 @@ void MaybeInitOncePrefs() {
 #define NEVER_PREF(name, cpp_type, value)
 #define ALWAYS_PREF(name, base_id, full_id, cpp_type, default_value) \
   cpp_type sMirror_##full_id(default_value);
+#define ALWAYS_DATAMUTEX_PREF(name, base_id, full_id, cpp_type, default_value) \
+  cpp_type sMirror_##full_id("DataMutexString");
 #define ONCE_PREF(name, base_id, full_id, cpp_type, default_value) \
   cpp_type sMirror_##full_id(default_value);
 #include "mozilla/StaticPrefListAll.h"
 #undef NEVER_PREF
 #undef ALWAYS_PREF
+#undef ALWAYS_DATAMUTEX_PREF
 #undef ONCE_PREF
 
 static void InitAll() {
@@ -5559,11 +5613,14 @@ static void InitAll() {
   InitPref_##cpp_type(name ""_ns, value);
 #define ALWAYS_PREF(name, base_id, full_id, cpp_type, value) \
   InitAlwaysPref(name ""_ns, &sMirror_##full_id, value);
+#define ALWAYS_DATAMUTEX_PREF(name, base_id, full_id, cpp_type, value) \
+  InitAlwaysPref(name ""_ns, sMirror_##full_id, value);
 #define ONCE_PREF(name, base_id, full_id, cpp_type, value) \
   InitPref_##cpp_type(name ""_ns, value);
 #include "mozilla/StaticPrefListAll.h"
 #undef NEVER_PREF
 #undef ALWAYS_PREF
+#undef ALWAYS_DATAMUTEX_PREF
 #undef ONCE_PREF
 }
 
@@ -5577,10 +5634,13 @@ static void StartObservingAlwaysPrefs() {
 #define NEVER_PREF(name, cpp_type, value)
 #define ALWAYS_PREF(name, base_id, full_id, cpp_type, value) \
   AddMirror(&sMirror_##full_id, name ""_ns, sMirror_##full_id);
+#define ALWAYS_DATAMUTEX_PREF(name, base_id, full_id, cpp_type, value) \
+  AddMirror(sMirror_##full_id, name ""_ns);
 #define ONCE_PREF(name, base_id, full_id, cpp_type, value)
 #include "mozilla/StaticPrefListAll.h"
 #undef NEVER_PREF
 #undef ALWAYS_PREF
+#undef ALWAYS_DATAMUTEX_PREF
 #undef ONCE_PREF
 }
 
@@ -5596,6 +5656,7 @@ static void InitOncePrefs() {
   // suggest that it should instead be `always`-mirrored.
 #define NEVER_PREF(name, cpp_type, value)
 #define ALWAYS_PREF(name, base_id, full_id, cpp_type, value)
+#define ALWAYS_DATAMUTEX_PREF(name, base_id, full_id, cpp_type, value)
 #ifdef DEBUG
 #  define ONCE_PREF(name, base_id, full_id, cpp_type, value)                   \
     {                                                                          \
@@ -5624,6 +5685,23 @@ static void InitOncePrefs() {
 #include "mozilla/StaticPrefListAll.h"
 #undef NEVER_PREF
 #undef ALWAYS_PREF
+#undef ALWAYS_DATAMUTEX_PREF
+#undef ONCE_PREF
+}
+
+static void ShutdownAlwaysPrefs() {
+  MOZ_ASSERT(NS_IsMainThread());
+
+  // We may need to do clean up for leak detection for some StaticPrefs.
+#define NEVER_PREF(name, cpp_type, value)
+#define ALWAYS_PREF(name, base_id, full_id, cpp_type, value)
+#define ALWAYS_DATAMUTEX_PREF(name, base_id, full_id, cpp_type, value) \
+  Internals::ClearMirror(sMirror_##full_id);
+#define ONCE_PREF(name, base_id, full_id, cpp_type, value)
+#include "mozilla/StaticPrefListAll.h"
+#undef NEVER_PREF
+#undef ALWAYS_PREF
+#undef ALWAYS_DATAMUTEX_PREF
 #undef ONCE_PREF
 }
 
@@ -5696,12 +5774,14 @@ static void RegisterOncePrefs(SharedPrefMapBuilder& aBuilder) {
   // "$$$" prefix and suffix to the preference name.
 #define NEVER_PREF(name, cpp_type, value)
 #define ALWAYS_PREF(name, base_id, full_id, cpp_type, value)
+#define ALWAYS_DATAMUTEX_PREF(name, base_id, full_id, cpp_type, value)
 #define ONCE_PREF(name, base_id, full_id, cpp_type, value)      \
   SaveOncePrefToSharedMap(aBuilder, ONCE_PREF_NAME(name) ""_ns, \
                           cpp_type(sMirror_##full_id));
 #include "mozilla/StaticPrefListAll.h"
 #undef NEVER_PREF
 #undef ALWAYS_PREF
+#undef ALWAYS_DATAMUTEX_PREF
 #undef ONCE_PREF
 }
 
@@ -5744,6 +5824,20 @@ static void InitStaticPrefsFromShared() {
     MOZ_ASSERT(NS_SUCCEEDED(rv), "Failed accessing " name);                    \
     StaticPrefs::sMirror_##full_id = val;                                      \
   }
+#define ALWAYS_DATAMUTEX_PREF(name, base_id, full_id, cpp_type, default_value) \
+  {                                                                            \
+    StripAtomic<cpp_type> val;                                                 \
+    if (!XRE_IsParentProcess() && IsString<cpp_type>::value &&                 \
+        gContentProcessPrefsAreInited && sCrashOnBlocklistedPref) {            \
+      MOZ_DIAGNOSTIC_ASSERT(                                                   \
+          !ShouldSanitizePreference(name, XRE_IsContentProcess()),             \
+          "Should not access the preference '" name "' in Content Processes"); \
+    }                                                                          \
+    DebugOnly<nsresult> rv = Internals::GetSharedPrefValue(name, &val);        \
+    MOZ_ASSERT(NS_SUCCEEDED(rv), "Failed accessing " name);                    \
+    Internals::AssignMirror(StaticPrefs::sMirror_##full_id,                    \
+                            std::forward<StripAtomic<cpp_type>>(val));         \
+  }
 #define ONCE_PREF(name, base_id, full_id, cpp_type, default_value)             \
   {                                                                            \
     cpp_type val;                                                              \
@@ -5761,6 +5855,7 @@ static void InitStaticPrefsFromShared() {
 #include "mozilla/StaticPrefListAll.h"
 #undef NEVER_PREF
 #undef ALWAYS_PREF
+#undef ALWAYS_DATAMUTEX_PREF
 #undef ONCE_PREF
 
   // `once`-mirrored prefs have been set to their value in the step above and

modules/libpref/StaticPrefsBase.h

@@ -10,6 +10,8 @@
 #include <type_traits>
 
 #include "mozilla/Atomics.h"
+#include "mozilla/DataMutex.h"
+#include "nsString.h"
 
 namespace mozilla {
 
@@ -19,12 +21,17 @@ class SharedPrefMapBuilder;
 
 typedef const char* String;
 
+using DataMutexString = StaticDataMutex<nsCString>;
+
 template <typename T>
 struct IsString : std::false_type {};
 
 template <>
 struct IsString<String> : std::true_type {};
 
+template <>
+struct IsString<DataMutexString> : std::true_type {};
+
 typedef Atomic<bool, Relaxed> RelaxedAtomicBool;
 typedef Atomic<bool, ReleaseAcquire> ReleaseAcquireAtomicBool;
 typedef Atomic<bool, SequentiallyConsistent> SequentiallyConsistentAtomicBool;
@@ -58,6 +65,11 @@ struct StripAtomicImpl<std::atomic<T>> {
   typedef T Type;
 };
 
+template <>
+struct StripAtomicImpl<DataMutexString> {
+  typedef nsCString Type;
+};
+
 template <typename T>
 using StripAtomic = typename StripAtomicImpl<T>::Type;
 

modules/libpref/init/StaticPrefList.yaml

@@ -42,10 +42,11 @@
 #   and JS code).
 #
 # - `type` is one of `bool`, `int32_t`, `uint32_t`, `float`, an atomic version
-#   of one of those, or `String`. Note that float prefs are stored internally
+#   of one of those, `String` or `DataMutexString`. Note that float prefs are
-#   as strings. The C++ preprocessor doesn't like template syntax in a macro
+#   stored internally as strings. The C++ preprocessor doesn't like template
-#   argument, so use the typedefs defined in StaticPrefsBase.h; for example,
+#   syntax in a macro argument, so use the typedefs defined in
-#   use `RelaxedAtomicBool` instead of `Atomic<bool, Relaxed>`.
+#   StaticPrefsBase.h; for example, use `RelaxedAtomicBool` instead of
+#   `Atomic<bool, Relaxed>`.
 #
 # - `value` is the default value. Its type should be appropriate for
 #   <cpp-type>, otherwise the generated code will fail to compile. A complex

modules/libpref/init/StaticPrefListBegin.h

@@ -37,6 +37,21 @@ namespace StaticPrefs {
   inline StripAtomic<cpp_type> GetPrefDefault_##base_id() {                    \
     return default_value;                                                      \
   }
+#define ALWAYS_DATAMUTEX_PREF(name, base_id, full_id, cpp_type, default_value) \
+  extern cpp_type sMirror_##full_id;                                           \
+  inline cpp_type::ConstAutoLock full_id() {                                   \
+    if (!XRE_IsParentProcess() && IsString<cpp_type>::value &&                 \
+        sCrashOnBlocklistedPref) {                                             \
+      MOZ_DIAGNOSTIC_ASSERT(                                                   \
+          !ShouldSanitizePreference(name, XRE_IsContentProcess()),             \
+          "Should not access the preference '" name "' in Content Processes"); \
+    }                                                                          \
+    return sMirror_##full_id.ConstLock();                                      \
+  }                                                                            \
+  inline const char* GetPrefName_##base_id() { return name; }                  \
+  inline StripAtomic<cpp_type> GetPrefDefault_##base_id() {                    \
+    return default_value;                                                      \
+  }
 #define ONCE_PREF(name, base_id, full_id, cpp_type, default_value)             \
   extern cpp_type sMirror_##full_id;                                           \
   inline cpp_type full_id() {                                                  \

modules/libpref/init/StaticPrefListEnd.h

@@ -13,6 +13,7 @@
 
 #undef NEVER_PREF
 #undef ALWAYS_PREF
+#undef ALWAYS_DATAMUTEX_PREF
 #undef ONCE_PREF
 
 }  // namespace StaticPrefs

modules/libpref/init/generate_static_pref_list.py

@@ -46,6 +46,7 @@ VALID_TYPES.update(
         "SequentiallyConsistentAtomicUint32": "uint32_t",
         "AtomicFloat": "float",
         "String": None,
+        "DataMutexString": None,
     }
 )
 
@@ -81,6 +82,14 @@ ALWAYS_PREF(
    {full_id},
   {typ}, {value}
 )
+""",
+    "always_datamutex": """\
+ALWAYS_DATAMUTEX_PREF(
+  "{name}",
+   {base_id},
+   {full_id},
+  {typ}, {value}
+)
 """,
 }
 
@@ -165,11 +174,11 @@ def check_pref_list(pref_list):
         if "value" not in pref:
             error("missing `value` key for pref `{}`".format(name))
         value = pref["value"]
-        if typ == "String":
+        if typ == "String" or typ == "DataMutexString":
             if type(value) != str:
                 error(
-                    "non-string `value` value `{}` for `String` pref `{}`; "
+                    "non-string `value` value `{}` for `{}` pref `{}`; "
-                    "add double quotes".format(value, name)
+                    "add double quotes".format(value, typ, name)
                 )
         elif typ in VALID_BOOL_TYPES:
             if value not in (True, False):
@@ -179,6 +188,8 @@ def check_pref_list(pref_list):
         if "mirror" not in pref:
             error("missing `mirror` key for pref `{}`".format(name))
         mirror = pref["mirror"]
+        if typ.startswith("DataMutex"):
+            mirror += "_datamutex"
         if mirror not in MIRROR_TEMPLATES:
             error("invalid `mirror` value `{}` for pref `{}`".format(mirror, name))
 
@@ -261,6 +272,8 @@ def generate_code(pref_list, input_filename):
             full_id += "_AtStartup"
         if do_not_use_directly:
             full_id += "_DoNotUseDirectly"
+        if typ.startswith("DataMutex"):
+            mirror += "_datamutex"
 
         group = mk_group(pref)
 
@@ -273,6 +286,9 @@ def generate_code(pref_list, input_filename):
         if typ == "String":
             # Quote string literals, and escape double-quote chars.
             value = '"{}"'.format(value.replace('"', '\\"'))
+        elif typ == "DataMutexString":
+            # Quote string literals, and escape double-quote chars.
+            value = '"{}"_ns'.format(value.replace('"', '\\"'))
         elif typ in VALID_BOOL_TYPES:
             # Convert Python bools to C++ bools.
             if value is True:

modules/libpref/test/test_generate_static_pref_list.py

@@ -69,6 +69,12 @@ good_input = """
   mirror: always
   rust: true
 
+# A comment.
+- name: my.datamutex.string
+  type: DataMutexString
+  value: "foobar"    # This string is quoted.
+  mirror: always
+
 # YAML+Python interprets `10 + 10 * 20` as a string, and so it is printed
 # unchanged.
 - name: my.atomic.int
@@ -152,6 +158,13 @@ ALWAYS_PREF(
   RelaxedAtomicBool, true
 )
 
+ALWAYS_DATAMUTEX_PREF(
+  "my.datamutex.string",
+   my_datamutex_string,
+   my_datamutex_string,
+  DataMutexString, "foobar"_ns
+)
+
 ALWAYS_PREF(
   "my.atomic.int",
    my_atomic_int,

xpcom/threads/DataMutex.h

@@ -38,32 +38,33 @@ namespace mozilla {
 template <typename T, typename MutexType>
 class DataMutexBase {
  public:
-  class MOZ_STACK_CLASS AutoLock {
+  template <typename V>
+  class MOZ_STACK_CLASS AutoLockBase {
    public:
-    T* operator->() const& { return &ref(); }
+    V* operator->() const& { return &ref(); }
-    T* operator->() const&& = delete;
+    V* operator->() const&& = delete;
 
-    T& operator*() const& { return ref(); }
+    V& operator*() const& { return ref(); }
-    T& operator*() const&& = delete;
+    V& operator*() const&& = delete;
 
     // Like RefPtr, make this act like its underlying raw pointer type
     // whenever it is used in a context where a raw pointer is expected.
-    operator T*() const& { return &ref(); }
+    operator V*() const& { return &ref(); }
 
     // Like RefPtr, don't allow implicit conversion of temporary to raw pointer.
-    operator T*() const&& = delete;
+    operator V*() const&& = delete;
 
-    T& ref() const& {
+    V& ref() const& {
       MOZ_ASSERT(mOwner);
       return mOwner->mValue;
     }
-    T& ref() const&& = delete;
+    V& ref() const&& = delete;
 
-    AutoLock(AutoLock&& aOther) : mOwner(aOther.mOwner) {
+    AutoLockBase(AutoLockBase&& aOther) : mOwner(aOther.mOwner) {
       aOther.mOwner = nullptr;
     }
 
-    ~AutoLock() {
+    ~AutoLockBase() {
       if (mOwner) {
         mOwner->mMutex.Unlock();
         mOwner = nullptr;
@@ -73,9 +74,9 @@ class DataMutexBase {
    private:
     friend class DataMutexBase;
 
-    AutoLock(const AutoLock& aOther) = delete;
+    AutoLockBase(const AutoLockBase& aOther) = delete;
 
-    explicit AutoLock(DataMutexBase<T, MutexType>* aDataMutex)
+    explicit AutoLockBase(DataMutexBase<T, MutexType>* aDataMutex)
         : mOwner(aDataMutex) {
       MOZ_ASSERT(!!mOwner);
       mOwner->mMutex.Lock();
@@ -84,12 +85,16 @@ class DataMutexBase {
     DataMutexBase<T, MutexType>* mOwner;
   };
 
+  using AutoLock = AutoLockBase<T>;
+  using ConstAutoLock = AutoLockBase<const T>;
+
   explicit DataMutexBase(const char* aName) : mMutex(aName) {}
 
   DataMutexBase(T&& aValue, const char* aName)
       : mMutex(aName), mValue(std::move(aValue)) {}
 
   AutoLock Lock() { return AutoLock(this); }
+  ConstAutoLock ConstLock() { return ConstAutoLock(this); }
 
   const MutexType& Mutex() const { return mMutex; }