# Phase 25: Memory Leak Audit - Research

**Researched:** 2026-02-24
**Domain:** React memory leak patterns, useEffect cleanup, Chrome DevTools memory profiling
**Confidence:** HIGH

## Summary

This research identifies memory leak risks in the recording-related code of jam-ui and documents the standard patterns for fixing them. The audit scope includes `useRecordingHelpers.js`, `JKSessionRecordingModal.js`, related Redux state in `sessionUISlice.js`, `fakeJamClientRecordings.js`, and global `window.JK.*` callback registrations.

The primary findings reveal:
1. **Missing useEffect cleanup functions** in `JKSessionRecordingModal.js` - both useEffect hooks lack cleanup returns
2. **Timer without cleanup on unmount** in `useRecordingHelpers.js` - the `waitingOnStopTimer` has internal cleanup but no useEffect cleanup return
3. **Global callback registration without cleanup** in `useRecordingHelpers.js` - `window.JK.*` callbacks are registered but never cleaned up

These patterns are well-documented memory leak sources in React applications. The established fix pattern follows Phase 22's approach: use `useRef` for cleanup stability and always return cleanup functions from useEffect.

**Primary recommendation:** Add cleanup functions to all recording-related useEffect hooks, following the established `useRef` pattern from Phase 22.

## Standard Stack

This phase does not introduce new libraries. The work uses existing React patterns and Chrome DevTools.

### Core Tools
| Tool | Purpose | Why Standard |
|------|---------|--------------|
| Chrome DevTools Memory Panel | Heap snapshot analysis | Official Chrome tool for memory profiling |
| Chrome Performance Monitor | Real-time heap size tracking | Built into Chrome DevTools |
| React DevTools | Component state inspection | Official React debugging extension |

### Patterns Used
| Pattern | Purpose | When to Use |
|---------|---------|-------------|
| useRef for cleanup | Store cleanup targets with stable reference | When cleanup function needs current values |
| AbortController | Cancel async fetch operations | When useEffect performs data fetching |
| ignore flag | Prevent state updates after unmount | When async operation may complete after unmount |

## Architecture Patterns

### Recommended useEffect Cleanup Pattern

Based on Phase 22's established approach and React official documentation:

```javascript
// Pattern: useRef for cleanup stability (from Phase 22)
const resourceRef = useRef(null);

useEffect(() => {
  // Setup
  resourceRef.current = someResource;

  // Cleanup function returned
  return () => {
    if (resourceRef.current) {
      resourceRef.current.cleanup();
      resourceRef.current = null;
    }
  };
}, [dependencies]);
```

### Pattern 1: Timer Cleanup

**What:** Clear timers when component unmounts or dependencies change
**When to use:** Any useEffect that calls setTimeout or setInterval
**Example:**
```javascript
// Source: React official docs - https://react.dev/learn/synchronizing-with-effects
useEffect(() => {
  const timerId = setTimeout(onTick, 5000);
  return () => clearTimeout(timerId);
}, [dependencies]);
```

### Pattern 2: Global Callback Cleanup

**What:** Remove globally registered callbacks when component unmounts
**When to use:** When registering callbacks on window, document, or global objects
**Example:**
```javascript
// Pattern for window.JK.* callbacks
const callbackRef = useRef(null);

useEffect(() => {
  callbackRef.current = handleCallback;
  window.JK = window.JK || {};
  window.JK.HandleSomething = callbackRef.current;

  return () => {
    // Don't delete if another instance registered a different callback
    if (window.JK?.HandleSomething === callbackRef.current) {
      delete window.JK.HandleSomething;
    }
  };
}, [handleCallback]);
```

### Pattern 3: Async Operation Cleanup with ignore Flag

**What:** Prevent state updates after component unmounts
**When to use:** Any useEffect with async operations that call setState
**Example:**
```javascript
// Source: React official docs
useEffect(() => {
  let ignore = false;

  async function fetchData() {
    const result = await someAsyncOperation();
    if (!ignore) {
      setState(result);
    }
  }

  fetchData();

  return () => {
    ignore = true;
  };
}, [dependencies]);
```

### Anti-Patterns to Avoid

- **Missing cleanup return:** useEffect hooks that set up resources but don't return cleanup functions
- **Stale closure in cleanup:** Using state directly in cleanup function instead of refs
- **Conditional cleanup:** Returning cleanup only in some code paths (must always return if needed)

## Don't Hand-Roll

| Problem | Don't Build | Use Instead | Why |
|---------|-------------|-------------|-----|
| Async cancellation | Manual flag tracking | AbortController | Built-in browser API, works with fetch |
| Stable callback refs | Manual closure management | useRef + useCallback | React-optimized approach |
| Memory profiling | Custom tracking code | Chrome DevTools | Comprehensive, accurate, zero overhead |

**Key insight:** React's useEffect cleanup pattern handles most memory leak scenarios. The Phase 22 useRef pattern specifically addresses stale closure issues that can cause cleanup to fail silently.

## Common Pitfalls

### Pitfall 1: Missing Cleanup Returns

**What goes wrong:** useEffect sets up timers, listeners, or callbacks but doesn't return a cleanup function
**Why it happens:** Developer focuses on setup logic and forgets cleanup is needed
**How to avoid:** Treat every useEffect with side effects as requiring a cleanup return
**Warning signs:** Console warnings about "Can't perform state update on unmounted component"

**Found in audit:**
- `JKSessionRecordingModal.js` line 54-71: useEffect fetches audio preference but has no cleanup
- `JKSessionRecordingModal.js` line 74-78: useEffect loads video sources but has no cleanup

### Pitfall 2: Global Callbacks Without Cleanup

**What goes wrong:** Callbacks registered on window/global objects persist after component unmounts
**Why it happens:** Callbacks seem "harmless" since they're just function references
**How to avoid:** Always clean up global registrations; use refs to track what was registered
**Warning signs:** Multiple callback instances running, callbacks referencing stale state

**Found in audit:**
- `useRecordingHelpers.js` line 459-468: Registers 5 `window.JK.*` callbacks without cleanup

### Pitfall 3: Timer Not Cleared on Unmount

**What goes wrong:** setTimeout/setInterval fires after component unmounts, causing state update on unmounted component
**Why it happens:** Timer is cleared in internal logic but not in useEffect cleanup
**How to avoid:** Return cleanup function that clears any active timers
**Warning signs:** Console errors about state updates on unmounted components

**Found in audit:**
- `useRecordingHelpers.js` line 186: `waitingOnStopTimer` has internal cleanup but no useEffect unmount cleanup

### Pitfall 4: Async State Updates After Unmount

**What goes wrong:** Async operations (API calls) complete and call setState after component unmounts
**Why it happens:** No mechanism to cancel or ignore outdated async results
**How to avoid:** Use ignore flag pattern or AbortController
**Warning signs:** Console warnings about state updates on unmounted components

**Found in audit:**
- `JKSessionRecordingModal.js` async calls to `jamClient.GetAudioRecordingPreference()` and `loadAvailableVideoSources()` may complete after modal closes

## Code Examples

### Example 1: Proper Timer Cleanup in useRecordingHelpers

```javascript
// Current problem (line 31 and 186):
const waitingOnStopTimer = useRef(null);
// Timer is set internally but no useEffect cleanup

// Solution: Add cleanup useEffect
useEffect(() => {
  return () => {
    if (waitingOnStopTimer.current) {
      clearTimeout(waitingOnStopTimer.current);
      waitingOnStopTimer.current = null;
    }
  };
}, []); // Empty deps - cleanup only on unmount
```

### Example 2: Global Callback Registration with Cleanup

```javascript
// Current problem (line 459-468):
useEffect(() => {
  if (window) {
    window.JK = window.JK || {};
    window.JK.HandleRecordingStartResult = handleRecordingStartResult;
    // ... more callbacks
  }
  // NO CLEANUP RETURNED
}, [handleRecordingStartResult, ...]);

// Solution: Store refs and clean up conditionally
const callbackRefs = useRef({});

useEffect(() => {
  // Store current callback references
  callbackRefs.current = {
    HandleRecordingStartResult: handleRecordingStartResult,
    HandleRecordingStopResult: handleRecordingStopResult,
    HandleRecordingStopped: handleRecordingStopped,
    HandleRecordingStarted: handleRecordingStarted,
    HandleRecordingAborted: handleRecordingAborted,
  };

  window.JK = window.JK || {};
  Object.assign(window.JK, callbackRefs.current);

  return () => {
    // Only delete if we still own the callback
    Object.keys(callbackRefs.current).forEach(key => {
      if (window.JK?.[key] === callbackRefs.current[key]) {
        delete window.JK[key];
      }
    });
  };
}, [handleRecordingStartResult, handleRecordingStopResult, handleRecordingStopped, handleRecordingStarted, handleRecordingAborted]);
```

### Example 3: Async Cleanup in Modal

```javascript
// Current problem (JKSessionRecordingModal.js line 54-71):
useEffect(() => {
  const fetchAudioStoreType = async () => {
    const pref = await jamClient.GetAudioRecordingPreference();
    setAudioStoreType(/* ... */);  // May run after unmount
  };
  fetchAudioStoreType();
}, [jamClient]);

// Solution: Add ignore flag
useEffect(() => {
  let ignore = false;

  const fetchAudioStoreType = async () => {
    try {
      if (jamClient) {
        const pref = await jamClient.GetAudioRecordingPreference();
        if (!ignore) {
          // Safe to update state
          if (AUDIO_STORE_TYPE_MIX_AND_STEMS['backendValues'].includes(pref)) {
            setAudioStoreType(AUDIO_STORE_TYPE_MIX_AND_STEMS['key']);
          } else if (AUDIO_STORE_TYPE_MIX_ONLY['backendValues'].includes(pref)) {
            setAudioStoreType(AUDIO_STORE_TYPE_MIX_ONLY['key']);
          }
        }
      }
    } catch (error) {
      if (!ignore) {
        console.error('Error fetching audio store type preference:', error);
      }
    }
  };

  fetchAudioStoreType();

  return () => {
    ignore = true;
  };
}, [jamClient]);
```

## Audit Findings Summary

### Files in Scope

| File | Issues Found | Severity |
|------|--------------|----------|
| `useRecordingHelpers.js` | 2 (timer + callbacks) | HIGH |
| `JKSessionRecordingModal.js` | 2 (async + missing cleanup) | MEDIUM |
| `sessionUISlice.js` | 0 | N/A (Redux, no hooks) |
| `fakeJamClientRecordings.js` | 1 (timers in class) | LOW (test only) |

### Issue Details

1. **useRecordingHelpers.js - Timer Cleanup (HIGH)**
   - Line 31: `waitingOnStopTimer` useRef
   - Line 186: Timer set but no useEffect cleanup on unmount
   - Risk: Timer fires after hook unmounts, causes stale state updates

2. **useRecordingHelpers.js - Global Callbacks (HIGH)**
   - Lines 459-468: 5 window.JK.* callbacks registered
   - No cleanup function returned
   - Risk: Callbacks accumulate on repeated mount/unmount cycles

3. **JKSessionRecordingModal.js - Async Without Ignore (MEDIUM)**
   - Lines 54-71: Async fetch without ignore flag
   - Lines 74-78: Video source loading without cleanup
   - Risk: State updates after modal closes

4. **fakeJamClientRecordings.js - Test Timers (LOW)**
   - Lines 45, 78: Timers for test simulation
   - Lines 199, 205: Timers cleared internally
   - Risk: Minimal (test code only), but could affect test reliability

## State of the Art

| Old Approach | Current Approach | When Changed | Impact |
|--------------|------------------|--------------|--------|
| No cleanup, rely on GC | Always return cleanup functions | React 16.8 (hooks) | Prevents memory leaks |
| State in cleanup closures | useRef for cleanup stability | Established pattern | Avoids stale closure bugs |
| Manual async tracking | AbortController | Widespread 2020+ | Native browser support |

**Current best practice:** React Strict Mode (development) remounts components to verify cleanup works correctly. Code should pass this test without console warnings.

## Open Questions

1. **Multiple hook instances sharing window.JK callbacks**
   - What we know: Both `JKSessionScreen` and `JKSessionRecordingModal` call `useRecordingHelpers`
   - What's unclear: Whether deleting callbacks on modal unmount affects screen
   - Recommendation: Use conditional cleanup that only removes if callback reference matches

2. **FakeJamClientRecordings test reliability**
   - What we know: Timer-based test simulation could be affected by cleanup
   - What's unclear: Whether tests depend on timer persistence
   - Recommendation: Low priority - address if test failures occur after fixes

## Memory Verification Method

Following Phase 23 UAT pattern, create checklist for manual verification:

### Chrome DevTools Steps

1. **Baseline Measurement**
   - Open DevTools > Memory panel
   - Take heap snapshot (Snapshot 1)
   - Record JS Heap size in Performance Monitor

2. **Test Scenario 1: Modal Idle**
   - Open recording modal
   - Leave open for 15+ minutes
   - Take heap snapshot (Snapshot 2)
   - Compare: Growth should be < 5MB

3. **Test Scenario 2: Active Recording**
   - Start recording
   - Record for 15+ minutes
   - Stop recording
   - Take heap snapshot (Snapshot 3)
   - Compare: No unbounded growth

4. **Test Scenario 3: Repeated Open/Close**
   - Open/close recording modal 20 times
   - Take heap snapshot
   - Look for: Retained closures, detached DOM nodes

### Success Metrics

- Zero "state update on unmounted component" warnings
- Heap growth < 50% during 15-minute test
- No detached DOM nodes from modal
- Callbacks not accumulating in window.JK

## Sources

### Primary (HIGH confidence)
- React Official Documentation: https://react.dev/learn/synchronizing-with-effects#step-3-add-cleanup-if-needed - useEffect cleanup patterns
- Chrome DevTools Documentation: https://developer.chrome.com/docs/devtools/memory-problems/heap-snapshots - Memory profiling

### Secondary (MEDIUM confidence)
- Phase 22 Summary: `.planning/phases/22-session-screen-fixes/22-01-SUMMARY.md` - useRef cleanup pattern
- Phase 23 UAT: `.planning/phases/23-verification/23-UAT.md` - Memory verification checklist template

### Tertiary (LOW confidence)
- LogRocket Blog: https://blog.logrocket.com/understanding-react-useeffect-cleanup-function/ - Cleanup patterns
- StackInsight Study: https://stackinsight.dev/blog/memory-leak-empirical-study - Memory leak statistics

## Metadata

**Confidence breakdown:**
- Cleanup patterns: HIGH - Verified against React official docs and established Phase 22 pattern
- Issue identification: HIGH - Direct codebase analysis with line numbers
- Verification method: HIGH - Based on Phase 23 proven approach

**Research date:** 2026-02-24
**Valid until:** 2026-03-24 (30 days - React patterns are stable)
