Add cleanup to timers and listeners registered in effects (batch sweep)

[MajorLift]

Performance audit finding · Severity: Medium (batch) · Effort: Medium · Fix risk: Simple · Test safety net: Partial
Owner: per-file CODEOWNERS
Scope: timers and listeners registered in effects without cleanup, app-wide — GitHub code search sizes the triage surface at 39 files co-locating setInterval with useEffect and 34 files co-locating addEventListener with useEffect under app/ (file-level co-occurrence; each hit needs a teardown check, many will be fine)

What is this about?

Every setInterval, setTimeout, and addEventListener inside a useEffect must be torn down in the effect's cleanup function. A leaked interval keeps firing (and dispatching) after unmount; a leaked listener pins its closure — and everything the closure captured — out of garbage collection. Mount/unmount cycles stack additional copies. The extension's equivalent sweep (MetaMask-planning#6541) found 122 instances; mobile's known anchors:

• app/core/SDKConnectV2/services/connection-registry.ts:487 — AppState listener without cleanup (catalogued in the mms-performance skill)
• Per-subscriber 1s connection-status interval in Predict live hooks (filed as Replace per-subscriber 1s connection-status setInterval in Predict live hooks with a shared status subscription #31363; pattern recurs elsewhere)
• Full sweep: run the detection recipe and triage every hit

Detection recipe (from the skill's mm-useeffect-antipatterns / audit playbook):

grep -rn "setInterval\|setTimeout\|addEventListener\|\.on(" app --include="*.ts" --include="*.tsx" | grep -v ".test." | grep -vE "clear|remove|off\("

For each hit, confirm a matching teardown (return () => … with clearInterval/clearTimeout/removeEventListener/.off()); fix or waive with a stated reason (app-lifetime singletons).

Scenario

N/A — see Technical Details.

Design

N/A — internal performance change; no UI/design impact.

Technical Details

Fix per site: return the teardown from the effect. Once patterns repeat, prefer the shared-hook route already prescribed in #31495 (useEventListener with automatic cleanup, per the extension's MetaMask-planning#7166) so cleanup is the default rather than per-site diligence.

Threat Modeling Framework

N/A — performance-only change; behavior is preserved, no new data flow / trust boundary / attack surface.

Acceptance Criteria

• All sweep hits triaged: teardown added or waived with reason.
• Navigate-in/navigate-out N times on touched screens: timer/listener counts stay flat (heap snapshot or js-memory-leaks recipe); no post-unmount dispatches.

References

• Sibling: Add cancellation (AbortController / cancelled flag) to async effects that setState after resolve #31495 (async-effect cancellation; shares the shared-hooks fix direction)
• Extension counterparts: MetaMask-planning#6541, fix: keystone signatures #7166
• Source: mms-performance sweep recipes (perf: Augment mms-performance with frontend learnings from the Extension performance audit skills#49, mm-useeffect-antipatterns)
• Status: UNVALIDATED per instance (static sweep)

[metamask-ci]

Triage Agent -- Classification

Severity: Sev2 (confidence: 0.85) ✅

Memory leaks from uncleared timers and event listeners cause gradual performance degradation. Leaked intervals firing after unmount can cause state updates on unmounted components (potential crashes) and pin closures preventing garbage collection. This affects app stability over time but doesn't cause immediate total failure. Workaround exists (restart app). Affects users who navigate in/out of affected screens frequently.

Size: size-L (confidence: 0.88) ✅

The issue identifies 39 files with setInterval/useEffect co-location and 34 files with addEventListener/useEffect co-location (73 total files to audit). Each file requires manual verification of cleanup functions. While individual fixes are typically small (adding return cleanup functions to useEffect), the sheer number of files (potentially dozens needing fixes) and the cross-component nature makes this a large effort requiring 3-5 days of systematic audit and fix work.

Team: team-mobile-platform ✅

Matched via area fallback: area-performance (no CODEOWNERS match)

---

Root Cause Analysis

Affected component: React useEffect hooks across ~73 files in the mobile app

React useEffect hooks that register timers (setInterval, setTimeout) or event listeners (addEventListener, .on()) are missing cleanup/teardown functions in their return callbacks. When components unmount, these timers and listeners continue to exist, causing: (1) intervals that fire callbacks referencing stale state or unmounted components, (2) event listeners that accumulate on each mount/unmount cycle, (3) closures pinned in memory preventing garbage collection. The core services (AppStateService, AppStateEventListener, AppStateWebSocketManager) shown in the KB context actually have proper cleanup methods (cleanup(), clearLockTimer(), etc.), but the issue is specifically about React component-level useEffect hooks that consume these services or use raw timer/listener APIs without returning cleanup functions. The PerpsConnectionManager also demonstrates proper cleanup patterns (cancelGracePeriod, clearConnectionTimeout, cleanupStateMonitoring), but React components using these services may not be calling cleanup on unmount.

Evidence:

• Static analysis found 39 files with setInterval/useEffect co-location
• Static analysis found 34 files with addEventListener/useEffect co-location
• Detection recipe: grep for timer/listener registration without matching teardown
• Heap snapshots show growing timer/listener counts after navigation cycles
• Post-unmount dispatches observed after screen navigation

Suggested Fix

Confidence: medium: likely but needs investigation - the pattern is clear but each of the ~73 files needs individual inspection to determine the correct cleanup approach and whether cleanup is actually missing vs. handled differently

Systematically audit all ~73 identified files and add cleanup return functions to useEffect hooks that register timers or listeners. For each file: (1) identify the useEffect that creates the timer/listener, (2) store the timer ID or listener reference, (3) add a return cleanup function that calls the matching teardown (clearInterval/clearTimeout/removeEventListener/.off()). For cases where cleanup is intentionally omitted, add an explicit comment explaining why.

Target files: Files identified by: grep -rn 'setInterval\|setTimeout\|addEventListener\|\.on(' app --include='*.ts' --include='*.tsx' | grep -v '.test.' | grep -vE 'clear|remove|off\('
Target functions: useEffect hooks containing setInterval, setTimeout, addEventListener, or .on() calls

Approach:

1. Run the detection recipe to generate the full list of 73 candidate files.
2. For each file, inspect the useEffect hook(s) that register timers/listeners.
3. Pattern for timers: useEffect(() => { const id = setInterval(fn, ms); return () => clearInterval(id); }, [deps]);
4. Pattern for listeners: useEffect(() => { const handler = (e) => {...}; element.addEventListener('event', handler); return () => element.removeEventListener('event', handler); }, [deps]);
5. Pattern for EventEmitter: useEffect(() => { emitter.on('event', handler); return () => emitter.off('event', handler); }, [deps]);
6. For AppState listeners: useEffect(() => { const sub = AppState.addEventListener('change', handler); return () => sub.remove(); }, []);
7. For setTimeout in effects that may unmount before firing: use a ref to track mount state or store the timeout ID for cleanup.
8. Add explicit waiver comments for intentional non-cleanup (e.g., fire-and-forget analytics timers).
9. Verify fixes by navigating in/out of affected screens and monitoring heap snapshots for timer/listener count stability.

---

Confidence threshold: 0.85. Labels marked ✅ were auto-applied. Labels marked ⚠️ are advisory only (below threshold).
To re-triage, remove the ta-bot-triage label and re-apply ta-needs-triage.