Fix: Multiple memory leaks and resource management issues#4209
Merged
Fix: Multiple memory leaks and resource management issues#4209
Conversation
The Timer class had a critical memory leak caused by an infinite requestAnimationFrame loop that continuously added new event listeners. Issues fixed: - Removed event-based tick loop that accumulated listeners in memory - Added proper animationFrameId tracking for cancellation - Added isRunning flag to prevent multiple simultaneous loops - Implemented proper cleanup with cancelAnimationFrame() The timer now uses a direct requestAnimationFrame pattern without event subscriptions, properly manages resources, and can be cleanly stopped without leaking memory or consuming CPU cycles. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The once() implementation had a memory leak caused by adding two listeners (the original and a wrapper) through a recursive on() call. Issues fixed: - Removed recursive this.on() call that created duplicate listeners - Now uses a single wrapper function that removes itself after execution - Wrapper properly cleans up by removing itself before calling original listener - Unsubscribe function now correctly references the wrapper The once() pattern now properly adds only one listener that removes itself after the first event emission, preventing memory leaks from accumulated listener references. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The _play() method had a memory leak caused by not properly cleaning up old AudioBufferSourceNode instances before creating new ones. Issues fixed: - Old bufferNode.onended event handler was not removed before disconnect - Event handler held reference to old bufferNode preventing garbage collection - Rapid play/pause cycles accumulated orphaned buffer nodes in memory - Now explicitly sets onended to null before disconnecting The bufferNode cleanup now properly removes event handlers first, allowing the old buffer nodes to be garbage collected and preventing memory leaks during frequent seeking or playback control changes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The virtualAppend() method had memory leaks from improper subscription
management and duplicate subscription tracking.
Issues fixed:
- Removed duplicate subscription: was pushing both once() return value
and the actual unsubscribe function to subscriptions array
- Separated cleanup logic: region.once('remove') now properly handles
unsubscribing from wavesurfer events
- Added comment clarifying that region check prevents orphaned subscriptions
if region is removed before setTimeout executes
- renderIfVisible closure no longer accumulates in subscriptions array
The virtualAppend method now properly tracks subscriptions and cleans them
up when regions are removed, preventing memory leaks from orphaned event
listeners on wavesurfer instances.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
The loadAudio() method had issues with concurrent loads and unnecessary defensive coding that suggested a potential race condition. Issues fixed: - Removed unnecessary optional chaining on freshly created AbortController - Added explicit abort() call for previous fetch before starting new load - Reset abortController to null after aborting to ensure clean state - Prevents multiple concurrent fetches from interfering with each other When load() is called while a fetch is in progress, the previous fetch is now properly aborted before starting the new one, ensuring only one fetch operation is active at a time and preventing race conditions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The setOptions() method would re-initialize drag handlers without checking if they were already set up, causing duplicate handlers to accumulate. Issues fixed: - Added dragUnsubscribe property to track if drag is initialized - initDrag() now returns early if drag is already set up - Prevents multiple drag handlers from responding to same event - Avoids memory leak from duplicate subscriptions accumulating When setOptions() is called multiple times with dragToSeek enabled, only one set of drag handlers will be active, preventing unpredictable behavior and memory leaks. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR addresses 6 critical memory leaks and resource management issues to improve stability during long-running sessions and prevent memory accumulation.
- Replaced event-based timer loop with direct requestAnimationFrame to eliminate infinite listener growth
- Fixed EventEmitter once() method to prevent duplicate listener accumulation
- Added proper cleanup for WebAudio buffer nodes and AbortController race conditions
- Improved drag handler initialization tracking and region subscription management
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/timer.ts | Replaces event-based tick loop with requestAnimationFrame to prevent memory leaks |
| src/event-emitter.ts | Fixes once() method to use single wrapper function instead of recursive subscription |
| src/webaudio.ts | Adds proper cleanup of buffer node event handlers before disconnect |
| src/wavesurfer.ts | Prevents AbortController race conditions and removes unnecessary optional chaining |
| src/renderer.ts | Tracks drag initialization state to prevent duplicate handler setup |
| src/plugins/regions.ts | Separates subscription cleanup logic and removes duplicate tracking |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Deploying wavesurfer-js with
|
| Latest commit: |
a731007
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://5bb40ccf.wavesurfer-js.pages.dev |
| Branch Preview URL: | https://fix-memory-leaks-combined.wavesurfer-js.pages.dev |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR addresses 6 critical memory leaks and resource management issues identified through
code analysis. These fixes prevent memory accumulation during long-running sessions and
improve overall stability.
Issues Fixed
1. Timer infinite loop memory leak (src/timer.ts)
Severity: CRITICAL
2. EventEmitter once() memory leak (src/event-emitter.ts)
Severity: HIGH
3. WebAudioPlayer bufferNode leak (src/webaudio.ts)
Severity: HIGH
4. Regions plugin subscription leak (src/plugins/regions.ts)
Severity: MEDIUM
5. AbortController race condition (src/wavesurfer.ts)
Severity: MEDIUM
6. Renderer drag handler accumulation (src/renderer.ts)
Severity: MEDIUM
Testing Recommendations
Related Issues
🤖 Generated with Claude Code
Co-Authored-By: Claude noreply@anthropic.com