diff --git a/dom/network/UDPSocketParent.cpp b/dom/network/UDPSocketParent.cpp index 5e75d01d32..35c6f755d5 100644 --- a/dom/network/UDPSocketParent.cpp +++ b/dom/network/UDPSocketParent.cpp @@ -294,7 +294,7 @@ UDPSocketParent::DoConnect(nsCOMPtr& aSocket, const UDPAddressInfo& aAddressInfo) { UDPSOCKET_LOG(("%s: %s:%u", __FUNCTION__, aAddressInfo.addr().get(), aAddressInfo.port())); - if (NS_FAILED(ConnectInternal(aAddressInfo.addr(), aAddressInfo.port()))) { + if (NS_FAILED(ConnectInternal(aSocket, aAddressInfo.addr(), aAddressInfo.port()))) { SendInternalError(aReturnThread, __LINE__); return; } @@ -320,13 +320,15 @@ UDPSocketParent::DoConnect(nsCOMPtr& aSocket, } nsresult -UDPSocketParent::ConnectInternal(const nsCString& aHost, const uint16_t& aPort) +UDPSocketParent::ConnectInternal(const nsCOMPtr& aSocket, + const nsCString& aHost, + const uint16_t& aPort) { nsresult rv; UDPSOCKET_LOG(("%s: %s:%u", __FUNCTION__, nsCString(aHost).get(), aPort)); - if (!mSocket) { + if (!aSocket) { return NS_ERROR_NOT_AVAILABLE; } @@ -340,7 +342,7 @@ UDPSocketParent::ConnectInternal(const nsCString& aHost, const uint16_t& aPort) mozilla::net::NetAddr addr; PRNetAddrToNetAddr(&prAddr, &addr); - rv = mSocket->Connect(&addr); + rv = aSocket->Connect(&addr); if (NS_WARN_IF(NS_FAILED(rv))) { return rv; } diff --git a/dom/network/UDPSocketParent.h b/dom/network/UDPSocketParent.h index ddad9b9125..3a167a0fac 100644 --- a/dom/network/UDPSocketParent.h +++ b/dom/network/UDPSocketParent.h @@ -62,7 +62,9 @@ private: const bool& aAddressReuse, const bool& aLoopback, const uint32_t& recvBufferSize, const uint32_t& sendBufferSize); - nsresult ConnectInternal(const nsCString& aHost, const uint16_t& aPort); + nsresult ConnectInternal(const nsCOMPtr& aSocket, + const nsCString& aHost, + const uint16_t& aPort); void FireInternalError(uint32_t aLineNo); void SendInternalError(nsIEventTarget *aThread, uint32_t aLineNo); diff --git a/dom/webidl/CSSStyleSheet.webidl b/dom/webidl/CSSStyleSheet.webidl index 45ef840208..3cd4575934 100644 --- a/dom/webidl/CSSStyleSheet.webidl +++ b/dom/webidl/CSSStyleSheet.webidl @@ -13,6 +13,7 @@ enum CSSStyleSheetParsingMode { "agent" }; +[Constructor] interface CSSStyleSheet : StyleSheet { [Pure] readonly attribute CSSRule? ownerRule; diff --git a/js/src/gc/GCRuntime.h b/js/src/gc/GCRuntime.h index 89da938017..78dc06bb22 100644 --- a/js/src/gc/GCRuntime.h +++ b/js/src/gc/GCRuntime.h @@ -13,7 +13,6 @@ #include "jsgc.h" #include "gc/Heap.h" -#include "gc/IdleGC.h" #include "gc/Nursery.h" #include "gc/Statistics.h" #include "gc/StoreBuffer.h" @@ -651,13 +650,6 @@ class GCRuntime void onOutOfMallocMemory(); void onOutOfMallocMemory(const AutoLockGC& lock); - /* Idle-time GC notifications. */ - void notifyJSExecutionStart(); - void notifyJSExecutionEnd(); - IdleGCManager& idleGCMgr() { - return idleGC; - } - size_t maxMallocBytesAllocated() { return maxMallocBytes; } uint64_t nextCellUniqueId() { @@ -974,8 +966,6 @@ class GCRuntime void sweepZones(FreeOp* fop, bool lastGC); void decommitAllWithoutUnlocking(const AutoLockGC& lock); void startDecommit(); - bool sweepBackgroundFinalizePhaseInParallel(ZoneList& zones, const FinalizePhase& phase, - Arena** emptyArenas); void queueZonesForBackgroundSweep(ZoneList& zones); void sweepBackgroundThings(ZoneList& zones, LifoAlloc& freeBlocks); void assertBackgroundSweepingFinished(); @@ -989,7 +979,7 @@ class GCRuntime [[nodiscard]] bool relocateArenas(Zone* zone, JS::gcreason::Reason reason, Arena*& relocatedListOut, SliceBudget& sliceBudget); void updateTypeDescrObjects(MovingTracer* trc, Zone* zone); - void updateCellPointers(MovingTracer* trc, Zone* zone, AllocKinds kinds); + void updateCellPointers(MovingTracer* trc, Zone* zone, AllocKinds kinds, size_t bgTaskCount); void updateAllCellPointers(MovingTracer* trc, Zone* zone); void updatePointersToRelocatedCells(Zone* zone, AutoLockForExclusiveAccess& lock); void protectAndHoldArenas(Arena* arenaList); @@ -1029,9 +1019,6 @@ class GCRuntime GCSchedulingTunables tunables; GCSchedulingState schedulingState; - /* Idle-time garbage collection manager. */ - IdleGCManager idleGC; - MemProfiler mMemProfiler; private: diff --git a/js/src/gc/IdleGC.cpp b/js/src/gc/IdleGC.cpp deleted file mode 100644 index 2e64819609..0000000000 --- a/js/src/gc/IdleGC.cpp +++ /dev/null @@ -1,262 +0,0 @@ -/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*- - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -/** - * IDLE-TIME GARBAGE COLLECTION SYSTEM - * - * Overview - * -------- - * This system implements idle-time-only garbage collection, deferring GC work - * to periods when the JavaScript engine is not actively processing code. - * This improves perceived responsiveness by avoiding GC pauses during critical - * execution windows. - * - * Architecture - * ----------- - * - * The system consists of several key components: - * - * 1. IdleGCManager (gc/IdleGC.h, gc/IdleGC.cpp) - * - Tracks when the JS engine is executing vs. idle - * - Maintains timestamp of last execution activity - * - Checks if sufficient idle time has passed - * - Configurable idle threshold (default: 100ms) - * - Can determine which GC reasons should bypass idle checks - * - * 2. GCRuntime Integration (gc/GCRuntime.h) - * - Contains an IdleGCManager instance - * - Provides public methods: notifyJSExecutionStart/End() - * - Modified checkIfGCAllowedInCurrentState() to check idle status - * - * 3. Activity Tracking (vm/Runtime.cpp) - * - triggerActivityCallback() now notifies IdleGCManager - * - Called when JS enters/exits request (execution boundary) - * - Updated in jsapi.cpp's StartRequest/StopRequest functions - * - * 4. Public API (jsapi.h, jsapi.cpp) - * - JS_SetIdleGCEnabled() / JS_IsIdleGCEnabled() - * - JS_SetIdleGCThreshold() / JS_GetIdleGCThreshold() - * - JS_GetIdleTimeSinceLastExecution() - * - Allows embedders to configure idle GC behavior - * - * Behavior - * -------- - * - * Normal GC Trigger (Idle GC Enabled): - * - * 1. JS code executes → notifyJSExecutionStart() called - * 2. JS code finishes → notifyJSExecutionEnd() called, timestamp recorded - * 3. GC needed → checkIfGCAllowedInCurrentState() checks idle status - * 4a. If idle >= threshold → GC proceeds normally - * 4b. If still executing/idle < threshold → GC is deferred - * 5. Once idle threshold is met, next GC request proceeds - * - * Critical GC Triggers (Always Proceed): - * - * The following GC reasons bypass idle checking: - * - OUT_OF_MEMORY: Memory pressure conditions - * - ALLOC_TRIGGER: Allocation threshold exceeded - * - MALLOC_PRESSURE: Malloc pressure from OS - * - EAGER_ALLOC_TRIGGER: Eager allocation trigger - * - API: Explicit JS API calls - * - DETERMINISTIC: Deterministic tests - * - EVICT_NURSERY: Nursery eviction - * - SHUTDOWN_CC, DESTROY_RUNTIME, LAST_DITCH: Shutdown GCs - * - DESTROY_ZONE, COMPARTMENT_REVOKED: Zone/compartment destruction - * - * Configuration - * ------------- - * - * Idle GC Enabled (default: true): - * - Enables idle-time-only GC mode - * - Can be toggled dynamically via JS_SetIdleGCEnabled() - * - * Idle Threshold (default: 100ms): - * - Minimum idle time before GC is permitted - * - Configurable via JS_SetIdleGCThreshold(ms) - * - Typical values: 50-200ms depending on application - * - * Integration Examples - * -------------------- - * - * Browser Integration: - * - * // When browser loads configuration - * JS_SetIdleGCEnabled(cx, true); - * JS_SetIdleGCThreshold(cx, 100); // 100ms idle threshold - * - * // Monitor idle GC effectiveness (optional) - * uint64_t idleTime = JS_GetIdleTimeSinceLastExecution(cx); - * if (idleTime > JS_GetIdleGCThreshold(cx)) { - * // System is idle, GC would be allowed if triggered - * } - * - * Disabling for Specific Scenarios: - * - * // During initialization when nothing is "idle" yet - * JS_SetIdleGCEnabled(cx, false); - * // ... do initial setup ... - * JS_SetIdleGCEnabled(cx, true); // Re-enable for normal operation - * - * Performance Considerations - * -------------------------- - * - * Benefits: - * - Reduced jank during active JS execution - * - GC pauses moved to idle periods where users won't notice - * - Especially effective for interactive applications - * - Improves Time-to-Interactive and First Input Delay metrics - * - * Tradeoffs: - * - May accumulate more garbage before collection - * - Requires predictable idle periods (not suitable for all workloads) - * - Critical memory pressure GCs still proceed immediately - * - * Tuning: - * - Lower threshold (50ms) = more frequent GC, less memory overhead - * - Higher threshold (200ms) = less GC overhead, more memory usage - * - Optimal value depends on application's execution pattern - * - * Testing - * ------- - * - * Unit Tests: - * // Test idle detection - * JS_SetIdleGCThreshold(cx, 100); - * // Simulate JS execution - * cx->runtime()->gc.notifyJSExecutionStart(); - * // ... wait 50ms ... - * MOZ_ASSERT(!cx->runtime()->gc.idleGCMgr().isIdleEnough()); - * cx->runtime()->gc.notifyJSExecutionEnd(); - * // ... wait 150ms ... - * MOZ_ASSERT(cx->runtime()->gc.idleGCMgr().isIdleEnough()); - * - * Integration Tests: - * - Verify GC is deferred during active execution - * - Verify GC proceeds after idle period - * - Verify critical GC reasons bypass idle check - * - Measure latency improvements - * - * Implementation Notes - * -------------------- - * - * Thread Safety: - * - IdleGCManager uses mozilla::Atomic for thread-safe state - * - TimeStamp operations are atomic - * - No additional locking needed beyond existing GC locks - * - * Compatibility: - * - Works with both incremental and non-incremental GC - * - Compatible with generational GC - * - Works with zone GC and full GC - * - Respects existing GC suppression mechanisms - * - * Future Enhancements - * ------------------- - * - * Potential improvements: - * - Adaptive idle threshold based on historical GC times - * - Per-zone idle configuration - * - Integration with browser rendering idle callback API - * - Metrics/telemetry for idle GC effectiveness - * - Machine learning-based prediction of idle periods - * - Cooperative GC scheduling with other subsystems - * - */ - -#include "gc/IdleGC.h" -#include "jsapi.h" - -namespace js { -namespace gc { - -IdleGCManager::IdleGCManager() - : lastExecutionTime_(mozilla::TimeStamp::Now()), - idleGCEnabled_(true), - idleThresholdMs_(100), // 100ms default idle threshold - isExecuting_(false) -{ -} - -void -IdleGCManager::notifyJSExecutionStart() -{ - isExecuting_ = true; -} - -void -IdleGCManager::notifyJSExecutionEnd() -{ - isExecuting_ = false; - lastExecutionTime_ = mozilla::TimeStamp::Now(); -} - -bool -IdleGCManager::isIdleEnough() const -{ - if (!idleGCEnabled_) { - return true; // If disabled, always consider idle - } - - if (isExecuting_) { - return false; // Still executing, not idle - } - - uint64_t idleTime = idleTimeSinceLastExecution(); - return idleTime >= idleThresholdMs_; -} - -uint64_t -IdleGCManager::idleTimeSinceLastExecution() const -{ - mozilla::TimeStamp now = mozilla::TimeStamp::Now(); - mozilla::TimeDuration idle = now - lastExecutionTime_; - return idle.ToMilliseconds(); -} - -bool -IdleGCManager::shouldBypassIdleCheck(JS::gcreason::Reason reason) -{ - // These reasons indicate urgent GC needs that should bypass idle checking - switch (reason) { - // Allocation and memory pressure conditions. - case JS::gcreason::ALLOC_TRIGGER: - case JS::gcreason::EAGER_ALLOC_TRIGGER: - case JS::gcreason::TOO_MUCH_MALLOC: - case JS::gcreason::MEM_PRESSURE: - case JS::gcreason::LAST_DITCH: - - // Nursery/store-buffer pressure. - case JS::gcreason::OUT_OF_NURSERY: - case JS::gcreason::EVICT_NURSERY: - case JS::gcreason::FULL_STORE_BUFFER: - case JS::gcreason::SHARED_MEMORY_LIMIT: - - // Explicit API calls - case JS::gcreason::API: - case JS::gcreason::ABORT_GC: - - // Shutdown and finalization - case JS::gcreason::SHUTDOWN_CC: - case JS::gcreason::DESTROY_RUNTIME: - case JS::gcreason::NSJSCONTEXT_DESTROY: - case JS::gcreason::XPCONNECT_SHUTDOWN: - - return true; - - default: - return false; - } -} - -void -IdleGCManager::reset() -{ - lastExecutionTime_ = mozilla::TimeStamp::Now(); - isExecuting_ = false; -} - -} // namespace gc -} // namespace js diff --git a/js/src/gc/IdleGC.h b/js/src/gc/IdleGC.h deleted file mode 100644 index 8763699783..0000000000 --- a/js/src/gc/IdleGC.h +++ /dev/null @@ -1,118 +0,0 @@ -/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*- - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -#ifndef gc_IdleGC_h -#define gc_IdleGC_h - -#include -#include "mozilla/Atomics.h" -#include "mozilla/TimeStamp.h" -#include "js/GCAPI.h" - -namespace js { -namespace gc { - -/* - * Idle-Time Garbage Collection System - * ==================================== - * - * This system defers garbage collection to occur only during periods when - * the browser is not actively processing JavaScript. This helps maintain - * responsiveness by avoiding GC pauses during critical execution windows. - * - * When JavaScript execution is active, GC triggers are deferred. Once the - * JS engine has been idle for a configurable threshold period, pending GC - * work is performed immediately or incrementally as appropriate. - * - * Key characteristics: - * - Tracks JavaScript activity via hooks in the execution engine - * - Configurable idle time threshold (default: 100ms) - * - Can be disabled per-zone or globally - * - Works with both incremental and non-incremental GC modes - * - Respects critical GC reasons that override idle checking - */ - -class IdleGCManager -{ - public: - // Initialize the idle GC manager - IdleGCManager(); - - /* - * Called when JavaScript execution begins. Marks the engine as active. - */ - void notifyJSExecutionStart(); - - /* - * Called when JavaScript execution ends. Records the end time for - * idle detection purposes. - */ - void notifyJSExecutionEnd(); - - /* - * Check if the system has been idle for long enough to permit GC. - * Returns true if sufficient idle time has passed since last JS execution. - */ - bool isIdleEnough() const; - - /* - * Get the amount of idle time since the last JS execution. - * Returns time in milliseconds. - */ - uint64_t idleTimeSinceLastExecution() const; - - /* - * Set the idle threshold - minimum idle time before GC is permitted. - * Time is in milliseconds. Default is 100ms. - */ - void setIdleThresholdMs(uint64_t thresholdMs) { - idleThresholdMs_ = thresholdMs; - } - - uint64_t idleThresholdMs() const { - return idleThresholdMs_; - } - - /* - * Enable or disable idle-time-only GC mode. - */ - void setIdleGCEnabled(bool enabled) { - idleGCEnabled_ = enabled; - } - - bool isIdleGCEnabled() const { - return idleGCEnabled_; - } - - /* - * Check if a GC reason should bypass idle checking. - * Critical reasons (OOM-like pressure, nursery pressure, explicit requests) - * always proceed. - */ - static bool shouldBypassIdleCheck(JS::gcreason::Reason reason); - - /* - * Reset idle tracking state (used during GC or at shutdown). - */ - void reset(); - - private: - // Timestamp of the last JavaScript execution activity - mozilla::TimeStamp lastExecutionTime_; - - // Whether idle-time-only GC mode is enabled - mozilla::Atomic idleGCEnabled_; - - // Minimum idle time (in milliseconds) before GC is permitted - mozilla::Atomic idleThresholdMs_; - - // Whether the JS engine is currently executing - mozilla::Atomic isExecuting_; -}; - -} // namespace gc -} // namespace js - -#endif // gc_IdleGC_h diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index 93164c7c5e..910790621f 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -1487,36 +1487,6 @@ JS_SetGCParametersBasedOnAvailableMemory(JSContext* cx, uint32_t availMem) JS_SetGCParameter(cx, config[i].key, config[i].value); } -JS_PUBLIC_API(void) -JS_SetIdleGCEnabled(JSContext* cx, bool enabled) -{ - cx->gc.idleGCMgr().setIdleGCEnabled(enabled); -} - -JS_PUBLIC_API(bool) -JS_IsIdleGCEnabled(JSContext* cx) -{ - return cx->gc.idleGCMgr().isIdleGCEnabled(); -} - -JS_PUBLIC_API(void) -JS_SetIdleGCThreshold(JSContext* cx, uint64_t milliseconds) -{ - cx->gc.idleGCMgr().setIdleThresholdMs(milliseconds); -} - -JS_PUBLIC_API(uint64_t) -JS_GetIdleGCThreshold(JSContext* cx) -{ - return cx->gc.idleGCMgr().idleThresholdMs(); -} - -JS_PUBLIC_API(uint64_t) -JS_GetIdleTimeSinceLastExecution(JSContext* cx) -{ - return cx->gc.idleGCMgr().idleTimeSinceLastExecution(); -} - JS_PUBLIC_API(JSString*) JS_NewExternalString(JSContext* cx, const char16_t* chars, size_t length, diff --git a/js/src/jsapi.h b/js/src/jsapi.h index 0f033fef37..a090bf1d78 100644 --- a/js/src/jsapi.h +++ b/js/src/jsapi.h @@ -1724,40 +1724,6 @@ JS_GetGCParameter(JSContext* cx, JSGCParamKey key); extern JS_PUBLIC_API(void) JS_SetGCParametersBasedOnAvailableMemory(JSContext* cx, uint32_t availMem); -/* - * Idle-time garbage collection control. - * These functions allow control over when GC occurs - specifically, whether - * GC should only run when the browser/application is not doing active JS work. - */ - -/** - * Enable or disable idle-time-only GC mode. - * When enabled, GC is deferred until the JS engine has been idle for the - * configured threshold period (default: 100ms). - */ -extern JS_PUBLIC_API(void) -JS_SetIdleGCEnabled(JSContext* cx, bool enabled); - -extern JS_PUBLIC_API(bool) -JS_IsIdleGCEnabled(JSContext* cx); - -/** - * Set the minimum idle time (in milliseconds) before GC is permitted. - * Use this to configure how long the JS engine must be inactive before - * pending GC work can proceed. - */ -extern JS_PUBLIC_API(void) -JS_SetIdleGCThreshold(JSContext* cx, uint64_t milliseconds); - -extern JS_PUBLIC_API(uint64_t) -JS_GetIdleGCThreshold(JSContext* cx); - -/** - * Get the current idle time since the last JS execution. - */ -extern JS_PUBLIC_API(uint64_t) -JS_GetIdleTimeSinceLastExecution(JSContext* cx); - /** * Create a new JSString whose chars member refers to external memory, i.e., * memory requiring application-specific finalization. diff --git a/js/src/jsgc.cpp b/js/src/jsgc.cpp index dac887b108..c4a034050a 100644 --- a/js/src/jsgc.cpp +++ b/js/src/jsgc.cpp @@ -214,7 +214,6 @@ #include "gc/FindSCCs.h" #include "gc/GCInternals.h" #include "gc/GCTrace.h" -#include "gc/IdleGC.h" #include "gc/Marking.h" #include "gc/Memory.h" #include "gc/Policy.h" @@ -2118,7 +2117,7 @@ ArenasToUpdate::next(AutoLockHelperThreadState& lock) // Find the next arena to update. // // This iterates through the GC thing kinds filtered by shouldProcessKind(), - // and then through the arenas of that kind. All state is held in the + // and then through thea arenas of that kind. All state is held in the // object and we just return when we find an arena. for (; kind < AllocKind::LIMIT; kind = nextAllocKind(kind)) { @@ -2208,12 +2207,21 @@ UpdatePointersTask::run() } // namespace gc } // namespace js -static const size_t MinCellUpdateBackgroundTasks = 1; +static const size_t MinCellUpdateBackgroundTasks = 2; static const size_t MaxCellUpdateBackgroundTasks = 8; -static bool -CanUpdateKindInBackground(AllocKind kind) +static size_t +CellUpdateBackgroundTaskCount() { + if (!CanUseExtraThreads()) + return 0; + + size_t targetTaskCount = HelperThreadState().cpuCount / 2; + return Min(Max(targetTaskCount, MinCellUpdateBackgroundTasks), MaxCellUpdateBackgroundTasks); +} + +static bool +CanUpdateKindInBackground(AllocKind kind) { // We try to update as many GC things in parallel as we can, but there are // kinds for which this might not be safe: // - we assume JSObjects that are foreground finalized are not safe to @@ -2225,34 +2233,6 @@ CanUpdateKindInBackground(AllocKind kind) return true; } -static size_t -CountBackgroundUpdateArenas(Zone* zone, AllocKinds kinds) -{ - size_t arenaCount = 0; - for (AllocKind kind : kinds) { - MOZ_ASSERT(CanUpdateKindInBackground(kind)); - for (Arena* arena = zone->arenas.getFirstArena(kind); arena; arena = arena->next) - arenaCount++; - } - return arenaCount; -} - -static size_t -CellUpdateBackgroundTaskCount(Zone* zone, AllocKinds kinds) -{ - if (!CanUseExtraThreads() || kinds.isEmpty()) - return 0; - - size_t arenaCount = CountBackgroundUpdateArenas(zone, kinds); - if (arenaCount < UpdatePointersTask::MaxArenasToProcess * 2) - return 0; - - size_t targetTaskCount = HelperThreadState().cpuCount / 2; - size_t workTaskCount = arenaCount / UpdatePointersTask::MaxArenasToProcess; - targetTaskCount = Min(targetTaskCount, workTaskCount); - return Min(Max(targetTaskCount, MinCellUpdateBackgroundTasks), MaxCellUpdateBackgroundTasks); -} - static AllocKinds ForegroundUpdateKinds(AllocKinds kinds) { @@ -2273,15 +2253,10 @@ GCRuntime::updateTypeDescrObjects(MovingTracer* trc, Zone* zone) } void -GCRuntime::updateCellPointers(MovingTracer* trc, Zone* zone, AllocKinds kinds) +GCRuntime::updateCellPointers(MovingTracer* trc, Zone* zone, AllocKinds kinds, size_t bgTaskCount) { - MOZ_ASSERT(trc); - - AllocKinds fgKinds = ForegroundUpdateKinds(kinds); + AllocKinds fgKinds = bgTaskCount == 0 ? kinds : ForegroundUpdateKinds(kinds); AllocKinds bgKinds = kinds - fgKinds; - size_t bgTaskCount = CellUpdateBackgroundTaskCount(zone, bgKinds); - if (bgTaskCount == 0) - fgKinds = kinds; ArenasToUpdate fgArenas(zone, fgKinds); ArenasToUpdate bgArenas(zone, bgKinds); @@ -2376,13 +2351,15 @@ GCRuntime::updateAllCellPointers(MovingTracer* trc, Zone* zone) { AutoDisableProxyCheck noProxyCheck(rt); // These checks assert when run in parallel. - updateCellPointers(trc, zone, UpdatePhaseMisc); + size_t bgTaskCount = CellUpdateBackgroundTaskCount(); + + updateCellPointers(trc, zone, UpdatePhaseMisc, bgTaskCount); // Update TypeDescrs before all other objects as typed objects access these // objects when we trace them. updateTypeDescrObjects(trc, zone); - updateCellPointers(trc, zone, UpdatePhaseObjects); + updateCellPointers(trc, zone, UpdatePhaseObjects, bgTaskCount); } /* @@ -2688,17 +2665,6 @@ ArenaLists::backgroundFinalize(FreeOp* fop, Arena* listHead, Arena** empty) lists->backgroundFinalizeState[thingKind] = BFS_DONE; } -void -ArenaLists::backgroundFinalizePhase(FreeOp* fop, const FinalizePhase& phase, Arena** empty) -{ - for (auto kind : phase.kinds) { - Arena* arenas = arenaListsToSweep[kind]; - MOZ_RELEASE_ASSERT(uintptr_t(arenas) != uintptr_t(-1)); - if (arenas) - backgroundFinalize(fop, arenas, empty); - } -} - void ArenaLists::queueForegroundObjectsForSweep(FreeOp* fop) { @@ -3037,138 +3003,6 @@ js::gc::BackgroundDecommitTask::run() } } -class BackgroundFinalizeTask : public GCParallelTaskHelper -{ - Zone* zone_; - const FinalizePhase* phase_; - Arena* emptyArenas_; - - BackgroundFinalizeTask(const BackgroundFinalizeTask&) = delete; - - public: - BackgroundFinalizeTask(Zone* zone, const FinalizePhase* phase) - : zone_(zone), - phase_(phase), - emptyArenas_(nullptr) - {} - - BackgroundFinalizeTask(BackgroundFinalizeTask&& other) - : GCParallelTaskHelper(mozilla::Move(other)), - zone_(other.zone_), - phase_(other.phase_), - emptyArenas_(other.emptyArenas_) - { - other.emptyArenas_ = nullptr; - } - - void run() { - AutoSetThreadIsSweeping threadIsSweeping; - finalize(); - } - - void runAlreadySweeping() { -#ifdef DEBUG - MOZ_ASSERT(CurrentThreadIsGCSweeping()); -#endif - finalize(); - } - - private: - void finalize() { - FreeOp fop(nullptr); - zone_->arenas.backgroundFinalizePhase(&fop, *phase_, &emptyArenas_); - } - - public: - Arena* takeEmptyArenas() { - Arena* empty = emptyArenas_; - emptyArenas_ = nullptr; - return empty; - } -}; - -using BackgroundFinalizeTaskVector = - Vector; - -static size_t -IdleHelperThreadCount(const AutoLockHelperThreadState&) -{ - if (!HelperThreadState().threads) - return 0; - - size_t idle = 0; - for (const auto& thread : *HelperThreadState().threads) { - if (thread.idle()) - idle++; - } - return idle; -} - -static void -PrependArenaList(Arena** head, Arena* arenas) -{ - if (!arenas) - return; - - Arena* tail = arenas; - while (tail->next) - tail = tail->next; - tail->next = *head; - *head = arenas; -} - -bool -GCRuntime::sweepBackgroundFinalizePhaseInParallel(ZoneList& zones, const FinalizePhase& phase, - Arena** emptyArenas) -{ - if (!CanUseExtraThreads()) - return false; - - size_t zoneCount = 0; - for (Zone* zone = zones.front(); zone; zone = zone->nextZone()) - zoneCount++; - - if (zoneCount < 2) - return false; - - BackgroundFinalizeTaskVector tasks; - if (!tasks.reserve(zoneCount)) - return false; - - for (Zone* zone = zones.front(); zone; zone = zone->nextZone()) - tasks.infallibleEmplaceBack(zone, &phase); - - size_t tasksStarted = 0; - - { - AutoLockHelperThreadState helperLock; - - // sweepBackgroundThings() itself runs as a GC helper task. Do not queue - // nested GC parallel tasks unless at least one other helper is idle. - if (IdleHelperThreadCount(helperLock) == 0) - return false; - - for (; tasksStarted < tasks.length(); tasksStarted++) { - if (!tasks[tasksStarted].startWithLockHeld(helperLock)) - break; - } - - { - AutoUnlockHelperThreadState unlock(helperLock); - for (size_t i = tasksStarted; i < tasks.length(); i++) - tasks[i].runAlreadySweeping(); - } - - for (size_t i = 0; i < tasksStarted; i++) - tasks[i].joinWithLockHeld(helperLock); - } - - for (auto& task : tasks) - PrependArenaList(emptyArenas, task.takeEmptyArenas()); - - return true; -} - void GCRuntime::sweepBackgroundThings(ZoneList& zones, LifoAlloc& freeBlocks) { @@ -3181,12 +3015,14 @@ GCRuntime::sweepBackgroundThings(ZoneList& zones, LifoAlloc& freeBlocks) Arena* emptyArenas = nullptr; FreeOp fop(nullptr); for (unsigned phase = 0 ; phase < ArrayLength(BackgroundFinalizePhases) ; ++phase) { - const FinalizePhase& finalizePhase = BackgroundFinalizePhases[phase]; - if (sweepBackgroundFinalizePhaseInParallel(zones, finalizePhase, &emptyArenas)) - continue; - - for (Zone* zone = zones.front(); zone; zone = zone->nextZone()) - zone->arenas.backgroundFinalizePhase(&fop, finalizePhase, &emptyArenas); + for (Zone* zone = zones.front(); zone; zone = zone->nextZone()) { + for (auto kind : BackgroundFinalizePhases[phase].kinds) { + Arena* arenas = zone->arenas.arenaListsToSweep[kind]; + MOZ_RELEASE_ASSERT(uintptr_t(arenas) != uintptr_t(-1)); + if (arenas) + ArenaLists::backgroundFinalize(&fop, arenas, &emptyArenas); + } + } } AutoLockGC lock(rt); @@ -4593,9 +4429,7 @@ MAKE_GC_SWEEP_TASK(SweepBaseShapesTask); MAKE_GC_SWEEP_TASK(SweepInitialShapesTask); MAKE_GC_SWEEP_TASK(SweepObjectGroupsTask); MAKE_GC_SWEEP_TASK(SweepRegExpsTask); -MAKE_GC_SWEEP_TASK(SweepSavedStacksTask); -MAKE_GC_SWEEP_TASK(SweepSelfHostingScriptSourceTask); -MAKE_GC_SWEEP_TASK(SweepNativeIteratorsTask); +MAKE_GC_SWEEP_TASK(SweepMiscTask); #undef MAKE_GC_SWEEP_TASK /* virtual */ void @@ -4628,25 +4462,13 @@ SweepRegExpsTask::run() } /* virtual */ void -SweepSavedStacksTask::run() +SweepMiscTask::run() { for (GCCompartmentGroupIter c(runtime); !c.done(); c.next()) { c->sweepSavedStacks(); - } -} - -/* virtual */ void -SweepSelfHostingScriptSourceTask::run() -{ - for (GCCompartmentGroupIter c(runtime); !c.done(); c.next()) c->sweepSelfHostingScriptSource(); -} - -/* virtual */ void -SweepNativeIteratorsTask::run() -{ - for (GCCompartmentGroupIter c(runtime); !c.done(); c.next()) c->sweepNativeIterators(); + } } void @@ -4687,7 +4509,7 @@ PrepareWeakCacheTasks(JSRuntime* rt) WeakCacheTaskVector out; for (GCZoneGroupIter zone(rt); !zone.done(); zone.next()) { for (JS::WeakCache* cache : zone->weakCaches_) { - if (!out.emplaceBack(rt, *cache)) { + if (!out.append(SweepWeakCacheTask(rt, *cache))) { SweepWeakCachesFromMainThread(rt); return WeakCacheTaskVector(); } @@ -4729,9 +4551,7 @@ GCRuntime::beginSweepingZoneGroup(AutoLockForExclusiveAccess& lock) SweepCCWrappersTask sweepCCWrappersTask(rt); SweepObjectGroupsTask sweepObjectGroupsTask(rt); SweepRegExpsTask sweepRegExpsTask(rt); - SweepSavedStacksTask sweepSavedStacksTask(rt); - SweepSelfHostingScriptSourceTask sweepSelfHostingScriptSourceTask(rt); - SweepNativeIteratorsTask sweepNativeIteratorsTask(rt); + SweepMiscTask sweepMiscTask(rt); WeakCacheTaskVector sweepCacheTasks = PrepareWeakCacheTasks(rt); for (GCZoneGroupIter zone(rt); !zone.done(); zone.next()) { @@ -4779,9 +4599,7 @@ GCRuntime::beginSweepingZoneGroup(AutoLockForExclusiveAccess& lock) startTask(sweepCCWrappersTask, gcstats::PHASE_SWEEP_CC_WRAPPER, helperLock); startTask(sweepObjectGroupsTask, gcstats::PHASE_SWEEP_TYPE_OBJECT, helperLock); startTask(sweepRegExpsTask, gcstats::PHASE_SWEEP_REGEXP, helperLock); - startTask(sweepSavedStacksTask, gcstats::PHASE_SWEEP_MISC, helperLock); - startTask(sweepSelfHostingScriptSourceTask, gcstats::PHASE_SWEEP_MISC, helperLock); - startTask(sweepNativeIteratorsTask, gcstats::PHASE_SWEEP_MISC, helperLock); + startTask(sweepMiscTask, gcstats::PHASE_SWEEP_MISC, helperLock); for (auto& task : sweepCacheTasks) startTask(task, gcstats::PHASE_SWEEP_MISC, helperLock); } @@ -4859,9 +4677,7 @@ GCRuntime::beginSweepingZoneGroup(AutoLockForExclusiveAccess& lock) joinTask(sweepCCWrappersTask, gcstats::PHASE_SWEEP_CC_WRAPPER, helperLock); joinTask(sweepObjectGroupsTask, gcstats::PHASE_SWEEP_TYPE_OBJECT, helperLock); joinTask(sweepRegExpsTask, gcstats::PHASE_SWEEP_REGEXP, helperLock); - joinTask(sweepSavedStacksTask, gcstats::PHASE_SWEEP_MISC, helperLock); - joinTask(sweepSelfHostingScriptSourceTask, gcstats::PHASE_SWEEP_MISC, helperLock); - joinTask(sweepNativeIteratorsTask, gcstats::PHASE_SWEEP_MISC, helperLock); + joinTask(sweepMiscTask, gcstats::PHASE_SWEEP_MISC, helperLock); for (auto& task : sweepCacheTasks) joinTask(task, gcstats::PHASE_SWEEP_MISC, helperLock); } @@ -5290,7 +5106,6 @@ GCRuntime::compactPhase(JS::gcreason::Reason reason, SliceBudget& sliceBudget, gcstats::AutoPhase ap(stats, gcstats::PHASE_COMPACT); Arena* relocatedArenas = nullptr; - while (!zonesToMaybeCompact.isEmpty()) { // TODO: JSScripts can move. If the sampler interrupts the GC in the // middle of relocating an arena, invalid JSScript pointers may be @@ -6019,10 +5834,6 @@ GCRuntime::checkIfGCAllowedInCurrentState(JS::gcreason::Reason reason) if (rt->isBeingDestroyed() && !IsShutdownGC(reason)) return false; - // Check if the system is idle enough for GC, unless this is a critical GC - if (!IdleGCManager::shouldBypassIdleCheck(reason) && !idleGC.isIdleEnough()) - return false; - return true; } @@ -6189,18 +6000,6 @@ GCRuntime::notifyDidPaint() interFrameGC = false; } -void -GCRuntime::notifyJSExecutionStart() -{ - idleGC.notifyJSExecutionStart(); -} - -void -GCRuntime::notifyJSExecutionEnd() -{ - idleGC.notifyJSExecutionEnd(); -} - static bool ZonesSelected(JSRuntime* rt) { diff --git a/js/src/jsgc.h b/js/src/jsgc.h index f6d3208d9e..521dea05c6 100644 --- a/js/src/jsgc.h +++ b/js/src/jsgc.h @@ -804,7 +804,6 @@ class ArenaLists bool foregroundFinalize(FreeOp* fop, AllocKind thingKind, SliceBudget& sliceBudget, SortedArenaList& sweepList); - void backgroundFinalizePhase(FreeOp* fop, const FinalizePhase& phase, Arena** empty); static void backgroundFinalize(FreeOp* fop, Arena* listHead, Arena** empty); // When finalizing arenas, whether to keep empty arenas on the list or diff --git a/js/src/moz.build b/js/src/moz.build index 500d00caac..faedaeaecf 100644 --- a/js/src/moz.build +++ b/js/src/moz.build @@ -172,7 +172,6 @@ main_deunified_sources = [ 'gc/Allocator.cpp', 'gc/Barrier.cpp', 'gc/GCTrace.cpp', - 'gc/IdleGC.cpp', 'gc/Iteration.cpp', 'gc/Marking.cpp', 'gc/Memory.cpp', diff --git a/js/src/vm/CharacterEncoding.cpp b/js/src/vm/CharacterEncoding.cpp index b126e8a05a..81dd12ec4f 100644 --- a/js/src/vm/CharacterEncoding.cpp +++ b/js/src/vm/CharacterEncoding.cpp @@ -226,10 +226,11 @@ JS::Utf8ToOneUcs4Char(const uint8_t* utf8Buffer, int utf8Length) } static void -ReportInvalidCharacter(JSContext* cx, uint32_t offset) -{ - char buffer[10]; - SprintfLiteral(buffer, "%u", offset); +ReportInvalidCharacter(JSContext* cx, size_t offset) { + // Max roundtrip digits, +1 to include largest numbers, +1 for null terminator + constexpr size_t BUFFER_LENGTH = std::numeric_limits::digits10 + 2; + char buffer[BUFFER_LENGTH]; + SprintfLiteral(buffer, "%zu", offset); JS_ReportErrorFlagsAndNumberASCII(cx, JSREPORT_ERROR, GetErrorMessage, nullptr, JSMSG_MALFORMED_UTF8_CHAR, buffer); } @@ -295,7 +296,7 @@ InflateUTF8StringToBuffer(ContextT* cx, const UTF8Chars src, CharT* dst, size_t* // |i| is the index into |src|, and |j| is the the index into |dst|. size_t srclen = src.length(); uint32_t j = 0; - for (uint32_t i = 0; i < srclen; i++, j++) { + for (size_t i = 0; i < srclen; i++, j++) { uint32_t v = uint32_t(src[i]); if (!(v & 0x80)) { // ASCII code unit. Simple copy. diff --git a/js/src/vm/HelperThreads.cpp b/js/src/vm/HelperThreads.cpp index 9e51ae49ff..f8d032b671 100644 --- a/js/src/vm/HelperThreads.cpp +++ b/js/src/vm/HelperThreads.cpp @@ -969,7 +969,8 @@ GlobalHelperThreadState::maxGCParallelThreads() const bool GlobalHelperThreadState::canStartWasmCompile(const AutoLockHelperThreadState& lock) { - if (wasmWorklist(lock).empty()) + // Don't execute an wasm job if an earlier one failed. + if (wasmWorklist(lock).empty() || numWasmFailedJobs) return false; // Honor the maximum allowed threads to compile wasm jobs at once, @@ -1419,13 +1420,13 @@ HelperThread::handleWasmWorkload(AutoLockHelperThreadState& locked) success = wasm::CompileFunction(task); } - // Append the task to the finished queue owned by its module generator. - if (!success) - task->setFailed(); + // On success, try to move work to the finished list. + if (success) + success = HelperThreadState().wasmFinishedList(locked).append(task); - AutoEnterOOMUnsafeRegion oomUnsafe; - if (!task->finishedList()->append(task)) - oomUnsafe.crash("HelperThread::handleWasmWorkload"); + // On failure, note the failure for harvesting by the parent. + if (!success) + HelperThreadState().noteWasmFailure(locked); // Notify the main thread in case it's waiting. HelperThreadState().notifyAll(GlobalHelperThreadState::CONSUMER, locked); diff --git a/js/src/vm/HelperThreads.h b/js/src/vm/HelperThreads.h index 6763f57b5d..14cfa80396 100644 --- a/js/src/vm/HelperThreads.h +++ b/js/src/vm/HelperThreads.h @@ -89,8 +89,8 @@ class GlobalHelperThreadState wasm::IonCompileTaskPtrVector wasmWorklist_, wasmFinishedList_; public: - // Helper-thread initiated wasm compilations are serialized to avoid the - // deadlock scenario described in WasmGenerator.cpp. + // For now, only allow a single parallel wasm compilation to happen at a + // time. This avoids race conditions on wasmWorklist/wasmFinishedList/etc. mozilla::Atomic wasmCompilationInProgress; private: diff --git a/js/src/vm/Runtime.cpp b/js/src/vm/Runtime.cpp index 3ab5f50ccd..4c35aeb784 100644 --- a/js/src/vm/Runtime.cpp +++ b/js/src/vm/Runtime.cpp @@ -731,12 +731,6 @@ JSRuntime::traceSharedIntlData(JSTracer* trc) void JSRuntime::triggerActivityCallback(bool active) { - if (active) { - gc.notifyJSExecutionStart(); - } else { - gc.notifyJSExecutionEnd(); - } - if (!activityCallback) return; diff --git a/js/src/wasm/WasmGenerator.cpp b/js/src/wasm/WasmGenerator.cpp index 8bf10cc4ef..daff135077 100644 --- a/js/src/wasm/WasmGenerator.cpp +++ b/js/src/wasm/WasmGenerator.cpp @@ -54,7 +54,6 @@ ModuleGenerator::ModuleGenerator(ImportVector&& imports) lastPatchedCallsite_(0), startOfUnpatchedCallsites_(0), parallel_(false), - parallelCompilationInProgressOnHelperThread_(false), outstanding_(0), activeFuncDef_(nullptr), startedFuncDefs_(false), @@ -72,21 +71,18 @@ ModuleGenerator::~ModuleGenerator() AutoLockHelperThreadState lock; while (true) { IonCompileTaskPtrVector& worklist = HelperThreadState().wasmWorklist(lock); - for (size_t i = worklist.length(); i > 0;) { - if (worklist[i - 1]->finishedList() == &finishedTasks_) { - HelperThreadState().remove(worklist, &i); - MOZ_ASSERT(outstanding_ > 0); - outstanding_--; - } else { - i--; - } - } + MOZ_ASSERT(outstanding_ >= worklist.length()); + outstanding_ -= worklist.length(); + worklist.clear(); - for (size_t i = finishedTasks_.length(); i > 0;) { - HelperThreadState().remove(finishedTasks_, &i); - MOZ_ASSERT(outstanding_ > 0); - outstanding_--; - } + IonCompileTaskPtrVector& finished = HelperThreadState().wasmFinishedList(lock); + MOZ_ASSERT(outstanding_ >= finished.length()); + outstanding_ -= finished.length(); + finished.clear(); + + uint32_t numFailed = HelperThreadState().harvestFailedWasmJobs(lock); + MOZ_ASSERT(outstanding_ >= numFailed); + outstanding_ -= numFailed; if (!outstanding_) break; @@ -95,10 +91,8 @@ ModuleGenerator::~ModuleGenerator() } } - if (parallelCompilationInProgressOnHelperThread_) { - MOZ_ASSERT(HelperThreadState().wasmCompilationInProgress); - HelperThreadState().wasmCompilationInProgress = false; - } + MOZ_ASSERT(HelperThreadState().wasmCompilationInProgress); + HelperThreadState().wasmCompilationInProgress = false; } else { MOZ_ASSERT(!outstanding_); } @@ -214,9 +208,12 @@ ModuleGenerator::finishOutstandingTask() while (true) { MOZ_ASSERT(outstanding_ > 0); - if (!finishedTasks_.empty()) { + if (HelperThreadState().wasmFailed(lock)) + return false; + + if (!HelperThreadState().wasmFinishedList(lock).empty()) { outstanding_--; - task = finishedTasks_.popCopy(); + task = HelperThreadState().wasmFinishedList(lock).popCopy(); break; } @@ -224,9 +221,6 @@ ModuleGenerator::finishOutstandingTask() } } - if (task->failed()) - return false; - return finishTask(task); } @@ -371,9 +365,6 @@ ModuleGenerator::patchFarJumps(const TrapExitOffsetArray& trapExits) bool ModuleGenerator::finishTask(IonCompileTask* task) { - if (task->failed()) - return false; - const FuncBytes& func = task->func(); FuncCompileResults& results = task->results(); @@ -865,25 +856,31 @@ ModuleGenerator::startFuncDefs() MOZ_ASSERT(!startedFuncDefs_); MOZ_ASSERT(!finishedFuncDefs_); - // Helper-thread initiated wasm compilations stay serialized so that we do - // not end up with multiple helper threads blocking on other helper threads. - // Main-thread compilations can still overlap because they drain their own - // finished-task queue and do not steal tasks from other generators. + // The wasmCompilationInProgress atomic ensures that there is only one + // parallel compilation in progress at a time. In the special case of + // asm.js, where the ModuleGenerator itself can be on a helper thread, this + // avoids the possibility of deadlock since at most 1 helper thread will be + // blocking on other helper threads and there are always >1 helper threads. + // With wasm, this restriction could be relaxed by moving the worklist state + // out of HelperThreadState since each independent compilation needs its own + // worklist pair. Alternatively, the deadlock could be avoided by having the + // ModuleGenerator thread make progress (on compile tasks) instead of + // blocking. GlobalHelperThreadState& threads = HelperThreadState(); MOZ_ASSERT(threads.threadCount > 1); uint32_t numTasks; - if (CanUseExtraThreads() && (!CurrentHelperThread() || - threads.wasmCompilationInProgress.compareExchange(false, true))) { + if (CanUseExtraThreads() && threads.wasmCompilationInProgress.compareExchange(false, true)) { #ifdef DEBUG { AutoLockHelperThreadState lock; + MOZ_ASSERT(!HelperThreadState().wasmFailed(lock)); MOZ_ASSERT(HelperThreadState().wasmWorklist(lock).empty()); + MOZ_ASSERT(HelperThreadState().wasmFinishedList(lock).empty()); } #endif parallel_ = true; - parallelCompilationInProgressOnHelperThread_ = !!CurrentHelperThread(); numTasks = 2 * threads.maxWasmCompilationThreads(); } else { numTasks = 1; @@ -894,9 +891,6 @@ ModuleGenerator::startFuncDefs() for (size_t i = 0; i < numTasks; i++) tasks_.infallibleEmplaceBack(*shared_, COMPILATION_LIFO_DEFAULT_CHUNK_SIZE); - for (auto& task : tasks_) - task.setFinishedList(&finishedTasks_); - if (!freeTasks_.reserve(numTasks)) return false; for (size_t i = 0; i < numTasks; i++) diff --git a/js/src/wasm/WasmGenerator.h b/js/src/wasm/WasmGenerator.h index 520e296d7b..85b23664a2 100644 --- a/js/src/wasm/WasmGenerator.h +++ b/js/src/wasm/WasmGenerator.h @@ -104,11 +104,9 @@ class MOZ_STACK_CLASS ModuleGenerator // Parallel compilation bool parallel_; - bool parallelCompilationInProgressOnHelperThread_; uint32_t outstanding_; IonCompileTaskVector tasks_; IonCompileTaskPtrVector freeTasks_; - IonCompileTaskPtrVector finishedTasks_; // Assertions DebugOnly activeFuncDef_; diff --git a/js/src/wasm/WasmIonCompile.h b/js/src/wasm/WasmIonCompile.h index 6d3a9728ba..dcd5c3c633 100644 --- a/js/src/wasm/WasmIonCompile.h +++ b/js/src/wasm/WasmIonCompile.h @@ -19,7 +19,6 @@ #define wasm_ion_compile_h #include "jit/MacroAssembler.h" -#include "vm/HelperThreads.h" #include "wasm/WasmTypes.h" namespace js { @@ -107,8 +106,6 @@ class IonCompileTask UniqueFuncBytes func_; CompileMode mode_; Maybe results_; - IonCompileTaskPtrVector* finishedList_; - bool failed_; IonCompileTask(const IonCompileTask&) = delete; IonCompileTask& operator=(const IonCompileTask&) = delete; @@ -116,26 +113,7 @@ class IonCompileTask public: IonCompileTask(const ModuleGeneratorData& mg, size_t defaultChunkSize) : mg_(mg), lifo_(defaultChunkSize), func_(nullptr), mode_(CompileMode::None) - , finishedList_(nullptr) - , failed_(false) {} - - void setFinishedList(IonCompileTaskPtrVector* finishedList) { - finishedList_ = finishedList; - } - - IonCompileTaskPtrVector* finishedList() const { - MOZ_ASSERT(finishedList_); - return finishedList_; - } - - void setFailed() { - failed_ = true; - } - - bool failed() const { - return failed_; - } LifoAlloc& lifo() { return lifo_; } @@ -165,7 +143,6 @@ class IonCompileTask results_.reset(); lifo_.releaseAll(); mode_ = CompileMode::None; - failed_ = false; } }; diff --git a/layout/base/FrameProperties.h b/layout/base/FrameProperties.h index 1a9a987776..d4348c7ba0 100644 --- a/layout/base/FrameProperties.h +++ b/layout/base/FrameProperties.h @@ -310,6 +310,14 @@ private: static uint64_t ToInternalValue(PropertyType aValue) { +#if MOZ_BIG_ENDIAN + if (sizeof(PropertyType) <= sizeof(uint32_t)) { + // make sure to lose the unimportant half on 32bit architectures + uint32_t v = 0; + memcpy(&v, &aValue, sizeof(aValue)); + return v; + } +#endif uint64_t v = 0; memcpy(&v, &aValue, sizeof(aValue)); return v; @@ -318,6 +326,13 @@ private: static PropertyType FromInternalValue(uint64_t aInternalValue) { PropertyType value; +#if MOZ_BIG_ENDIAN + if (sizeof(value) <= sizeof(uint32_t)) { + uint32_t v32 = (uint32_t)aInternalValue; + memcpy(&value, &v32, sizeof(value)); + return value; + } +#endif memcpy(&value, &aInternalValue, sizeof(value)); return value; } diff --git a/layout/style/StyleSheet.cpp b/layout/style/StyleSheet.cpp index adcd0fc01b..3c0856d840 100644 --- a/layout/style/StyleSheet.cpp +++ b/layout/style/StyleSheet.cpp @@ -12,8 +12,10 @@ #include "mozilla/CSSStyleSheet.h" #include "mozAutoDocUpdate.h" +#include "nsContentUtils.h" #include "nsIMediaList.h" #include "nsNullPrincipal.h" +#include "nsPIDOMWindow.h" using namespace mozilla::dom; @@ -222,6 +224,40 @@ StyleSheet::DeleteRule(uint32_t aIndex) // WebIDL CSSStyleSheet API +/* static */ already_AddRefed +StyleSheet::Constructor(const GlobalObject& aGlobal, ErrorResult& aRv) +{ + nsCOMPtr window = + do_QueryInterface(aGlobal.GetAsSupports()); + if (!window) { + aRv.Throw(NS_ERROR_FAILURE); + return nullptr; + } + + nsCOMPtr document = window->GetDoc(); + if (!document) { + aRv.Throw(NS_ERROR_FAILURE); + return nullptr; + } + + nsCOMPtr documentURI = document->GetDocumentURI(); + nsCOMPtr baseURI = document->GetBaseURI(); + nsIPrincipal* principal = nsContentUtils::ObjectPrincipal(aGlobal.Get()); + if (!documentURI || !baseURI || !principal) { + aRv.Throw(NS_ERROR_FAILURE); + return nullptr; + } + + RefPtr sheet = + new CSSStyleSheet(css::eAuthorSheetFeatures, CORS_NONE, + document->GetReferrerPolicy()); + sheet->SetURIs(documentURI, nullptr, baseURI); + sheet->SetPrincipal(principal); + sheet->SetComplete(); + + return sheet.forget(); +} + dom::CSSRuleList* StyleSheet::GetCssRules(nsIPrincipal& aSubjectPrincipal, ErrorResult& aRv) diff --git a/layout/style/StyleSheet.h b/layout/style/StyleSheet.h index fd1e9b3652..99ed270127 100644 --- a/layout/style/StyleSheet.h +++ b/layout/style/StyleSheet.h @@ -149,6 +149,8 @@ public: // The XPCOM SetDisabled is fine for WebIDL. // WebIDL CSSStyleSheet API + static already_AddRefed Constructor(const dom::GlobalObject& aGlobal, + ErrorResult& aRv); virtual css::Rule* GetDOMOwnerRule() const = 0; dom::CSSRuleList* GetCssRules(nsIPrincipal& aSubjectPrincipal, ErrorResult& aRv); diff --git a/layout/style/test/mochitest.ini b/layout/style/test/mochitest.ini index 1e4900e62f..e0eaf7ae1c 100644 --- a/layout/style/test/mochitest.ini +++ b/layout/style/test/mochitest.ini @@ -167,6 +167,7 @@ prefs = layout.css.filters.enabled=true [test_condition_text.html] [test_condition_text_assignment.html] [test_contain_formatting_context.html] +[test_constructed_stylesheet.html] [test_counter_descriptor_storage.html] [test_counter_style.html] [test_css_cross_domain.html] diff --git a/layout/style/test/test_constructed_stylesheet.html b/layout/style/test/test_constructed_stylesheet.html new file mode 100644 index 0000000000..685faf3e48 --- /dev/null +++ b/layout/style/test/test_constructed_stylesheet.html @@ -0,0 +1,39 @@ + + + + + Test CSSStyleSheet constructor + + + + +

+
+
+
diff --git a/modules/libjar/nsJARChannel.cpp b/modules/libjar/nsJARChannel.cpp
index f6c7232367..90588d76e4 100644
--- a/modules/libjar/nsJARChannel.cpp
+++ b/modules/libjar/nsJARChannel.cpp
@@ -363,6 +363,11 @@ nsJARChannel::LookupFile(bool aAllowAsync)
     // have e.g. spaces in their filenames.
     NS_UnescapeURL(mJarEntry);
 
+    if (mJarEntry.FindChar('\0') != -1) {
+        // Refuse any entries with NULL in them.
+        return NS_ERROR_MALFORMED_URI;
+    }
+
     // try to get a nsIFile directly from the url, which will often succeed.
     {
         nsCOMPtr fileURL = do_QueryInterface(mJarBaseURI);
diff --git a/netwerk/base/TLSServerSocket.cpp b/netwerk/base/TLSServerSocket.cpp
index 3e21f8ff4b..7f867e2986 100644
--- a/netwerk/base/TLSServerSocket.cpp
+++ b/netwerk/base/TLSServerSocket.cpp
@@ -92,10 +92,11 @@ TLSServerSocket::CreateClientTransport(PRFileDesc* aClientFD,
   SSL_AuthCertificateHook(aClientFD, AuthCertificateHook, nullptr);
   // Once the TLS handshake has completed, the server consumer is notified and
   // has access to various TLS state details.
-  // It's safe to pass info here because the socket transport holds it as
-  // |mSecInfo| which keeps it alive for the lifetime of the socket.
+  trans->mFDDetachCallback = [aliveRef = RefPtr{info}](PRFileDesc* fd) {
+    SSL_HandshakeCallback(fd, nullptr, nullptr);
+  };
   SSL_HandshakeCallback(aClientFD, TLSServerConnectionInfo::HandshakeCallback,
-                        info);
+                        info.get());
 
   // Notify the consumer of the new client so it can manage the streams.
   // Security details aren't known yet.  The security observer will be notified
@@ -436,9 +437,11 @@ TLSServerConnectionInfo::GetMacLength(uint32_t* aMacLength)
 void
 TLSServerConnectionInfo::HandshakeCallback(PRFileDesc* aFD, void* aArg)
 {
+  // aArg is a raw pointer kept alive by the ref captured in
+  // the transport's mFDDetachCallback (set in CreateClientTransport).
   RefPtr info =
     static_cast(aArg);
-  nsISocketTransport* transport = info->mTransport;
+  RefPtr transport = info->mTransport;
   // No longer needed outside this function, so clear the weak ref
   info->mTransport = nullptr;
   nsresult rv = info->HandshakeCallback(aFD);
diff --git a/netwerk/base/nsRequestObserverProxy.cpp b/netwerk/base/nsRequestObserverProxy.cpp
index 4c3c718baf..25ed7c99f7 100644
--- a/netwerk/base/nsRequestObserverProxy.cpp
+++ b/netwerk/base/nsRequestObserverProxy.cpp
@@ -50,14 +50,15 @@ public:
     NS_IMETHOD Run() override
     {
         LOG(("nsOnStartRequestEvent::HandleEvent [req=%x]\n", mRequest.get()));
+        nsMainThreadPtrHandle observer = mProxy->mObserver;
 
-        if (!mProxy->mObserver) {
+        if (!observer) {
             NS_NOTREACHED("already handled onStopRequest event (observer is null)");
             return NS_OK;
         }
 
         LOG(("handle startevent=%p\n", this));
-        nsresult rv = mProxy->mObserver->OnStartRequest(mRequest, mProxy->mContext);
+        nsresult rv = observer->OnStartRequest(mRequest, mProxy->mContext);
         if (NS_FAILED(rv)) {
             LOG(("OnStartRequest failed [rv=%x] canceling request!\n", rv));
             rv = mRequest->Cancel(rv);
diff --git a/netwerk/base/nsSocketTransport2.cpp b/netwerk/base/nsSocketTransport2.cpp
index 157cb2add1..472aec724e 100644
--- a/netwerk/base/nsSocketTransport2.cpp
+++ b/netwerk/base/nsSocketTransport2.cpp
@@ -2093,6 +2093,10 @@ nsSocketTransport::OnSocketDetached(PRFileDesc *fd)
     {
         MutexAutoLock lock(mLock);
         if (mFD.IsInitialized()) {
+            auto callback = std::move(mFDDetachCallback);
+            if (callback) {
+                callback(mFD);
+            }
             ReleaseFD_Locked(mFD);
             // flag mFD as unusable; this prevents other consumers from 
             // acquiring a reference to mFD.
diff --git a/netwerk/base/nsSocketTransport2.h b/netwerk/base/nsSocketTransport2.h
index c36d46c04e..a358188803 100644
--- a/netwerk/base/nsSocketTransport2.h
+++ b/netwerk/base/nsSocketTransport2.h
@@ -9,6 +9,9 @@
 #define ENABLE_SOCKET_TRACING
 #endif
 
+#include  // for std::function
+
+#include "mozilla/Atomics.h"
 #include "mozilla/Mutex.h"
 #include "nsSocketTransportService2.h"
 #include "nsString.h"
@@ -294,7 +297,7 @@ private:
     bool mProxyTransparent;
     bool mProxyTransparentResolvesHost;
     bool mHttpsProxy;
-    uint32_t     mConnectionFlags;
+    Atomic mConnectionFlags;
     bool mReuseAddrPort;
 
     // The origin attributes are used to create sockets.  The first party domain
@@ -385,11 +388,17 @@ private:
     nsCOMPtr mEventSink;
     nsCOMPtr           mSecInfo;
 
+    // Called just before the fd is closed in OnSocketDetached. Used by
+    // TLSServerSocket to clear NSS handshake callbacks and release refs
+    // that would otherwise leak if the handshake never completed.
+    std::function mFDDetachCallback;
+
     nsSocketInputStream  mInput;
     nsSocketOutputStream mOutput;
 
     friend class nsSocketInputStream;
     friend class nsSocketOutputStream;
+    friend class TLSServerSocket;
 
     // socket timeouts are not protected by any lock.
     uint16_t mTimeouts[2];
diff --git a/security/nss/lib/certdb/alg1485.c b/security/nss/lib/certdb/alg1485.c
index 9a69c5bc51..153dca6823 100644
--- a/security/nss/lib/certdb/alg1485.c
+++ b/security/nss/lib/certdb/alg1485.c
@@ -836,6 +836,11 @@ get_hex_string(SECItem* data)
     static const char hex[] = { "0123456789ABCDEF" };
 
     /* '#' + 2 chars per octet + terminator */
+    /* Reject lengths that would overflow data->len * 2 + 2 in unsigned int. */
+    if (data->len > (UINT_MAX - 2) / 2) {
+        PORT_SetError(SEC_ERROR_INPUT_LEN);
+        return NULL;
+    }
     rv = SECITEM_AllocItem(NULL, NULL, data->len * 2 + 2);
     if (!rv) {
         return NULL;
diff --git a/security/nss/lib/freebl/aes-x86.c b/security/nss/lib/freebl/aes-x86.c
index 508fcc6502..1265251cf2 100644
--- a/security/nss/lib/freebl/aes-x86.c
+++ b/security/nss/lib/freebl/aes-x86.c
@@ -67,7 +67,7 @@ native_key_expansion192(AESContext *cx, const unsigned char *key)
     pre_align __m128i tmp3 post_align;
     pre_align __m128i carry post_align;
     keySchedule[0] = _mm_loadu_si128((__m128i *)key);
-    keySchedule[1] = _mm_loadu_si128((__m128i *)(key + 16));
+    keySchedule[1] = _mm_loadl_epi64((__m128i *)(key + 16));
     EXPAND_KEY192(keySchedule[0], keySchedule[1], keySchedule[2],
                   keySchedule[3], carry, 0x1, 0x2);
     EXPAND_KEY192_PART2(keySchedule[4], carry, keySchedule[3]);
diff --git a/security/nss/lib/pk11wrap/pk11pbe.c b/security/nss/lib/pk11wrap/pk11pbe.c
index 1e2fabd24b..a62774ecdf 100644
--- a/security/nss/lib/pk11wrap/pk11pbe.c
+++ b/security/nss/lib/pk11wrap/pk11pbe.c
@@ -1103,7 +1103,7 @@ SEC_PKCS5GetIV(SECAlgorithmID *algid, SECItem *pwitem, PRBool faulty3DES)
     CK_MECHANISM_TYPE type;
     SECItem *param = NULL;
     SECItem *iv = NULL;
-    SECItem src;
+    SECItem src = { siBuffer, NULL, 0 };
     int iv_len = 0;
     PK11SymKey *symKey;
     PK11SlotInfo *slot;
@@ -1143,7 +1143,7 @@ SEC_PKCS5GetIV(SECAlgorithmID *algid, SECItem *pwitem, PRBool faulty3DES)
     type = PK11_AlgtagToMechanism(pbeAlg);
     param = PK11_ParamFromAlgid(algid);
     if (param == NULL) {
-        goto done;
+        goto loser;
     }
     slot = PK11_GetInternalSlot();
     symKey = PK11_RawPBEKeyGen(slot, type, param, pwitem, faulty3DES, NULL);
diff --git a/security/nss/lib/pkcs7/p7decode.c b/security/nss/lib/pkcs7/p7decode.c
index 4687c239c5..3f7f1eb6e8 100644
--- a/security/nss/lib/pkcs7/p7decode.c
+++ b/security/nss/lib/pkcs7/p7decode.c
@@ -160,7 +160,9 @@ sec_pkcs7_decoder_work_data(SEC_PKCS7DecoderContext *p7dcx,
      */
     if (len) {
         for (i = 0; i < worker->digcnt; i++) {
-            (*worker->digobjs[i]->update)(worker->digcxs[i], data, len);
+            if (worker->digobjs[i]) {
+                (*worker->digobjs[i]->update)(worker->digcxs[i], data, len);
+            }
         }
     }
 
@@ -249,10 +251,10 @@ sec_pkcs7_decoder_start_digests(SEC_PKCS7DecoderContext *p7dcx, int depth,
     if (digcnt == 0)
         return SECSuccess;
 
-    p7dcx->worker.digcxs = (void **)PORT_ArenaAlloc(p7dcx->tmp_poolp,
-                                                    digcnt * sizeof(void *));
-    p7dcx->worker.digobjs = (const SECHashObject **)PORT_ArenaAlloc(p7dcx->tmp_poolp,
-                                                                    digcnt * sizeof(SECHashObject *));
+    p7dcx->worker.digcxs = (void **)PORT_ArenaZAlloc(p7dcx->tmp_poolp,
+                                                     digcnt * sizeof(void *));
+    p7dcx->worker.digobjs = (const SECHashObject **)PORT_ArenaZAlloc(p7dcx->tmp_poolp,
+                                                                     digcnt * sizeof(SECHashObject *));
     if (p7dcx->worker.digcxs == NULL || p7dcx->worker.digobjs == NULL) {
         p7dcx->error = SEC_ERROR_NO_MEMORY;
         return SECFailure;
@@ -261,8 +263,10 @@ sec_pkcs7_decoder_start_digests(SEC_PKCS7DecoderContext *p7dcx, int depth,
     p7dcx->worker.depth = depth;
 
     /*
-     * Create a digest context for each algorithm.
+     * Create a digest context for each algorithm.  Store at the original
+     * index so digcxs/digobjs stay aligned with digestAlgorithms.
      */
+    PRBool hasDigests = PR_FALSE;
     for (i = 0; i < digcnt; i++) {
         SECAlgorithmID *algid = digestalgs[i];
         SECOidTag oidTag = SECOID_FindOIDTag(&(algid->algorithm));
@@ -284,13 +288,14 @@ sec_pkcs7_decoder_start_digests(SEC_PKCS7DecoderContext *p7dcx, int depth,
         digcx = (*digobj->create)();
         if (digcx != NULL) {
             (*digobj->begin)(digcx);
-            p7dcx->worker.digobjs[p7dcx->worker.digcnt] = digobj;
-            p7dcx->worker.digcxs[p7dcx->worker.digcnt] = digcx;
-            p7dcx->worker.digcnt++;
+            p7dcx->worker.digobjs[i] = digobj;
+            p7dcx->worker.digcxs[i] = digcx;
+            hasDigests = PR_TRUE;
         }
     }
+    p7dcx->worker.digcnt = digcnt;
 
-    if (p7dcx->worker.digcnt != 0)
+    if (hasDigests)
         SEC_ASN1DecoderSetFilterProc(p7dcx->dcx,
                                      sec_pkcs7_decoder_filter,
                                      p7dcx,
@@ -358,6 +363,9 @@ sec_pkcs7_decoder_finish_digests(SEC_PKCS7DecoderContext *p7dcx,
 
     for (int i = 0; i < worker->digcnt; i++) {
         const SECHashObject *digobj = worker->digobjs[i];
+        if (!digobj) {
+            continue;
+        }
         digests[i] = SECITEM_AllocItem(poolp, NULL, digobj->length);
         if (!digests[i]) {
             p7dcx->error = PORT_GetError();
@@ -370,6 +378,9 @@ sec_pkcs7_decoder_finish_digests(SEC_PKCS7DecoderContext *p7dcx,
         void *digcx = worker->digcxs[i];
         const SECHashObject *digobj = worker->digobjs[i];
 
+        if (!digobj) {
+            continue;
+        }
         (*digobj->end)(digcx, digests[i]->data, &(digests[i]->len), digests[i]->len);
         (*digobj->destroy)(digcx, PR_TRUE);
     }
@@ -1473,7 +1484,7 @@ sec_pkcs7_verify_signature(SEC_PKCS7ContentInfo *cinfo,
      * stashed in the struct itself or passed in explicitly (as would
      * be done for detached contents).
      */
-    if ((digests == NULL || digests[0] == NULL) && (detached_digest == NULL || detached_digest->data == NULL))
+    if (digests == NULL && (detached_digest == NULL || detached_digest->data == NULL))
         goto done;
 
     /*
@@ -1517,6 +1528,10 @@ sec_pkcs7_verify_signature(SEC_PKCS7ContentInfo *cinfo,
         }
 
         digest = digests[i];
+        if (digest == NULL) {
+            PORT_SetError(SEC_ERROR_PKCS7_BAD_SIGNATURE);
+            goto done;
+        }
     }
 
     encTag = SECOID_FindOIDTag(&(signerinfo->digestEncAlg.algorithm));
diff --git a/security/nss/lib/pkcs7/p7encode.c b/security/nss/lib/pkcs7/p7encode.c
index af3da59187..52d064a198 100644
--- a/security/nss/lib/pkcs7/p7encode.c
+++ b/security/nss/lib/pkcs7/p7encode.c
@@ -714,11 +714,9 @@ sec_pkcs7_encoder_sig_and_certs(SEC_PKCS7ContentInfo *cinfo,
                 if (digestalgtag == SECOID_GetAlgorithmTag(digestalgs[di]))
                     break;
             }
-            if (digestalgs[di] == NULL) {
-                /* XXX oops; do what? set an error? */
+            if (digestalgs[di] == NULL || digests[di] == NULL) {
                 return SECFailure;
             }
-            PORT_Assert(digests[di] != NULL);
 
             cert = signerinfo->cert;
             privkey = PK11_FindKeyByAnyCert(cert, pwfnarg);
diff --git a/security/nss/lib/pki/trustdomain.c b/security/nss/lib/pki/trustdomain.c
index c8f9f1d5b9..f0db5468f9 100644
--- a/security/nss/lib/pki/trustdomain.c
+++ b/security/nss/lib/pki/trustdomain.c
@@ -62,14 +62,15 @@ static void
 token_destructor(void *t)
 {
     NSSToken *tok = (NSSToken *)t;
-    /* Remove the token list's reference to the token */
-    (void)nssToken_Destroy(tok);
 
     /* Signal that the slot should not give out any more references to the
-     * token. The token might still have a positive refcount after this call.
-     * The token has a reference to the slot, so the slot will not be destroyed
-     * until after the token's refcount drops to 0. */
+     * token. Do this first, while |tok| (and its reference to the slot) is
+     * still alive: the list may hold the last reference, in which case
+     * nssToken_Destroy() below frees the arena that contains |tok|. */
     PK11Slot_SetNSSToken(tok->pk11slot, NULL);
+
+    /* Remove the token list's reference to the token */
+    (void)nssToken_Destroy(tok);
 }
 
 NSS_IMPLEMENT PRStatus
diff --git a/security/nss/lib/smime/cmscinfo.c b/security/nss/lib/smime/cmscinfo.c
index 453ccaadaa..37163b27ee 100644
--- a/security/nss/lib/smime/cmscinfo.c
+++ b/security/nss/lib/smime/cmscinfo.c
@@ -156,37 +156,40 @@ SECStatus
 NSS_CMSContentInfo_SetContent(NSSCMSMessage *cmsg, NSSCMSContentInfo *cinfo,
                               SECOidTag type, void *ptr)
 {
-    SECStatus rv;
     if (cinfo == NULL || cmsg == NULL) {
+        PORT_SetError(SEC_ERROR_INVALID_ARGS);
         return SECFailure;
     }
 
-    cinfo->contentTypeTag = SECOID_FindOIDByTag(type);
-    if (cinfo->contentTypeTag == NULL) {
+    SECOidData *contentTypeTag = SECOID_FindOIDByTag(type);
+    if (!contentTypeTag) {
         return SECFailure;
     }
 
-    /* do not copy the oid, just create a reference */
-    rv = SECITEM_CopyItem(cmsg->poolp, &(cinfo->contentType), &(cinfo->contentTypeTag->oid));
+    SECItem contentType = { siBuffer, NULL, 0 };
+    SECStatus rv = SECITEM_CopyItem(cmsg->poolp, &contentType, &(contentTypeTag->oid));
     if (rv != SECSuccess) {
         return SECFailure;
     }
 
-    cinfo->content.pointer = ptr;
-
+    SECItem *rawContent;
     if (NSS_CMSType_IsData(type) && ptr) {
-        cinfo->rawContent = ptr;
+        rawContent = ptr;
     } else {
         /* as we always have some inner data,
-     * we need to set it to something, just to fool the encoder enough to work on it
-     * and get us into nss_cms_encoder_notify at that point */
-        cinfo->rawContent = SECITEM_AllocItem(cmsg->poolp, NULL, 1);
-        if (cinfo->rawContent == NULL) {
-            PORT_SetError(SEC_ERROR_NO_MEMORY);
+         * we need to set it to something, just to fool the encoder enough to work on it
+         * and get us into nss_cms_encoder_notify at that point */
+        rawContent = SECITEM_AllocItem(cmsg->poolp, NULL, 1);
+        if (!rawContent) {
             return SECFailure;
         }
     }
 
+    cinfo->contentType = contentType;
+    cinfo->content.pointer = ptr;
+    cinfo->contentTypeTag = contentTypeTag;
+    cinfo->rawContent = rawContent;
+
     return SECSuccess;
 }
 
diff --git a/security/nss/lib/softoken/pkcs11c.c b/security/nss/lib/softoken/pkcs11c.c
index be596fc025..464bc43c3b 100644
--- a/security/nss/lib/softoken/pkcs11c.c
+++ b/security/nss/lib/softoken/pkcs11c.c
@@ -5750,8 +5750,10 @@ NSC_WrapKey(CK_SESSION_HANDLE hSession,
 
             /* Find out if this is a block cipher. */
             crv = sftk_GetContext(hSession, &context, SFTK_ENCRYPT, PR_FALSE, NULL);
-            if (crv != CKR_OK || !context)
+            if (crv != CKR_OK || !context) {
+                sftk_FreeAttribute(attribute);
                 break;
+            }
             if (context->blockSize > 1) {
                 unsigned int remainder = pText.len % context->blockSize;
                 if (!context->doPad && remainder) {
@@ -5765,6 +5767,7 @@ NSC_WrapKey(CK_SESSION_HANDLE hSession,
                         memcpy(pText.data, attribute->attrib.pValue,
                                attribute->attrib.ulValueLen);
                     else {
+                        sftk_FreeAttribute(attribute);
                         crv = CKR_HOST_MEMORY;
                         break;
                     }
@@ -7575,8 +7578,8 @@ NSC_DeriveKey(CK_SESSION_HANDLE hSession,
             if (keySize == 0)
                 keySize = tmpKeySize;
             if (keySize > tmpKeySize) {
-                sftk_FreeObject(newKey);
                 sftk_FreeAttribute(att2);
+                sftk_FreeObject(newKey);
                 crv = CKR_TEMPLATE_INCONSISTENT;
                 break;
             }
@@ -8483,12 +8486,13 @@ NSC_DigestKey(CK_SESSION_HANDLE hSession, CK_OBJECT_HANDLE hKey)
     }
     /* get the key value */
     att = sftk_FindAttribute(key, CKA_VALUE);
-    sftk_FreeObject(key);
     if (!att) {
+        sftk_FreeObject(key);
         return CKR_KEY_HANDLE_INVALID;
     }
     crv = NSC_DigestUpdate(hSession, (CK_BYTE_PTR)att->attrib.pValue,
                            att->attrib.ulValueLen);
     sftk_FreeAttribute(att);
+    sftk_FreeObject(key);
     return crv;
 }
diff --git a/security/nss/lib/util/secasn1e.c b/security/nss/lib/util/secasn1e.c
index fb3feef522..716f474303 100644
--- a/security/nss/lib/util/secasn1e.c
+++ b/security/nss/lib/util/secasn1e.c
@@ -702,6 +702,10 @@ sec_asn1e_contents_length(const SEC_ASN1Template *theTemplate, void *src,
                 }
                 break;
 
+            case SEC_ASN1_NULL:
+                len = 0;
+                break;
+
             default:
                 len = ((SECItem *)src)->len;
                 break;
diff --git a/security/nss/lib/util/secitem.c b/security/nss/lib/util/secitem.c
index cd69961782..56897be765 100644
--- a/security/nss/lib/util/secitem.c
+++ b/security/nss/lib/util/secitem.c
@@ -469,8 +469,9 @@ SECITEM_ZfreeArray(SECItemArray *array, PRBool freeit)
 SECItemArray *
 SECITEM_DupArray(PLArenaPool *arena, const SECItemArray *from)
 {
-    SECItemArray *result;
+    SECItemArray *result = NULL;
     unsigned int i;
+    void *mark = NULL;
 
     /* Require a "from" array.
      * Reject an inconsistent "from" array with NULL data and nonzero length.
@@ -479,18 +480,36 @@ SECITEM_DupArray(PLArenaPool *arena, const SECItemArray *from)
     if (!from || (!from->items && from->len))
         return NULL;
 
+    if (arena != NULL) {
+        mark = PORT_ArenaMark(arena);
+    }
+
     result = SECITEM_AllocArray(arena, NULL, from->len);
     if (!result)
-        return NULL;
+        goto loser;
 
     for (i = 0; i < from->len; ++i) {
         SECStatus rv = SECITEM_CopyItem(arena,
                                         &result->items[i], &from->items[i]);
         if (rv != SECSuccess) {
-            SECITEM_ZfreeArray(result, PR_TRUE);
-            return NULL;
+            goto loser;
         }
     }
 
+    if (mark) {
+        PORT_ArenaUnmark(arena, mark);
+    }
     return result;
+
+loser:
+    if (arena != NULL) {
+        /* Release rolls back all allocations made since the mark. */
+        if (mark) {
+            PORT_ArenaZRelease(arena, mark);
+        }
+    } else if (result != NULL) {
+        /* Non-arena path: heap-free is correct here. */
+        SECITEM_ZfreeArray(result, PR_TRUE);
+    }
+    return NULL;
 }
diff --git a/security/nss/lib/util/utilpars.c b/security/nss/lib/util/utilpars.c
index f9b807f7ed..779a991095 100644
--- a/security/nss/lib/util/utilpars.c
+++ b/security/nss/lib/util/utilpars.c
@@ -913,6 +913,23 @@ NSSUTIL_MkModuleSpec(char *dllName, char *commonName, char *parameters,
     return NSSUTIL_MkModuleSpecEx(dllName, commonName, parameters, NSS, NULL);
 }
 
+/* Count the number of name=value parameters in a parameter string. */
+static size_t
+nssutil_CountParams(const char *params)
+{
+    size_t count = 0;
+    const char *p = params;
+    while (*p) {
+        p = NSSUTIL_ArgStrip(p);
+        if (!*p) {
+            break;
+        }
+        p = NSSUTIL_ArgSkipParameter(p);
+        count++;
+    }
+    return count;
+}
+
 /************************************************************************
  * add a single flag to the Flags= section inside the spec's NSS= section */
 char *
@@ -946,7 +963,11 @@ NSSUTIL_AddNSSFlagToModuleSpec(char *spec, char *addFlag)
     } else {
         const char *iNss = nss;
         PRBool alreadyAdded = PR_FALSE;
-        size_t maxSize = strlen(nss) + strlen(addFlag) + prefixLen + 2; /* space and null terminator */
+        // Allocate enough space for the current string, space delimiters
+        // between all existing parameters, a space before the new flags
+        // parameter, and a null terminator.
+        size_t nParams = nssutil_CountParams(nss);
+        size_t maxSize = strlen(nss) + nParams + strlen(addFlag) + prefixLen + 2;
         nss2 = PORT_Alloc(maxSize);
         *nss2 = 0;
         while (*iNss) {