Bug 1416174 - part 1 - OSFileConstants must be a singleton, r=smaug
authorAndrea Marchesini <amarchesini@mozilla.com>
Fri, 10 Nov 2017 19:27:03 +0100
changeset 444640 dc82839201a696be532b0309ceddd31b73960035
parent 444639 35bb5af0f317f65c257b69bff7bbfdb42ece6afe
child 444641 9b1d10a8644bd66cf35e7994b9772d6f89613d37
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1416174
milestone58.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1416174 - part 1 - OSFileConstants must be a singleton, r=smaug
dom/system/OSFileConstants.cpp
dom/system/OSFileConstants.h
dom/workers/RegisterBindings.cpp
dom/workers/RuntimeService.cpp
layout/build/nsLayoutModule.cpp
--- a/dom/system/OSFileConstants.cpp
+++ b/dom/system/OSFileConstants.cpp
@@ -64,16 +64,19 @@
 #include "nsString.h"
 #include "nsSystemInfo.h"
 #include "nsAutoPtr.h"
 #include "nsDirectoryServiceDefs.h"
 #include "nsXULAppAPI.h"
 #include "nsAppDirectoryServiceDefs.h"
 #include "mozJSComponentLoader.h"
 
+#include "mozilla/ClearOnShutdown.h"
+#include "mozilla/StaticPtr.h"
+
 #include "OSFileConstants.h"
 #include "nsIOSFileConstantsService.h"
 #include "nsZipArchive.h"
 
 #if defined(__DragonFly__) || defined(__FreeBSD__) \
   || defined(__NetBSD__) || defined(__OpenBSD__)
 #define __dd_fd dd_fd
 #endif
@@ -81,23 +84,19 @@
 /**
  * This module defines the basic libc constants (error numbers, open modes,
  * etc.) used by OS.File and possibly other OS-bound JavaScript libraries.
  */
 
 
 namespace mozilla {
 
-// Use an anonymous namespace to hide the symbols and avoid any collision
-// with, for instance, |extern bool gInitialized;|
 namespace {
-/**
- * |true| if this module has been initialized, |false| otherwise
- */
-bool gInitialized = false;
+
+StaticRefPtr<OSFileConstantsService> gInstance;
 
 struct Paths {
   /**
    * The name of the directory holding all the libraries (libxpcom, libnss, etc.)
    */
   nsString libDir;
   nsString tmpDir;
   nsString profileDir;
@@ -249,25 +248,24 @@ DelayedPathSetter::Observe(nsISupports*,
 
   return NS_OK;
 }
 
 /**
  * Perform the part of initialization that can only be
  * executed on the main thread.
  */
-nsresult InitOSFileConstants()
+nsresult
+OSFileConstantsService::InitOSFileConstants()
 {
   MOZ_ASSERT(NS_IsMainThread());
-  if (gInitialized) {
+  if (mInitialized) {
     return NS_OK;
   }
 
-  gInitialized = true;
-
   nsAutoPtr<Paths> paths(new Paths);
 
   // Initialize paths->libDir
   nsCOMPtr<nsIFile> file;
   nsresult rv = NS_GetSpecialDirectory(NS_XPCOM_LIBRARY_FILE, getter_AddRefs(file));
   if (NS_FAILED(rv)) {
     return rv;
   }
@@ -328,36 +326,21 @@ nsresult InitOSFileConstants()
 
   // Get the umask from the system-info service.
   // The property will always be present, but it will be zero on
   // non-Unix systems.
   // nsSystemInfo::gUserUmask is initialized by NS_InitXPCOM2 so we don't need
   // to initialize the service.
   gUserUmask = nsSystemInfo::gUserUmask;
 
+  mInitialized = true;
   return NS_OK;
 }
 
 /**
- * Perform the cleaning up that can only be executed on the main thread.
- */
-void CleanupOSFileConstants()
-{
-  MOZ_ASSERT(NS_IsMainThread());
-  if (!gInitialized) {
-    return;
-  }
-
-  gInitialized = false;
-  delete gPaths;
-  gPaths = nullptr;
-}
-
-
-/**
  * Define a simple read-only property holding an integer.
  *
  * @param name The name of the constant. Used both as the JS name for the
  * constant and to access its value. Must be defined.
  *
  * Produces a |ConstantSpec|.
  */
 #define INT_CONSTANT(name)      \
@@ -870,109 +853,111 @@ bool SetStringProperty(JSContext *cx, JS
 }
 
 /**
  * Define OS-specific constants.
  *
  * This function creates or uses JS object |OS.Constants| to store
  * all its constants.
  */
-bool DefineOSFileConstants(JSContext *cx, JS::Handle<JSObject*> global)
+bool
+OSFileConstantsService::DefineOSFileConstants(JSContext* aCx,
+                                              JS::Handle<JSObject*> aGlobal)
 {
-  if (!gInitialized || gPaths == nullptr) {
+  if (!mInitialized || gPaths == nullptr) {
     // If an initialization error was ignored, we may end up with
     // |gInitialized == true| but |gPaths == nullptr|. We cannot
     // |MOZ_ASSERT| this, as this would kill precompile_cache.js,
     // so we simply return an error.
-    JS_ReportErrorNumberASCII(cx, js::GetErrorMessage, nullptr,
+    JS_ReportErrorNumberASCII(aCx, js::GetErrorMessage, nullptr,
                               JSMSG_CANT_OPEN,
                               "OSFileConstants", "initialization has failed");
     return false;
   }
 
-  JS::Rooted<JSObject*> objOS(cx);
-  if (!(objOS = GetOrCreateObjectProperty(cx, global, "OS"))) {
+  JS::Rooted<JSObject*> objOS(aCx);
+  if (!(objOS = GetOrCreateObjectProperty(aCx, aGlobal, "OS"))) {
     return false;
   }
-  JS::Rooted<JSObject*> objConstants(cx);
-  if (!(objConstants = GetOrCreateObjectProperty(cx, objOS, "Constants"))) {
+  JS::Rooted<JSObject*> objConstants(aCx);
+  if (!(objConstants = GetOrCreateObjectProperty(aCx, objOS, "Constants"))) {
     return false;
   }
 
   // Build OS.Constants.libc
 
-  JS::Rooted<JSObject*> objLibc(cx);
-  if (!(objLibc = GetOrCreateObjectProperty(cx, objConstants, "libc"))) {
+  JS::Rooted<JSObject*> objLibc(aCx);
+  if (!(objLibc = GetOrCreateObjectProperty(aCx, objConstants, "libc"))) {
     return false;
   }
-  if (!dom::DefineConstants(cx, objLibc, gLibcProperties)) {
+  if (!dom::DefineConstants(aCx, objLibc, gLibcProperties)) {
     return false;
   }
 
 #if defined(XP_WIN)
   // Build OS.Constants.Win
 
-  JS::Rooted<JSObject*> objWin(cx);
-  if (!(objWin = GetOrCreateObjectProperty(cx, objConstants, "Win"))) {
+  JS::Rooted<JSObject*> objWin(aCx);
+  if (!(objWin = GetOrCreateObjectProperty(aCx, objConstants, "Win"))) {
     return false;
   }
-  if (!dom::DefineConstants(cx, objWin, gWinProperties)) {
+  if (!dom::DefineConstants(aCx, objWin, gWinProperties)) {
     return false;
   }
 #endif // defined(XP_WIN)
 
   // Build OS.Constants.Sys
 
-  JS::Rooted<JSObject*> objSys(cx);
-  if (!(objSys = GetOrCreateObjectProperty(cx, objConstants, "Sys"))) {
+  JS::Rooted<JSObject*> objSys(aCx);
+  if (!(objSys = GetOrCreateObjectProperty(aCx, objConstants, "Sys"))) {
     return false;
   }
 
   nsCOMPtr<nsIXULRuntime> runtime = do_GetService(XULRUNTIME_SERVICE_CONTRACTID);
   if (runtime) {
     nsAutoCString os;
     DebugOnly<nsresult> rv = runtime->GetOS(os);
     MOZ_ASSERT(NS_SUCCEEDED(rv));
 
-    JSString* strVersion = JS_NewStringCopyZ(cx, os.get());
+    JSString* strVersion = JS_NewStringCopyZ(aCx, os.get());
     if (!strVersion) {
       return false;
     }
 
-    JS::Rooted<JS::Value> valVersion(cx, JS::StringValue(strVersion));
-    if (!JS_SetProperty(cx, objSys, "Name", valVersion)) {
+    JS::Rooted<JS::Value> valVersion(aCx, JS::StringValue(strVersion));
+    if (!JS_SetProperty(aCx, objSys, "Name", valVersion)) {
       return false;
     }
   }
 
 #if defined(DEBUG)
-  JS::Rooted<JS::Value> valDebug(cx, JS::TrueValue());
-  if (!JS_SetProperty(cx, objSys, "DEBUG", valDebug)) {
+  JS::Rooted<JS::Value> valDebug(aCx, JS::TrueValue());
+  if (!JS_SetProperty(aCx, objSys, "DEBUG", valDebug)) {
     return false;
   }
 #endif
 
 #if defined(HAVE_64BIT_BUILD)
-  JS::Rooted<JS::Value> valBits(cx, JS::Int32Value(64));
+  JS::Rooted<JS::Value> valBits(aCx, JS::Int32Value(64));
 #else
-  JS::Rooted<JS::Value> valBits(cx, JS::Int32Value(32));
+  JS::Rooted<JS::Value> valBits(aCx, JS::Int32Value(32));
 #endif //defined (HAVE_64BIT_BUILD)
-  if (!JS_SetProperty(cx, objSys, "bits", valBits)) {
+  if (!JS_SetProperty(aCx, objSys, "bits", valBits)) {
     return false;
   }
 
-  if (!JS_DefineProperty(cx, objSys, "umask", gUserUmask,
+  if (!JS_DefineProperty(aCx, objSys, "umask", gUserUmask,
                          JSPROP_ENUMERATE | JSPROP_READONLY | JSPROP_PERMANENT)) {
       return false;
   }
 
   // Build OS.Constants.Path
 
-  JS::Rooted<JSObject*> objPath(cx);
-  if (!(objPath = GetOrCreateObjectProperty(cx, objConstants, "Path"))) {
+  JS::Rooted<JSObject*> objPath(aCx);
+  if (!(objPath = GetOrCreateObjectProperty(aCx, objConstants, "Path"))) {
     return false;
   }
 
   // Locate libxul
   // Note that we don't actually provide the full path, only the name of the
   // library, which is sufficient to link to the library using js-ctypes.
 
 #if defined(XP_MACOSX)
@@ -985,72 +970,72 @@ bool DefineOSFileConstants(JSContext *cx
   // On other platforms, libxul is a library "xul" with regular
   // library prefix/suffix.
   nsAutoString libxul;
   libxul.AppendLiteral(DLL_PREFIX);
   libxul.AppendLiteral("xul");
   libxul.AppendLiteral(DLL_SUFFIX);
 #endif // defined(XP_MACOSX)
 
-  if (!SetStringProperty(cx, objPath, "libxul", libxul)) {
+  if (!SetStringProperty(aCx, objPath, "libxul", libxul)) {
     return false;
   }
 
-  if (!SetStringProperty(cx, objPath, "libDir", gPaths->libDir)) {
+  if (!SetStringProperty(aCx, objPath, "libDir", gPaths->libDir)) {
     return false;
   }
 
-  if (!SetStringProperty(cx, objPath, "tmpDir", gPaths->tmpDir)) {
+  if (!SetStringProperty(aCx, objPath, "tmpDir", gPaths->tmpDir)) {
     return false;
   }
 
   // Configure profileDir only if it is available at this stage
   if (!gPaths->profileDir.IsVoid()
-    && !SetStringProperty(cx, objPath, "profileDir", gPaths->profileDir)) {
+    && !SetStringProperty(aCx, objPath, "profileDir", gPaths->profileDir)) {
     return false;
   }
 
   // Configure localProfileDir only if it is available at this stage
   if (!gPaths->localProfileDir.IsVoid()
-    && !SetStringProperty(cx, objPath, "localProfileDir", gPaths->localProfileDir)) {
+    && !SetStringProperty(aCx, objPath, "localProfileDir", gPaths->localProfileDir)) {
     return false;
   }
 
-  if (!SetStringProperty(cx, objPath, "homeDir", gPaths->homeDir)) {
+  if (!SetStringProperty(aCx, objPath, "homeDir", gPaths->homeDir)) {
     return false;
   }
 
-  if (!SetStringProperty(cx, objPath, "desktopDir", gPaths->desktopDir)) {
+  if (!SetStringProperty(aCx, objPath, "desktopDir", gPaths->desktopDir)) {
     return false;
   }
 
-  if (!SetStringProperty(cx, objPath, "userApplicationDataDir", gPaths->userApplicationDataDir)) {
+  if (!SetStringProperty(aCx, objPath, "userApplicationDataDir", gPaths->userApplicationDataDir)) {
     return false;
   }
 
 #if defined(XP_WIN)
-  if (!SetStringProperty(cx, objPath, "winAppDataDir", gPaths->winAppDataDir)) {
+  if (!SetStringProperty(aCx, objPath, "winAppDataDir", gPaths->winAppDataDir)) {
     return false;
   }
 
-  if (!SetStringProperty(cx, objPath, "winStartMenuProgsDir", gPaths->winStartMenuProgsDir)) {
+  if (!SetStringProperty(aCx, objPath, "winStartMenuProgsDir", gPaths->winStartMenuProgsDir)) {
     return false;
   }
 #endif // defined(XP_WIN)
 
 #if defined(XP_MACOSX)
-  if (!SetStringProperty(cx, objPath, "macUserLibDir", gPaths->macUserLibDir)) {
+  if (!SetStringProperty(aCx, objPath, "macUserLibDir", gPaths->macUserLibDir)) {
     return false;
   }
 
-  if (!SetStringProperty(cx, objPath, "macLocalApplicationsDir", gPaths->macLocalApplicationsDir)) {
+  if (!SetStringProperty(aCx, objPath, "macLocalApplicationsDir", gPaths->macLocalApplicationsDir)) {
     return false;
   }
 
-  if (!SetStringProperty(cx, objPath, "macTrashDir", gPaths->macTrashDir)) {
+  if (!SetStringProperty(aCx, objPath, "macTrashDir", gPaths->macTrashDir)) {
     return false;
   }
 #endif // defined(XP_MACOSX)
 
   // sqlite3 is linked from different places depending on the platform
   nsAutoString libsqlite3;
 #if defined(ANDROID)
   // On Android, we use the system's libsqlite3
@@ -1062,48 +1047,75 @@ bool DefineOSFileConstants(JSContext *cx
   libsqlite3.AppendLiteral(DLL_PREFIX);
   libsqlite3.AppendLiteral("nss3");
   libsqlite3.AppendLiteral(DLL_SUFFIX);
 #else
     // On other platforms, we link sqlite3 into libxul
   libsqlite3 = libxul;
 #endif // defined(ANDROID) || defined(XP_WIN)
 
-  if (!SetStringProperty(cx, objPath, "libsqlite3", libsqlite3)) {
+  if (!SetStringProperty(aCx, objPath, "libsqlite3", libsqlite3)) {
     return false;
   }
 
   return true;
 }
 
 NS_IMPL_ISUPPORTS(OSFileConstantsService, nsIOSFileConstantsService)
 
+/* static */ already_AddRefed<OSFileConstantsService>
+OSFileConstantsService::GetOrCreate()
+{
+  if (!gInstance) {
+    MOZ_ASSERT(NS_IsMainThread());
+
+    RefPtr<OSFileConstantsService> service = new OSFileConstantsService();
+    nsresult rv = service->InitOSFileConstants();
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return nullptr;
+    }
+
+    gInstance = service.forget();
+    ClearOnShutdown(&gInstance);
+  }
+
+  RefPtr<OSFileConstantsService> copy = gInstance;
+  return copy.forget();
+}
+
 OSFileConstantsService::OSFileConstantsService()
+  : mInitialized(false)
 {
   MOZ_ASSERT(NS_IsMainThread());
 }
 
 OSFileConstantsService::~OSFileConstantsService()
 {
-  mozilla::CleanupOSFileConstants();
+  MOZ_ASSERT(NS_IsMainThread());
+
+  if (mInitialized) {
+    delete gPaths;
+    gPaths = nullptr;
+  }
 }
 
-
 NS_IMETHODIMP
 OSFileConstantsService::Init(JSContext *aCx)
 {
-  nsresult rv = mozilla::InitOSFileConstants();
+  MOZ_ASSERT(NS_IsMainThread());
+
+  nsresult rv = InitOSFileConstants();
   if (NS_FAILED(rv)) {
     return rv;
   }
 
   mozJSComponentLoader* loader = mozJSComponentLoader::Get();
   JS::Rooted<JSObject*> targetObj(aCx);
   loader->FindTargetObject(aCx, &targetObj);
 
-  if (!mozilla::DefineOSFileConstants(aCx, targetObj)) {
+  if (!DefineOSFileConstants(aCx, targetObj)) {
     return NS_ERROR_FAILURE;
   }
 
   return NS_OK;
 }
 
 } // namespace mozilla
--- a/dom/system/OSFileConstants.h
+++ b/dom/system/OSFileConstants.h
@@ -8,53 +8,39 @@
 #define mozilla_osfileconstants_h__
 
 #include "nsIOSFileConstantsService.h"
 #include "mozilla/Attributes.h"
 
 namespace mozilla {
 
 /**
- * Perform initialization of this module.
- *
- * This function _must_ be called:
- * - from the main thread;
- * - before any Chrome Worker is created.
- *
- * The function is idempotent.
- */
-nsresult InitOSFileConstants();
-
-/**
- * Perform cleanup of this module.
- *
- * This function _must_ be called:
- * - from the main thread;
- * - after all Chrome Workers are dead.
- *
- * The function is idempotent.
- */
-void CleanupOSFileConstants();
-
-/**
- * Define OS-specific constants.
- *
- * This function creates or uses JS object |OS.Constants| to store
- * all its constants.
- */
-bool DefineOSFileConstants(JSContext *cx, JS::Handle<JSObject*> global);
-
-/**
  * XPConnect initializer, for use in the main thread.
+ * This class is thread-safe but it must be first be initialized on the
+ * main-thread.
  */
 class OSFileConstantsService final : public nsIOSFileConstantsService
 {
  public:
-  NS_DECL_ISUPPORTS
+  NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSIOSFILECONSTANTSSERVICE
+
+  static already_AddRefed<OSFileConstantsService>
+  GetOrCreate();
+
+  bool
+  DefineOSFileConstants(JSContext* aCx,
+                        JS::Handle<JSObject*> aGlobal);
+
+private:
+
+  nsresult
+  InitOSFileConstants();
+
   OSFileConstantsService();
-private:
   ~OSFileConstantsService();
+
+  bool mInitialized;
 };
 
 } // namespace mozilla
 
 #endif // mozilla_osfileconstants_h__
--- a/dom/workers/RegisterBindings.cpp
+++ b/dom/workers/RegisterBindings.cpp
@@ -20,18 +20,23 @@ bool
 WorkerPrivate::RegisterBindings(JSContext* aCx, JS::Handle<JSObject*> aGlobal)
 {
   // Init Web IDL bindings
   if (!RegisterWorkerBindings(aCx, aGlobal)) {
     return false;
   }
 
   if (IsChromeWorker()) {
-    if (!DefineChromeWorkerFunctions(aCx, aGlobal) ||
-        !DefineOSFileConstants(aCx, aGlobal)) {
+    if (!DefineChromeWorkerFunctions(aCx, aGlobal)) {
+      return false;
+    }
+
+    RefPtr<OSFileConstantsService> service =
+      OSFileConstantsService::GetOrCreate();
+    if (!service->DefineOSFileConstants(aCx, aGlobal)) {
       return false;
     }
   }
 
   if (!JS_DefineProfilingFunctions(aCx, aGlobal)) {
     return false;
   }
 
--- a/dom/workers/RuntimeService.cpp
+++ b/dom/workers/RuntimeService.cpp
@@ -2027,19 +2027,20 @@ RuntimeService::Init()
                                              MAX_WORKERS_PER_DOMAIN);
   gMaxWorkersPerDomain = std::max(0, maxPerDomain);
 
   int32_t maxHardwareConcurrency =
     Preferences::GetInt(PREF_WORKERS_MAX_HARDWARE_CONCURRENCY,
                         MAX_HARDWARE_CONCURRENCY);
   gMaxHardwareConcurrency = std::max(0, maxHardwareConcurrency);
 
-  rv = InitOSFileConstants();
-  if (NS_FAILED(rv)) {
-    return rv;
+  RefPtr<OSFileConstantsService> osFileConstantsService =
+    OSFileConstantsService::GetOrCreate();
+  if (NS_WARN_IF(!osFileConstantsService)) {
+    return NS_ERROR_FAILURE;
   }
 
   if (NS_WARN_IF(!IndexedDatabaseManager::GetOrCreate())) {
     return NS_ERROR_UNEXPECTED;
   }
 
   return NS_OK;
 }
@@ -2200,17 +2201,16 @@ RuntimeService::Cleanup()
         NS_WARNING("Failed to unregister for offline notification event!");
       }
       obs->RemoveObserver(this, NS_XPCOM_SHUTDOWN_THREADS_OBSERVER_ID);
       obs->RemoveObserver(this, NS_XPCOM_SHUTDOWN_OBSERVER_ID);
       mObserved = false;
     }
   }
 
-  CleanupOSFileConstants();
   nsLayoutStatics::Release();
 }
 
 void
 RuntimeService::AddAllTopLevelWorkersToArray(nsTArray<WorkerPrivate*>& aWorkers)
 {
   for (auto iter = mDomainMap.Iter(); !iter.Done(); iter.Next()) {
 
--- a/layout/build/nsLayoutModule.cpp
+++ b/layout/build/nsLayoutModule.cpp
@@ -527,17 +527,19 @@ NS_GENERIC_FACTORY_CONSTRUCTOR(CSPServic
 NS_GENERIC_FACTORY_CONSTRUCTOR(nsMixedContentBlocker)
 
 NS_GENERIC_FACTORY_CONSTRUCTOR(ContentPrincipal)
 NS_GENERIC_FACTORY_SINGLETON_CONSTRUCTOR(SystemPrincipal,
     nsScriptSecurityManager::SystemPrincipalSingletonConstructor)
 NS_GENERIC_FACTORY_CONSTRUCTOR_INIT(NullPrincipal, Init)
 NS_GENERIC_FACTORY_CONSTRUCTOR(nsStructuredCloneContainer)
 
-NS_GENERIC_FACTORY_CONSTRUCTOR(OSFileConstantsService)
+NS_GENERIC_FACTORY_SINGLETON_CONSTRUCTOR(OSFileConstantsService,
+                                         OSFileConstantsService::GetOrCreate);
+
 NS_GENERIC_FACTORY_CONSTRUCTOR(UDPSocketChild)
 
 NS_GENERIC_FACTORY_SINGLETON_CONSTRUCTOR(GeckoMediaPluginService, GeckoMediaPluginService::GetGeckoMediaPluginService)
 
 NS_GENERIC_FACTORY_CONSTRUCTOR(nsScriptError)
 
 #ifdef ACCESSIBILITY
 #include "xpcAccessibilityService.h"