From 13b09b8b4585aca3025e8243c63fcc0a6d366d31 Mon Sep 17 00:00:00 2001 From: Justin Lebar Date: Mon, 9 Apr 2012 13:53:08 -0400 Subject: [PATCH] Bug 741540 - Add AvailableMemoryTracker's hooks before any threads have started up. r=bsmedberg --HG-- extra : rebase_source : 5135df39b3ae022e75f8651b2e10a378edbae158 --- xpcom/base/AvailableMemoryTracker.cpp | 77 ++++++++++++++++----------- xpcom/base/AvailableMemoryTracker.h | 14 ++++- xpcom/build/nsXPComInit.cpp | 8 ++- 3 files changed, 64 insertions(+), 35 deletions(-) diff --git a/xpcom/base/AvailableMemoryTracker.cpp b/xpcom/base/AvailableMemoryTracker.cpp index 67b9a1baf97..8a33a9341a7 100644 --- a/xpcom/base/AvailableMemoryTracker.cpp +++ b/xpcom/base/AvailableMemoryTracker.cpp @@ -152,8 +152,11 @@ PRUint32 sNumLowPhysicalMemEvents = 0; WindowsDllInterceptor sKernel32Intercept; WindowsDllInterceptor sGdi32Intercept; -// Have we installed the kernel intercepts above? -bool sHooksInstalled = false; +// Has Init() been called? +bool sInitialized = false; + +// Has Activate() been called? The hooks don't do anything until this happens. +bool sHooksActive = false; // Alas, we'd like to use mozilla::TimeStamp, but we can't, because it acquires // a lock! @@ -208,6 +211,10 @@ bool MaybeScheduleMemoryPressureEvent() void CheckMemAvailable() { + if (!sHooksActive) { + return; + } + MEMORYSTATUSEX stat; stat.dwLength = sizeof(stat); bool success = GlobalMemoryStatusEx(&stat); @@ -263,7 +270,7 @@ VirtualAllocHook(LPVOID aAddress, SIZE_T aSize, // virtual memory. Similarly, don't call CheckMemAvailable for MEM_COMMIT if // we're not tracking low physical memory. if ((sLowVirtualMemoryThreshold != 0 && aAllocationType & MEM_RESERVE) || - (sLowPhysicalMemoryThreshold != 0 && aAllocationType & MEM_COMMIT)) { + (sLowPhysicalMemoryThreshold != 0 && aAllocationType & MEM_COMMIT)) { LOG3("VirtualAllocHook(size=", aSize, ")"); CheckMemAvailable(); } @@ -300,7 +307,7 @@ CreateDIBSectionHook(HDC aDC, // If aSection is non-null, CreateDIBSection won't allocate any new memory. bool doCheck = false; - if (!aSection && aBitmapInfo) { + if (sHooksActive && !aSection && aBitmapInfo) { PRUint16 bitCount = aBitmapInfo->bmiHeader.biBitCount; if (bitCount == 0) { // MSDN says bitCount == 0 means that it figures out how many bits each @@ -382,12 +389,11 @@ public: aDescription.AssignLiteral( "Number of low-virtual-memory events fired since startup. "); - if (sLowVirtualMemoryThreshold == 0 || !sHooksInstalled) { - aDescription.Append(nsPrintfCString(1024, + if (sLowVirtualMemoryThreshold == 0) { + aDescription.AppendLiteral( "Tracking low-virtual-memory events is disabled, but you can enable it " "by giving the memory.low_virtual_mem_threshold_mb pref a non-zero " - "value%s.", - sHooksInstalled ? "" : " and restarting")); + "value."); } else { aDescription.Append(nsPrintfCString(1024, @@ -425,12 +431,11 @@ public: aDescription.AssignLiteral( "Number of low-commit-space events fired since startup. "); - if (sLowCommitSpaceThreshold == 0 || !sHooksInstalled) { - aDescription.Append(nsPrintfCString(1024, + if (sLowCommitSpaceThreshold == 0) { + aDescription.Append( "Tracking low-commit-space events is disabled, but you can enable it " "by giving the memory.low_commit_space_threshold_mb pref a non-zero " - "value%s.", - sHooksInstalled ? "" : " and restarting")); + "value."); } else { aDescription.Append(nsPrintfCString(1024, @@ -468,12 +473,11 @@ public: aDescription.AssignLiteral( "Number of low-physical-memory events fired since startup. "); - if (sLowPhysicalMemoryThreshold == 0 || !sHooksInstalled) { - aDescription.Append(nsPrintfCString(1024, + if (sLowPhysicalMemoryThreshold == 0) { + aDescription.Append( "Tracking low-physical-memory events is disabled, but you can enable it " "by giving the memory.low_physical_memory_threshold_mb pref a non-zero " - "value%s.", - sHooksInstalled ? "" : " and restarting")); + "value."); } else { aDescription.Append(nsPrintfCString(1024, @@ -495,8 +499,11 @@ NS_IMPL_ISUPPORTS1(NumLowPhysicalMemoryEventsMemoryReporter, nsIMemoryReporter) namespace mozilla { namespace AvailableMemoryTracker { -void Init() +void Activate() { + MOZ_ASSERT(sInitialized); + MOZ_ASSERT(!sHooksActive); + // On 64-bit systems, hardcode sLowVirtualMemoryThreshold to 0 -- we assume // we're not going to run out of virtual memory! if (sizeof(void*) > 4) { @@ -514,14 +521,27 @@ void Init() Preferences::AddUintVarCache(&sLowMemoryNotificationIntervalMS, "memory.low_memory_notification_interval_ms", 10000); - // Don't register the hooks if we're a build instrumented for PGO or if both - // thresholds are 0. (If we're an instrumented build, the compiler adds - // function calls all over the place which may call VirtualAlloc; this makes - // it hard to prevent VirtualAllocHook from reentering itself.) + NS_RegisterMemoryReporter(new NumLowCommitSpaceEventsMemoryReporter()); + NS_RegisterMemoryReporter(new NumLowPhysicalMemoryEventsMemoryReporter()); + if (sizeof(void*) == 4) { + NS_RegisterMemoryReporter(new NumLowVirtualMemoryEventsMemoryReporter()); + } + sHooksActive = true; +} + +void Init() +{ + // Don't register the hooks if we're a build instrumented for PGO: If we're + // an instrumented build, the compiler adds function calls all over the place + // which may call VirtualAlloc; this makes it hard to prevent + // VirtualAllocHook from reentering itself. + + if (!PR_GetEnv("MOZ_PGO_INSTRUMENTED")) { + // Careful, this is not thread-safe! AddHook sets up the trampoline before + // writing to the out param, and anyway, it writes to the function + // non-atomically. So this must happen before any other threads which + // might call VirtualAlloc, MapViewOfFile, or CreateDIBSection start up. - if (!PR_GetEnv("MOZ_PGO_INSTRUMENTED") && - (sLowVirtualMemoryThreshold != 0 || sLowPhysicalMemoryThreshold != 0)) { - sHooksInstalled = true; sKernel32Intercept.Init("Kernel32.dll"); sKernel32Intercept.AddHook("VirtualAlloc", reinterpret_cast(VirtualAllocHook), @@ -535,15 +555,8 @@ void Init() reinterpret_cast(CreateDIBSectionHook), (void**) &sCreateDIBSectionOrig); } - else { - sHooksInstalled = false; - } - NS_RegisterMemoryReporter(new NumLowCommitSpaceEventsMemoryReporter()); - NS_RegisterMemoryReporter(new NumLowPhysicalMemoryEventsMemoryReporter()); - if (sizeof(void*) == 4) { - NS_RegisterMemoryReporter(new NumLowVirtualMemoryEventsMemoryReporter()); - } + sInitialized = true; } } // namespace AvailableMemoryTracker diff --git a/xpcom/base/AvailableMemoryTracker.h b/xpcom/base/AvailableMemoryTracker.h index 111cb725771..304f7f55c6a 100644 --- a/xpcom/base/AvailableMemoryTracker.h +++ b/xpcom/base/AvailableMemoryTracker.h @@ -44,13 +44,23 @@ namespace mozilla { namespace AvailableMemoryTracker { // The AvailableMemoryTracker is implemented only on Windows. But to make -// callers' lives easier, we stub out an empty Init() call. So you can always -// initialize the AvailableMemoryTracker; it just might not do anything. +// callers' lives easier, we stub out empty calls for all its public functions. +// So you can always initialize the AvailableMemoryTracker; it just might not +// do anything. +// +// Init() must be called before any other threads have started, because it +// modifies the in-memory implementations of some DLL functions in +// non-thread-safe ways. +// +// The hooks don't do anything until Activate() is called. It's an error to +// call Activate() without first calling Init(). #if defined(XP_WIN) void Init(); +void Activate(); #else void Init() {} +void Activate() {} #endif } // namespace AvailableMemoryTracker diff --git a/xpcom/build/nsXPComInit.cpp b/xpcom/build/nsXPComInit.cpp index 0cdc1c8b3a6..13375b815a7 100644 --- a/xpcom/build/nsXPComInit.cpp +++ b/xpcom/build/nsXPComInit.cpp @@ -350,6 +350,12 @@ NS_InitXPCOM2(nsIServiceManager* *result, // We are not shutting down gXPCOMShuttingDown = false; + NS_TIME_FUNCTION_MARK("Next: AvailableMemoryTracker Init()"); + + // Initialize the available memory tracker before other threads have had a + // chance to start up, because the initialization is not thread-safe. + mozilla::AvailableMemoryTracker::Init(); + NS_TIME_FUNCTION_MARK("Next: log init"); NS_LogInit(); @@ -514,7 +520,7 @@ NS_InitXPCOM2(nsIServiceManager* *result, nsDirectoryService::gService->RegisterCategoryProviders(); mozilla::scache::StartupCache::GetSingleton(); - mozilla::AvailableMemoryTracker::Init(); + mozilla::AvailableMemoryTracker::Activate(); NS_TIME_FUNCTION_MARK("Next: create services from category");