Fix: Additional memory leaks and performance issues in plugins by katspaugh · Pull Request #4211 · katspaugh/wavesurfer.js

katspaugh · 2025-10-01T13:29:07Z

Summary

This PR fixes 15 additional memory leaks, performance issues, and bugs discovered in a follow-up analysis of the codebase. These issues primarily affect plugins and could cause memory leaks in long-running applications.

Issues Fixed

Critical Performance Issues

Timeline Plugin (Add API for events like click, ready etc #19): Refactored to use single scroll listener instead of one per notch
- Previously created N listeners for N notches (e.g., 60 for a 60-second audio)
- Significant improvement for scroll performance with many timeline notches

Memory Leaks Fixed

Envelope Plugin (Presetting markers #16, Cursor reached a mark #21, Mobile Browser Performance #22):
- Clear throttle timeout on destroy
- Remove touch event listeners (touchstart, touchmove, touchend)
- Remove double-click listener
Hover Plugin (Assorted features and fixes #17, Fixed webAudio initializer to use global Audio Context. #24):
- Properly track and clean up zoom/scroll event listeners
- Store only pointer coordinates instead of entire PointerEvent object
Zoom Plugin (Refactored seeking for the same behavior when clicking as calling skip() #18):
- Fix wheel listener removal (was removing from wrong element)
Minimap Plugin (Load new wave? #20, Fixed webAudio initializer to use global Audio Context. #25):
- Clear container reference on destroy
- Prevent race condition with concurrent initMinimap() calls

Safety & Validation Improvements

Decoder (Fixed webAudio initializer to use global Audio Context. #23, webkit prefix in webkitRequestAnimationFrame (webaudio.js) #29):
- Use explicit try/finally for AudioContext cleanup
- Add input validation to prevent invalid AudioBuffer creation
- Prevent division by zero errors

Code Quality Improvements

Renderer (Fixing bug #26): Clarify createDelay logic and variable naming
WebAudioPlayer (Restore scrollParent option #27): Add documentation about blob URL management
Player (Current position marker stops updating #30): Reorder destroy operations for logical cleanup sequence

Testing Recommendations

Create/destroy plugins repeatedly and monitor event listener counts
Test timeline with very long audio files (hours)
Test envelope plugin on touch devices
Load files rapidly to verify no race conditions
Monitor memory usage during extended use

Impact

These fixes will prevent memory leaks in applications that:

Create and destroy plugins frequently
Use timeline with long audio files
Run for extended periods
Use touch devices with envelope plugin

🤖 Generated with Claude Code

- Clear throttleTimeout in EnvelopePlugin destroy to prevent callbacks after destruction - Store and remove dblclick listener from Polyline SVG element - Store and remove touch event listeners (touchstart, touchmove, touchend) - Clear pressTimer on Polyline destroy to prevent pending callbacks Fixes memory leaks in envelope plugin (issues #16, #21, #22)

- Create unsubscribe functions immediately after attaching listeners - Store only pointer coordinates instead of entire PointerEvent object - Properly track zoom and scroll event unsubscribers Fixes issues #17 and #24

- Listener was added to container but removed from wrapper - Now correctly removes listener from container element Fixes issue #18

- Refactored to use one scroll listener instead of one per notch - Store notch metadata in a Map for batch visibility updates - Significantly improves scroll performance with many notches - Previously created N listeners for N notches (e.g., 60 for a 60-second audio) Fixes issue #19

- Clear container reference on destroy to aid garbage collection - Add isInitializing flag to prevent concurrent initMinimap() calls - Ensures proper cleanup and prevents orphaned WaveSurfer instances Fixes issues #20 and #25

- Use explicit try/finally for AudioContext cleanup in decode() - Add input validation to createBuffer() to prevent invalid AudioBuffer - Prevent division by zero and undefined channel data errors Fixes issues #23 and #29

- Rename 'reject' variable to 'rejectFn' for clarity - Clear both timeout and reject function in onClear - Add explicit typing and comments for better maintainability - Ensure proper cleanup of pending delays Fixes issue #26

- Add documentation noting WebAudioPlayer doesn't manage blob URLs - Clarify that Player class handles blob URL lifecycle - Helps prevent memory leaks when using WebAudioPlayer standalone Addresses issue #27

- Call media.load() before media.remove() instead of after - Ensures media element is properly reset before DOM removal - More logical cleanup sequence Fixes issue #30

Copilot

Pull Request Overview

This PR addresses 15 memory leaks, performance issues, and bugs primarily affecting plugins in long-running applications. The changes focus on proper cleanup of event listeners, timeouts, and references to prevent memory accumulation.

Timeline plugin refactored to use single scroll listener instead of one per notch for significant performance improvement
Memory leaks fixed in envelope, hover, zoom, and minimap plugins through proper event listener cleanup
Safety improvements added to decoder with input validation and proper AudioContext cleanup

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/webaudio.ts	Added documentation about blob URL management responsibility
src/renderer.ts	Improved createDelay logic with clearer variable naming and cleanup
src/plugins/zoom.ts	Fixed wheel listener removal from correct element
src/plugins/timeline.ts	Refactored to use single scroll listener and batch notch updates
src/plugins/minimap.ts	Added race condition prevention and container cleanup
src/plugins/hover.ts	Fixed event listener cleanup and reduced memory usage
src/plugins/envelope.ts	Added comprehensive cleanup for touch events and timers
src/player.ts	Reordered destroy operations for logical cleanup sequence
src/decoder.ts	Added input validation and proper AudioContext cleanup

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-01T13:29:56Z

src/plugins/hover.ts

-      if (this.lastPointerMove) this.onPointerMove(this.lastPointerMove)
+      if (this.lastPointerPosition) {
+        // Recreate a minimal PointerEvent-like object
+        this.onPointerMove(this.lastPointerPosition as PointerEvent)


The type assertion as PointerEvent is unsafe since lastPointerPosition only contains clientX and clientY properties. Consider creating a proper interface or passing the required properties explicitly to avoid potential runtime errors if onPointerMove expects other PointerEvent properties.

Copilot · 2025-10-01T13:29:57Z

src/decoder.ts

+  if (!channelData[0] || channelData[0].length === 0) {
+    throw new Error('channelData must contain non-empty channel arrays')
+  }
+
  // If a single array of numbers is passed, make it an array of arrays
  if (typeof channelData[0] === 'number') channelData = [channelData as unknown as number[]]



This validation occurs after the type coercion check on line 45, but the validation assumes channelData[0] exists as an array. If channelData[0] is a number (which gets converted to an array later), this check will incorrectly fail since numbers don't have a length property. Move this validation after the type coercion logic.

Suggested change

if (!channelData[0] || channelData[0].length === 0) {

throw new Error('channelData must contain non-empty channel arrays')

}

// If a single array of numbers is passed, make it an array of arrays

if (typeof channelData[0] === 'number') channelData = [channelData as unknown as number[]]

// If a single array of numbers is passed, make it an array of arrays

if (typeof channelData[0] === 'number') channelData = [channelData as unknown as number[]]

if (!channelData[0] || channelData[0].length === 0) {

throw new Error('channelData must contain non-empty channel arrays')

}

Copilot · 2025-10-01T13:29:57Z

src/plugins/timeline.ts

-      if (!this.wavesurfer) return
-      const width = element.clientWidth
-      const isVisible = start >= scrollLeft && start + width < scrollRight
+    const notchData = this.notchElements.get(element)!


Using the non-null assertion operator ! without a prior existence check is risky. Consider adding a guard clause to handle the case where the element is not found in the map to prevent potential runtime errors.

Suggested change

const notchData = this.notchElements.get(element)!

const notchData = this.notchElements.get(element)

if (!notchData) {

// If the element is not found in the map, do not proceed

return

}

cloudflare-workers-and-pages · 2025-10-01T13:31:25Z

Deploying wavesurfer-js with Cloudflare Pages

Latest commit:	`ae548c9`
Status:	✅ Deploy successful!
Preview URL:	https://33a0bd2d.wavesurfer-js.pages.dev
Branch Preview URL:	https://fix-additional-memory-leaks.wavesurfer-js.pages.dev

View logs

- Validate channel length after converting flat array to array of arrays - Prevents false validation errors for valid flat array inputs - Fixes failing e2e tests for pre-decoded waveform and blob loading

katspaugh added 9 commits October 1, 2025 15:19

Fix: Improve hover plugin event listener cleanup and memory usage

5e7d8b9

- Create unsubscribe functions immediately after attaching listeners - Store only pointer coordinates instead of entire PointerEvent object - Properly track zoom and scroll event unsubscribers Fixes issues #17 and #24

Fix: Remove zoom wheel listener from correct element

23aae4c

- Listener was added to container but removed from wrapper - Now correctly removes listener from container element Fixes issue #18

Fix: Improve decoder AudioContext safety and input validation

a1cc6d3

- Use explicit try/finally for AudioContext cleanup in decode() - Add input validation to createBuffer() to prevent invalid AudioBuffer - Prevent division by zero and undefined channel data errors Fixes issues #23 and #29

Fix: Clarify renderer createDelay logic and cleanup

4f24b7e

- Rename 'reject' variable to 'rejectFn' for clarity - Clear both timeout and reject function in onClear - Add explicit typing and comments for better maintainability - Ensure proper cleanup of pending delays Fixes issue #26

Doc: Clarify blob URL management responsibility in WebAudioPlayer

7435bdb

- Add documentation noting WebAudioPlayer doesn't manage blob URLs - Clarify that Player class handles blob URL lifecycle - Helps prevent memory leaks when using WebAudioPlayer standalone Addresses issue #27

Fix: Reorder player destroy operations for logical cleanup

b5a9b8c

- Call media.load() before media.remove() instead of after - Ensures media element is properly reset before DOM removal - More logical cleanup sequence Fixes issue #30

Copilot AI review requested due to automatic review settings October 1, 2025 13:29

Copilot AI reviewed Oct 1, 2025

View reviewed changes

Fix: Move validation after channelData conversion in decoder

ae548c9

- Validate channel length after converting flat array to array of arrays - Prevents false validation errors for valid flat array inputs - Fixes failing e2e tests for pre-decoded waveform and blob loading

katspaugh merged commit 6727a80 into main Oct 1, 2025
6 checks passed

katspaugh deleted the fix/additional-memory-leaks-and-bugs branch October 1, 2025 13:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix: Additional memory leaks and performance issues in plugins#4211

Fix: Additional memory leaks and performance issues in plugins#4211
katspaugh merged 10 commits intomainfrom
fix/additional-memory-leaks-and-bugs

katspaugh commented Oct 1, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Oct 1, 2025

Uh oh!

Copilot AI Oct 1, 2025

Uh oh!

Copilot AI Oct 1, 2025

Uh oh!

cloudflare-workers-and-pages bot commented Oct 1, 2025 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

katspaugh commented Oct 1, 2025

Summary

Issues Fixed

Critical Performance Issues

Memory Leaks Fixed

Safety & Validation Improvements

Code Quality Improvements

Testing Recommendations

Impact

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

cloudflare-workers-and-pages bot commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deploying wavesurfer-js with Cloudflare Pages

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cloudflare-workers-and-pages bot commented Oct 1, 2025 •

edited

Loading