seth
/
jam-cloud
mirror of https://bitbucket.org/jamkazam/jam-cloud.git
1
0
Fork 0
Code Issues Actions Packages Projects Releases Wiki Activity

docs(19): research phase - memory leak audit and discovery 

Phase 19: Audit and Discovery
- Standard stack: Chrome DevTools Memory Panel for leak detection
- Architecture patterns: Document timer/interval locations and cleanup status
- Pitfalls: Common React memory leak patterns catalogued
- Identified areas: VU meters (LOW), Chat (HIGH), Session screen (MEDIUM)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Nuwan 2026-02-08 14:05:50 +05:30
parent 5fbb51158f
commit a66cbdca92
1 changed files with 353 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

353
.planning/phases/19-audit-and-discovery/19-RESEARCH.md Normal file
View File

@ -0,0 +1,353 @@
# Phase 19: Audit and Discovery - Research
**Researched:** 2026-02-08
**Domain:** React memory leak detection and cleanup patterns
**Confidence:** HIGH
## Summary
This phase involves auditing the jam-ui React application to identify memory leaks causing the ~10-minute session freeze. Research focused on examining the codebase for common React memory leak patterns: missing cleanup functions in useEffect, uncleared setInterval/setTimeout timers, unbounded state growth, and leaked event listeners.
The codebase analysis revealed several areas requiring attention:
1. **VU Meter system**: Uses a callback-driven pattern via `window.JK.HandleBridgeCallback2` that is properly cleaned up
2. **WebSocket/Chat**: Multiple polling intervals and message callbacks that need audit for cleanup
3. **Session Screen**: Complex component with multiple timers, event listeners, and callbacks that require verification
**Primary recommendation:** Systematically audit all `useEffect` hooks, `setInterval`, `setTimeout`, and `addEventListener` calls to ensure proper cleanup functions exist and are called on unmount.
## Standard Stack
### Core Tools (Already in Project)
| Tool | Purpose | Why Use |
|------|---------|---------|
| Chrome DevTools Memory Panel | Heap snapshots, allocation timeline | Definitive memory leak evidence |
| React DevTools | Component tree inspection | Detect retained components |
| Browser Console | Error logging | "Can't perform state update on unmounted component" warnings |
### Audit Methodology
| Approach | Purpose | When to Use |
|----------|---------|-------------|
| Static Code Analysis | Find missing cleanup patterns | First pass - identify candidates |
| Heap Snapshot Comparison | Verify actual memory growth | Validate suspected leaks |
| Allocation Timeline | Trace allocation sources | Isolate specific leak origin |
### No Additional Libraries Needed
This is a code audit phase - no new dependencies required. The existing codebase provides all the patterns needed for fixes.
## Architecture Patterns
### Codebase Timer/Interval Summary
Based on code analysis, here are the timer/interval locations in the session screen area:
#### JKSessionScreen.js
- **Lines 454-472**: Three `setTimeout` calls for track sync (timer1, timer2, timer3) - **HAS CLEANUP** in return function
- **Line 985**: `setTimeout` for video loading button - **NO CLEANUP** (not a leak - short timeout in finally block)
#### JKSessionBackingTrackPlayer.js
- **Lines 155-218**: `setInterval` for playback position polling - **HAS CLEANUP** via `clearInterval(intervalId)`
- **Line 301**: `setTimeout` in async Promise - **NO CLEANUP NEEDED** (one-time delay)
- **Lines 230-239**: Unmount cleanup effect for stopping playback
#### JKSessionJamTrackPlayer.js
- **Lines 415-461**: `setInterval` for position/duration polling - **HAS CLEANUP** via `clearInterval(intervalId)` return
- **Lines 465-473**: Event listener for visibilitychange - **HAS CLEANUP**
#### useJamServer.js (WebSocket Hook)
- **Lines 270-272**: `reconnectTimeoutRef` setTimeout - **HAS CLEANUP** on disconnect and unmount
- **Lines 311-320**: Timeout in sendRequest - **HAS CLEANUP** via clearTimeout
- **Lines 485-493**: `startHeartbeatTimeout`, `heartbeatInterval`, `heartbeatAckCheckInterval` - **HAS CLEANUP** via `stopHeartbeat()`
- **Lines 608-634**: `activityTimeout` for user activity - **HAS CLEANUP** in markActive and cleanup functions
#### useMixerStore.js
- **Line 79**: Sets `window.JK.HandleBridgeCallback2` - **HAS CLEANUP** on line 87 via delete
#### useSessionStats.js
- **Lines 151-153**: `setInterval` for stats refresh every 2 seconds - **HAS CLEANUP** via return clearInterval
### Pattern: Proper Interval Cleanup
```javascript
// Source: React official documentation pattern
useEffect(() => {
const intervalId = setInterval(() => {
// callback logic
}, 1000);
return () => {
clearInterval(intervalId); // CRITICAL: Must clear on unmount
};
}, [dependencies]);
```
### Pattern: Proper Event Listener Cleanup
```javascript
// Source: Current codebase pattern (JKSessionChatWindow.js lines 59-67)
useEffect(() => {
const handleKeyDown = (e) => {
if (e.key === 'Escape') {
dispatch(closeChatWindow());
}
};
window.addEventListener('keydown', handleKeyDown);
return () => window.removeEventListener('keydown', handleKeyDown);
}, [dispatch]);
```
### Pattern: WebSocket Callback Registration
```javascript
// Source: JKSessionScreen.js lines 616-629
// Register callbacks
callbacksToRegister.forEach(({ type, callback }) => {
registerMessageCallback(type, callback);
});
// Store for cleanup
setRegisteredCallbacks(callbacksToRegister);
// On leave/unmount
const unregisterMessageCallbacks = useCallback(() => {
registeredCallbacks.forEach(({ type, callback }) => {
unregisterMessageCallback(type, callback);
});
setRegisteredCallbacks([]);
}, [registeredCallbacks, unregisterMessageCallback]);
```
## Don't Hand-Roll
| Problem | Don't Build | Use Instead | Why |
|---------|-------------|-------------|-----|
| Debounced cleanup | Custom debounce with manual cleanup | useRef + useEffect pattern | Standard React pattern, less error-prone |
| Timer ID tracking | Global variable tracking | useRef for timer IDs | Survives re-renders, cleaned with component |
| WebSocket cleanup | Manual unsubscribe tracking | Store callback references in state | Ensures all callbacks tracked for cleanup |
**Key insight:** The codebase already follows most React best practices. The focus should be on finding edge cases where cleanup is missing or incomplete.
## Common Pitfalls
### Pitfall 1: Stale Closure in Cleanup
**What goes wrong:** Cleanup function captures stale values from closure
**Why it happens:** Dependencies not properly specified in useEffect
**How to avoid:** Include all referenced values in dependency array
**Warning signs:** Cleanup runs but timer/listener still fires
### Pitfall 2: Conditional Timer Setup Without Conditional Cleanup
**What goes wrong:** Timer created conditionally but cleanup always runs
**Why it happens:** Timer ID undefined when clearInterval called
**How to avoid:** Check if timer ID exists before clearing
**Warning signs:** No error but timer leaks when condition was false
### Pitfall 3: State Update After Unmount
**What goes wrong:** Console warning "Can't perform state update on unmounted component"
**Why it happens:** Async operation completes after component unmounts, tries to setState
**How to avoid:** Use mounted ref pattern or abort controller
**Warning signs:** Console warnings during navigation
### Pitfall 4: Unbounded Array Growth in Redux
**What goes wrong:** Redux state grows indefinitely (e.g., chat messages)
**Why it happens:** Messages added without limit or cleanup
**How to avoid:** Implement max message limit or pagination
**Warning signs:** Memory grows linearly with time/messages
### Pitfall 5: Event Listener Registered Multiple Times
**What goes wrong:** Same listener added on every render
**Why it happens:** Missing dependency array or listener not memoized
**How to avoid:** Empty dependency array for one-time setup, or memoize callback
**Warning signs:** Callback fires multiple times for single event
## Code Examples
### Current VU Meter Pattern (VuContext.js + useVuHelpers.js)
```javascript
// Source: jam-ui/src/hooks/useVuHelpers.js lines 117-122
useEffect(() => {
return () => {
// Cleanup on unmount
setVuStates({});
};
}, []);
// VU updates come from native client via:
// window.JK.HandleBridgeCallback2 in useMixerStore.js
// This is properly cleaned up on line 87
```
### Current WebSocket Chat Pattern (sessionChatSlice.js)
```javascript
// Source: jam-ui/src/store/features/sessionChatSlice.js
// Messages stored in: state.messagesByChannel[channel]
// POTENTIAL ISSUE: No max message limit implemented
// Messages accumulate indefinitely during session
```
### Current Polling Pattern (JKSessionBackingTrackPlayer.js)
```javascript
// Source: jam-ui/src/components/client/JKSessionBackingTrackPlayer.js lines 145-227
useEffect(() => {
let intervalId = null;
if (isPlaying && jamClient) {
const pollingInterval = isVisible ? 500 : 2000;
intervalId = setInterval(async () => {
// Poll position/duration
}, pollingInterval);
}
// CLEANUP: Properly clears interval
return () => {
if (intervalId) {
clearInterval(intervalId);
}
};
}, [isPlaying, jamClient, backingTrack, isVisible, ...]);
```
## Identified Areas for Audit
### Priority 1: VU Meters (VUMTR)
| File | Issue | Severity | Evidence |
|------|-------|----------|----------|
| `useMixerStore.js` | VU callback on `window.JK` | LOW | Has cleanup on unmount |
| `useVuHelpers.js` | VuStates accumulation | MEDIUM | States cleared on unmount but not per-mixer |
| `SessionTrackVU.js` | Mixer registration | LOW | Has cleanup return function |
**Audit Action:** Verify VU state doesn't grow unbounded when tracks added/removed
### Priority 2: Chat Window (CHAT)
| File | Issue | Severity | Evidence |
|------|-------|----------|----------|
| `sessionChatSlice.js` | Message array growth | HIGH | No max limit on messagesByChannel |
| `JKSessionChatWindow.js` | Keyboard listener | LOW | Has cleanup |
| `JKChatMessageList.js` | Scroll timeout | LOW | Has cleanup |
| `useSessionWebSocket.js` | Chat callback | MEDIUM | Callback stored but cleanup unclear |
**Audit Action:** Implement message limit or pagination; verify WebSocket callback cleanup
### Priority 3: Session Screen (SESS)
| File | Issue | Severity | Evidence |
|------|-------|----------|----------|
| `JKSessionScreen.js` | Track sync timers | LOW | Has cleanup on lines 470-472 |
| `JKSessionScreen.js` | Message callbacks | MEDIUM | Stored in state, cleanup on leave |
| `JKSessionScreen.js` | window.JK.HandleAlertCallback | LOW | Deleted on cleanup |
| `useJamServer.js` | Heartbeat intervals | LOW | Proper cleanup in stopHeartbeat |
| `useJamServer.js` | Activity timeout | LOW | Cleared in multiple places |
| `useSessionModel.js` | backendMixerAlertThrottleTimer | MEDIUM | Check cleanup completeness |
**Audit Action:** Verify all cleanup runs on unmount; trace callback lifecycle
### Priority 4: Players
| File | Issue | Severity | Evidence |
|------|-------|----------|----------|
| `JKSessionBackingTrackPlayer.js` | Position polling | LOW | Has cleanup |
| `JKSessionJamTrackPlayer.js` | Position polling | LOW | Has cleanup |
| `JKSessionMetronomePlayer.js` | Visibility listener | LOW | Has cleanup |
**Audit Action:** Verify polling stops when hidden/closed
## Verification Strategy
### Phase 1: Static Code Audit
1. Grep for all `setInterval`, `setTimeout`, `requestAnimationFrame` calls
2. Verify each has corresponding cleanup in return function
3. Check all `addEventListener` calls have `removeEventListener`
4. Review Redux slices for unbounded array growth patterns
### Phase 2: Runtime Verification
1. Open Chrome DevTools > Memory tab
2. Take heap snapshot before joining session
3. Join session, interact for 5 minutes
4. Take heap snapshot
5. Compare: look for growing objects (especially arrays, listeners)
6. Force GC and take another snapshot to confirm actual leaks
### Phase 3: Stress Testing
1. Join session, enable all features (chat, VU meters, players)
2. Monitor memory usage in Chrome Task Manager
3. Run for 15+ minutes
4. Memory should plateau, not grow linearly
5. Document specific growth patterns if found
## State of the Art
| Old Approach | Current Approach | When Changed | Impact |
|--------------|------------------|--------------|--------|
| Manual timer tracking | useRef for timer IDs | React 16.8+ | Cleaner cleanup, survives re-renders |
| isMounted flag pattern | AbortController / useEffect cleanup | React 17+ | Better async cancellation |
| ComponentWillUnmount | useEffect return function | React 16.8+ | Hooks-based cleanup |
**Deprecated/outdated:**
- `isMounted` pattern: Considered an anti-pattern, use AbortController instead
- Direct DOM manipulation for cleanup: Use React refs and effects
## Open Questions
1. **What causes the 10-minute freeze specifically?**
- What we know: App freezes, likely memory-related
- What's unclear: Specific trigger - is it memory exhaustion or GC pause?
- Recommendation: Add performance monitoring to pinpoint exact moment
2. **WebSocket callback lifecycle**
- What we know: Callbacks registered on join, should unregister on leave
- What's unclear: Are they always unregistered on all exit paths (error, timeout)?
- Recommendation: Trace all exit paths in JKSessionScreen
3. **Redux message limit**
- What we know: Messages accumulate in messagesByChannel
- What's unclear: What's the typical message volume per session?
- Recommendation: Implement 500-message limit as starting point
## Sources
### Primary (HIGH confidence)
- Codebase analysis of jam-ui/src components and hooks
- [React & useEffect cleanups | Tasos Kakouris](https://tasoskakour.com/blog/react-use-effect-cleanups/)
- [Chrome DevTools Memory Panel | Chrome for Developers](https://developer.chrome.com/docs/devtools/memory)
- [Fix memory problems | Chrome DevTools](https://developer.chrome.com/docs/devtools/memory-problems)
### Secondary (MEDIUM confidence)
- [Understanding Memory Leaks in React | Medium](https://medium.com/@ignatovich.dm/understanding-memory-leaks-in-react-how-to-find-and-fix-them-fc782cf182be)
- [How to Fix Memory Leaks in React | freeCodeCamp](https://www.freecodecamp.org/news/fix-memory-leaks-in-react-apps/)
- [Isolating memory leaks with Chrome's Allocation Timeline | LogRocket](https://blog.logrocket.com/isolating-memory-leaks-with-chromes-allocation-timeline-244fa9c48e8e/)
- [Detect and Prevent Memory Leaks in React.js | DhiWise](https://www.dhiwise.com/post/the-complete-guide-to-detect-and-prevent-memory-leaks-in-react-js)
### Tertiary (LOW confidence)
- General React memory leak best practices from training data
## Metadata
**Confidence breakdown:**
- Standard stack: HIGH - Chrome DevTools and React patterns are well-documented
- Architecture patterns: HIGH - Based on direct codebase analysis
- Pitfalls: HIGH - Common React patterns, verified against codebase
**Research date:** 2026-02-08
**Valid until:** 30 days (stable React patterns)
## Appendix: Files to Audit
### Primary Focus (Session Screen Components)
- `/jam-ui/src/components/client/JKSessionScreen.js`
- `/jam-ui/src/components/client/SessionTrackVU.js`
- `/jam-ui/src/components/client/JKSessionChatWindow.js`
- `/jam-ui/src/components/client/chat/JKChatMessageList.js`
- `/jam-ui/src/components/client/JKSessionBackingTrackPlayer.js`
- `/jam-ui/src/components/client/JKSessionJamTrackPlayer.js`
### Supporting Hooks
- `/jam-ui/src/hooks/useJamServer.js`
- `/jam-ui/src/hooks/useMixerStore.js`
- `/jam-ui/src/hooks/useMixerHelper.js`
- `/jam-ui/src/hooks/useVuHelpers.js`
- `/jam-ui/src/hooks/useSessionStats.js`
- `/jam-ui/src/hooks/useSessionModel.js`
### Context Providers
- `/jam-ui/src/context/VuContext.js`
- `/jam-ui/src/context/MixersContext.js`
- `/jam-ui/src/context/JamServerContext.js`
### Redux Slices
- `/jam-ui/src/store/features/sessionChatSlice.js`
- `/jam-ui/src/store/features/activeSessionSlice.js`
- `/jam-ui/src/store/features/mixersSlice.js`