-
Notifications
You must be signed in to change notification settings - Fork 13.5k
Memory leak due to uncleared setInterval in Video/AudioMessageRecorder #40039
Description
Memory Leak in VideoMessageRecorder and AudioMessageRecorder Components
Description
Both VideoMessageRecorder and AudioMessageRecorder components have memory leak vulnerabilities due to improper cleanup of setInterval timers. The intervals created during recording are not guaranteed to be cleared if the component unmounts while recording is in progress, leading to:
- Orphaned timers continuing to run after component unmount
- State updates on unmounted components causing React warnings
- Accumulated memory leaks during extended usage sessions
- Potential performance degradation over time
Steps to Reproduce
VideoMessageRecorder
- Open any chat room
- Click the video record button to start recording
- While recording is active (timer running), quickly:
- Navigate to a different room, OR
- Close the composer, OR
- Refresh the page
- Check browser's DevTools Performance/Memory tab - the interval may still be active
AudioMessageRecorder
- Open any chat room
- Click the audio record button
- While recording is active, quickly navigate away or close the composer
- Same memory leak behavior occurs
Code Analysis
VideoMessageRecorder.tsx (Lines 46, 71-79, 103-113)
// Line 46: Interval stored in state
const [recordingInterval, setRecordingInterval] = useState<ReturnType<typeof setInterval> | null>(null);
// Lines 71-79: Interval created during recording
setRecordingInterval(
setInterval(() => {
const now = new Date();
const distance = (now.getTime() - startTime.getTime()) / 1000;
const minutes = Math.floor(distance / 60);
const seconds = Math.floor(distance % 60);
setTime(`${String(minutes).padStart(2, '0')}:${String(seconds).padStart(2, '0')}`);
}, 1000),
);
// Lines 103-113: useEffect cleanup - DOES NOT clear the interval!
useEffect(() => {
if (!VideoRecorder.getSupportedMimeTypes()) {
return dispatchToastMessage({ type: 'error', message: t('Browser_does_not_support_recording_video') });
}
VideoRecorder.start(videoRef.current ?? undefined, (success) => (!success ? handleCancel() : undefined));
return () => {
VideoRecorder.stop(); // Only stops video, NOT the interval
};
}, [dispatchToastMessage, handleCancel, t]);Problem: The useEffect cleanup function only calls VideoRecorder.stop() but does NOT clear recordingInterval.
AudioMessageRecorder.tsx (Lines 24, 62-70, 95-101)
// Line 24: Interval stored in state
const [recordingInterval, setRecordingInterval] = useState<ReturnType<typeof setInterval> | null>(null);
// Lines 62-70: Interval created during recording
setRecordingInterval(
setInterval(() => {
const now = new Date();
const distance = (now.getTime() - startTime.getTime()) / 1000;
const minutes = Math.floor(distance / 60);
const seconds = Math.floor(distance % 60);
setTime(`${String(minutes).padStart(2, '0')}:${String(seconds).padStart(2, '0')}`);
}, 1000),
);
// Lines 95-101: useEffect cleanup
useEffect(() => {
handleRecord();
return () => {
handleUnmount(); // Async function - may not complete before unmount
};
}, [handleUnmount, handleRecord]);Problem:
handleUnmountis an async function but cleanup functions should be synchronous- The interval cleanup depends on
stopRecordingwhich may not execute in time - Race condition between unmount and interval cleanup
Root Causes
- Missing interval cleanup in useEffect return: The main effect that runs when component mounts doesn't clear the recording interval on unmount
- Async cleanup functions:
handleUnmountandstopRecordingare async but React cleanup should be synchronous - State closure issues: The
recordingIntervalstate may be stale when cleanup runs - Duplicate logic: Both components share identical timer logic but it's duplicated instead of shared
Expected Behavior
When the component unmounts (user navigates away, closes composer, etc.):
- All active intervals should be immediately cleared
- No state updates should occur after unmount
- Camera/microphone resources should be released
- No memory leaks should occur
Actual Behavior
- Intervals continue running after component unmount
setTimeis called on unmounted component (React warning in dev mode)- Memory usage increases with each recording session
- Browser performance degrades over time
Proposed Solution
Option 1: Use useRef for interval (Recommended)
Store interval in a ref instead of state to avoid closure issues:
const VideoMessageRecorder = ({ rid, tmid, reference }: VideoMessageRecorderProps) => {
const recordingIntervalRef = useRef<ReturnType<typeof setInterval> | null>(null);
useEffect(() => {
return () => {
// Clear interval synchronously on unmount
if (recordingIntervalRef.current) {
clearInterval(recordingIntervalRef.current);
recordingIntervalRef.current = null;
}
VideoRecorder.stop();
};
}, [dispatchToastMessage, handleCancel, t]);
const handleRecord = async () => {
if (recordingState === 'recording') {
stopVideoRecording(rid, tmid);
} else {
recordingIntervalRef.current = setInterval(() => {
// ... timer logic ...
}, 1000);
}
};
const stopVideoRecording = async (rid: IRoom['_id'], tmid?: IMessage['_id']) => {
if (recordingIntervalRef.current) {
clearInterval(recordingIntervalRef.current);
recordingIntervalRef.current = null;
}
// ... rest of cleanup ...
};
};Option 2: Extract to custom hook
Create a reusable useRecordingTimer hook to eliminate duplication:
// hooks/useRecordingTimer.ts
export const useRecordingTimer = () => {
const intervalRef = useRef<ReturnType<typeof setInterval> | null>(null);
const [time, setTime] = useState('00:00');
const startTimer = useCallback(() => {
const startTime = new Date();
intervalRef.current = setInterval(() => {
const now = new Date();
const distance = (now.getTime() - startTime.getTime()) / 1000;
const minutes = Math.floor(distance / 60);
const seconds = Math.floor(distance % 60);
setTime(`${String(minutes).padStart(2, '0')}:${String(seconds).padStart(2, '0')}`);
}, 1000);
}, []);
const stopTimer = useCallback(() => {
if (intervalRef.current) {
clearInterval(intervalRef.current);
intervalRef.current = null;
}
setTime('00:00');
}, []);
useEffect(() => {
return () => {
if (intervalRef.current) {
clearInterval(intervalRef.current);
}
};
}, []);
return { time, startTimer, stopTimer };
};Environment
- Rocket.Chat Version: 8.4.0-develop
- Browser: All browsers (Chrome, Firefox, Safari, Edge)
- OS: All platforms
- Node Version: 22.16.0
Impact
- Severity: Medium
- User Impact: Performance degradation during extended chat sessions
- Technical Debt: Code duplication between Video and Audio recorders
Files to Modify
apps/meteor/client/views/composer/VideoMessageRecorder/VideoMessageRecorder.tsxapps/meteor/client/views/composer/AudioMessageRecorder/AudioMessageRecorder.tsx- (Optional) Create shared hook:
apps/meteor/client/hooks/useRecordingTimer.ts
Checklist
- Interval is cleared synchronously in useEffect cleanup
- No state updates occur after component unmount
- Camera/microphone resources are properly released
- Code is DRY (Don't Repeat Yourself) - shared hook created
- Unit tests added for timer cleanup behavior
- Tested in multiple browsers
- No console warnings about state updates on unmounted components
Related Issues
- May be related to performance issues in long-running chat sessions
- Could contribute to memory issues reported in mobile browsers
Additional Notes
The same timer logic is duplicated in both components (lines 72-78 in VideoMessageRecorder and lines 63-69 in AudioMessageRecorder). This is a good opportunity to extract shared logic into a custom hook, improving maintainability and ensuring consistent behavior.