Skip to content

Commit 6727a80

Browse files
authored
Fix: Additional memory leaks and performance issues in plugins (#4211)
* Fix: Clear envelope plugin timeouts and event listeners on destroy - 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) * Fix: Improve hover plugin event listener cleanup and memory usage - 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 - Listener was added to container but removed from wrapper - Now correctly removes listener from container element Fixes issue #18 * Fix: Use single scroll listener for all timeline notches - 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 * Fix: Clear minimap container reference and prevent race condition - 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 * Fix: Improve decoder AudioContext safety and input validation - 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 - 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 - 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 - 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 * Fix: Move validation after channelData conversion in decoder - 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
1 parent 6d2a98b commit 6727a80

File tree

9 files changed

+179
-71
lines changed

9 files changed

+179
-71
lines changed

src/decoder.ts

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
/** Decode an array buffer into an audio buffer */
22
async function decode(audioData: ArrayBuffer, sampleRate: number): Promise<AudioBuffer> {
33
const audioCtx = new AudioContext({ sampleRate })
4-
const decode = audioCtx.decodeAudioData(audioData)
5-
return decode.finally(() => audioCtx.close())
4+
try {
5+
return await audioCtx.decodeAudioData(audioData)
6+
} finally {
7+
// Ensure AudioContext is always closed, even on synchronous errors
8+
audioCtx.close()
9+
}
610
}
711

812
/** Normalize peaks to -1..1 */
@@ -26,9 +30,22 @@ function normalize<T extends Array<Float32Array | number[]>>(channelData: T): T
2630

2731
/** Create an audio buffer from pre-decoded audio data */
2832
function createBuffer(channelData: Array<Float32Array | number[]>, duration: number): AudioBuffer {
33+
// Validate inputs
34+
if (!channelData || channelData.length === 0) {
35+
throw new Error('channelData must be a non-empty array')
36+
}
37+
if (duration <= 0) {
38+
throw new Error('duration must be greater than 0')
39+
}
40+
2941
// If a single array of numbers is passed, make it an array of arrays
3042
if (typeof channelData[0] === 'number') channelData = [channelData as unknown as number[]]
3143

44+
// Validate channel data after conversion
45+
if (!channelData[0] || channelData[0].length === 0) {
46+
throw new Error('channelData must contain non-empty channel arrays')
47+
}
48+
3249
// Normalize to -1..1
3350
normalize(channelData)
3451

src/player.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,11 +91,12 @@ class Player<T extends GeneralEventTypes> extends EventEmitter<T> {
9191
protected destroy() {
9292
if (this.isExternalMedia) return
9393
this.media.pause()
94-
this.media.remove()
9594
this.revokeSrc()
9695
this.media.removeAttribute('src')
9796
// Load resets the media element to its initial state
9897
this.media.load()
98+
// Remove from DOM after cleanup
99+
this.media.remove()
99100
}
100101

101102
protected setMediaElement(element: HTMLMediaElement) {

src/plugins/envelope.ts

Lines changed: 64 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,11 @@ class Polyline extends EventEmitter<{
5656
}
5757
>
5858
private subscriptions: (() => void)[] = []
59+
private dblClickListener?: (e: MouseEvent) => void
60+
private touchStartListener?: (e: TouchEvent) => void
61+
private touchMoveListener?: () => void
62+
private touchEndListener?: () => void
63+
private pressTimer?: number
5964

6065
constructor(options: Options, wrapper: HTMLElement) {
6166
super()
@@ -131,37 +136,42 @@ class Polyline extends EventEmitter<{
131136
}
132137

133138
// Listen to double click to add a new point
134-
svg.addEventListener('dblclick', (e) => {
139+
this.dblClickListener = (e) => {
135140
const rect = svg.getBoundingClientRect()
136141
const x = e.clientX - rect.left
137142
const y = e.clientY - rect.top
138143
this.emit('point-create', x / rect.width, y / rect.height)
139-
})
144+
}
145+
svg.addEventListener('dblclick', this.dblClickListener)
140146

141147
// Long press on touch devices
142-
{
143-
let pressTimer: number
144-
145-
const clearTimer = () => clearTimeout(pressTimer)
146-
147-
svg.addEventListener('touchstart', (e) => {
148-
if (e.touches.length === 1) {
149-
pressTimer = window.setTimeout(() => {
150-
e.preventDefault()
151-
const rect = svg.getBoundingClientRect()
152-
const x = e.touches[0].clientX - rect.left
153-
const y = e.touches[0].clientY - rect.top
154-
this.emit('point-create', x / rect.width, y / rect.height)
155-
}, 500)
156-
} else {
157-
clearTimer()
158-
}
159-
})
160-
161-
svg.addEventListener('touchmove', clearTimer)
162-
163-
svg.addEventListener('touchend', clearTimer)
148+
const clearTimer = () => {
149+
if (this.pressTimer !== undefined) {
150+
clearTimeout(this.pressTimer)
151+
this.pressTimer = undefined
152+
}
153+
}
154+
155+
this.touchStartListener = (e) => {
156+
if (e.touches.length === 1) {
157+
this.pressTimer = window.setTimeout(() => {
158+
e.preventDefault()
159+
const rect = svg.getBoundingClientRect()
160+
const x = e.touches[0].clientX - rect.left
161+
const y = e.touches[0].clientY - rect.top
162+
this.emit('point-create', x / rect.width, y / rect.height)
163+
}, 500)
164+
} else {
165+
clearTimer()
166+
}
164167
}
168+
169+
this.touchMoveListener = clearTimer
170+
this.touchEndListener = clearTimer
171+
172+
svg.addEventListener('touchstart', this.touchStartListener)
173+
svg.addEventListener('touchmove', this.touchMoveListener)
174+
svg.addEventListener('touchend', this.touchEndListener)
165175
}
166176

167177
private makeDraggable(draggable: SVGElement, onDrag: (x: number, y: number) => void) {
@@ -279,6 +289,30 @@ class Polyline extends EventEmitter<{
279289
}
280290

281291
destroy() {
292+
// Clear pending press timer
293+
if (this.pressTimer !== undefined) {
294+
clearTimeout(this.pressTimer)
295+
this.pressTimer = undefined
296+
}
297+
298+
// Remove event listeners
299+
if (this.dblClickListener) {
300+
this.svg.removeEventListener('dblclick', this.dblClickListener)
301+
this.dblClickListener = undefined
302+
}
303+
if (this.touchStartListener) {
304+
this.svg.removeEventListener('touchstart', this.touchStartListener)
305+
this.touchStartListener = undefined
306+
}
307+
if (this.touchMoveListener) {
308+
this.svg.removeEventListener('touchmove', this.touchMoveListener)
309+
this.touchMoveListener = undefined
310+
}
311+
if (this.touchEndListener) {
312+
this.svg.removeEventListener('touchend', this.touchEndListener)
313+
this.touchEndListener = undefined
314+
}
315+
282316
this.subscriptions.forEach((unsubscribe) => unsubscribe())
283317
this.polyPoints.clear()
284318
this.svg.remove()
@@ -363,6 +397,12 @@ class EnvelopePlugin extends BasePlugin<EnvelopePluginEvents, EnvelopePluginOpti
363397
* Destroy the plugin instance.
364398
*/
365399
public destroy() {
400+
// Clear pending throttle timeout
401+
if (this.throttleTimeout) {
402+
clearTimeout(this.throttleTimeout)
403+
this.throttleTimeout = null
404+
}
405+
366406
this.polyline?.destroy()
367407
super.destroy()
368408
}

src/plugins/hover.ts

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ class HoverPlugin extends BasePlugin<HoverPluginEvents, HoverPluginOptions> {
5252
protected options: HoverPluginOptions & typeof defaultOptions
5353
private wrapper: HTMLElement
5454
private label: HTMLElement
55-
private lastPointerMove: PointerEvent | null = null
55+
private lastPointerPosition: { clientX: number; clientY: number } | null = null
5656
private unsubscribe: () => void = () => undefined
5757

5858
constructor(options?: HoverPluginOptions) {
@@ -109,31 +109,36 @@ class HoverPlugin extends BasePlugin<HoverPluginEvents, HoverPluginOptions> {
109109
const container = this.wavesurfer.getWrapper()
110110
container.appendChild(this.wrapper)
111111

112-
// Attach pointer events
113-
container.addEventListener('pointermove', this.onPointerMove)
114-
container.addEventListener('pointerleave', this.onPointerLeave)
115-
116112
// When zoom or scroll happens, re-run the pointer move logic
117113
// with the last known mouse position
118114
const onUpdate = () => {
119-
if (this.lastPointerMove) this.onPointerMove(this.lastPointerMove)
115+
if (this.lastPointerPosition) {
116+
// Recreate a minimal PointerEvent-like object
117+
this.onPointerMove(this.lastPointerPosition as PointerEvent)
118+
}
120119
}
121120

122-
this.wavesurfer.on('zoom', onUpdate)
123-
this.wavesurfer.on('scroll', onUpdate)
121+
// Create unsubscribe function for wavesurfer events
122+
const unsubscribeZoom = this.wavesurfer.on('zoom', onUpdate)
123+
const unsubscribeScroll = this.wavesurfer.on('scroll', onUpdate)
124+
125+
// Attach pointer events and create complete unsubscribe function
126+
container.addEventListener('pointermove', this.onPointerMove)
127+
container.addEventListener('pointerleave', this.onPointerLeave)
128+
124129
this.unsubscribe = () => {
125130
container.removeEventListener('pointermove', this.onPointerMove)
126131
container.removeEventListener('pointerleave', this.onPointerLeave)
127-
this.wavesurfer?.un('zoom', onUpdate)
128-
this.wavesurfer?.un('scroll', onUpdate)
132+
unsubscribeZoom()
133+
unsubscribeScroll()
129134
}
130135
}
131136

132137
private onPointerMove = (e: PointerEvent) => {
133138
if (!this.wavesurfer) return
134139

135-
// Used when zooming
136-
this.lastPointerMove = e
140+
// Store only the position data needed for zoom/scroll updates
141+
this.lastPointerPosition = { clientX: e.clientX, clientY: e.clientY }
137142

138143
// Position
139144
const bbox = this.wavesurfer.getWrapper().getBoundingClientRect()
@@ -157,7 +162,7 @@ class HoverPlugin extends BasePlugin<HoverPluginEvents, HoverPluginOptions> {
157162

158163
private onPointerLeave = () => {
159164
this.wrapper.style.opacity = '0'
160-
this.lastPointerMove = null
165+
this.lastPointerPosition = null
161166
}
162167

163168
/** Unmount */

src/plugins/minimap.ts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ class MinimapPlugin extends BasePlugin<MinimapPluginEvents, MinimapPluginOptions
5454
private miniWavesurfer: WaveSurfer | null = null
5555
private overlay: HTMLElement
5656
private container: HTMLElement | null = null
57+
private isInitializing = false
5758

5859
constructor(options: MinimapPluginOptions) {
5960
super(options)
@@ -122,16 +123,26 @@ class MinimapPlugin extends BasePlugin<MinimapPluginEvents, MinimapPluginOptions
122123
}
123124

124125
private initMinimap() {
126+
// Prevent concurrent initialization
127+
if (this.isInitializing) return
128+
this.isInitializing = true
129+
125130
if (this.miniWavesurfer) {
126131
this.miniWavesurfer.destroy()
127132
this.miniWavesurfer = null
128133
}
129134

130-
if (!this.wavesurfer) return
135+
if (!this.wavesurfer) {
136+
this.isInitializing = false
137+
return
138+
}
131139

132140
const data = this.wavesurfer.getDecodedData()
133141
const media = this.wavesurfer.getMediaElement()
134-
if (!data || !media) return
142+
if (!data || !media) {
143+
this.isInitializing = false
144+
return
145+
}
135146

136147
const peaks = []
137148
for (let i = 0; i < data.numberOfChannels; i++) {
@@ -209,6 +220,9 @@ class MinimapPlugin extends BasePlugin<MinimapPluginEvents, MinimapPluginOptions
209220
this.emit('timeupdate', currentTime)
210221
}),
211222
)
223+
224+
// Reset flag after initialization completes
225+
this.isInitializing = false
212226
}
213227

214228
private getOverlayWidth(): number {
@@ -249,6 +263,7 @@ class MinimapPlugin extends BasePlugin<MinimapPluginEvents, MinimapPluginOptions
249263
public destroy() {
250264
this.miniWavesurfer?.destroy()
251265
this.minimapWrapper.remove()
266+
this.container = null
252267
super.destroy()
253268
}
254269
}

src/plugins/timeline.ts

Lines changed: 37 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ class TimelinePlugin extends BasePlugin<TimelinePluginEvents, TimelinePluginOpti
5858
private timelineWrapper: HTMLElement
5959
private unsubscribeNotches: (() => void)[] = []
6060
protected options: TimelinePluginOptions & typeof defaultOptions
61+
private notchElements: Map<HTMLElement, { start: number; width: number; wasVisible: boolean }> = new Map()
6162

6263
constructor(options?: TimelinePluginOptions) {
6364
super(options || {})
@@ -150,39 +151,46 @@ class TimelinePlugin extends BasePlugin<TimelinePluginEvents, TimelinePluginOpti
150151
}
151152

152153
private virtualAppend(start: number, container: HTMLElement, element: HTMLElement) {
153-
let wasVisible = false
154+
// Store notch metadata for batch updates
155+
this.notchElements.set(element, {
156+
start,
157+
width: element.clientWidth,
158+
wasVisible: false,
159+
})
160+
161+
// Initial render check
162+
if (!this.wavesurfer) return
163+
const scrollLeft = this.wavesurfer.getScroll()
164+
const scrollRight = scrollLeft + this.wavesurfer.getWidth()
154165

155-
const renderIfVisible = (scrollLeft: number, scrollRight: number) => {
156-
if (!this.wavesurfer) return
157-
const width = element.clientWidth
158-
const isVisible = start >= scrollLeft && start + width < scrollRight
166+
const notchData = this.notchElements.get(element)!
167+
const isVisible = start >= scrollLeft && start + notchData.width < scrollRight
168+
notchData.wasVisible = isVisible
159169

160-
if (isVisible === wasVisible) return
161-
wasVisible = isVisible
170+
if (isVisible) {
171+
container.appendChild(element)
172+
}
173+
}
174+
175+
private updateVisibleNotches(scrollLeft: number, scrollRight: number, container: HTMLElement) {
176+
this.notchElements.forEach((notchData, element) => {
177+
const isVisible = notchData.start >= scrollLeft && notchData.start + notchData.width < scrollRight
178+
179+
if (isVisible === notchData.wasVisible) return
180+
notchData.wasVisible = isVisible
162181

163182
if (isVisible) {
164183
container.appendChild(element)
165184
} else {
166185
element.remove()
167186
}
168-
}
169-
170-
if (!this.wavesurfer) return
171-
const scrollLeft = this.wavesurfer.getScroll()
172-
const scrollRight = scrollLeft + this.wavesurfer.getWidth()
173-
174-
renderIfVisible(scrollLeft, scrollRight)
175-
176-
this.unsubscribeNotches.push(
177-
this.wavesurfer.on('scroll', (_start, _end, scrollLeft, scrollRight) => {
178-
renderIfVisible(scrollLeft, scrollRight)
179-
}),
180-
)
187+
})
181188
}
182189

183190
private initTimeline() {
184191
this.unsubscribeNotches.forEach((unsubscribe) => unsubscribe())
185192
this.unsubscribeNotches = []
193+
this.notchElements.clear()
186194

187195
const duration = this.wavesurfer?.getDuration() ?? this.options.duration ?? 0
188196
const pxPerSec = (this.wavesurfer?.getWrapper().scrollWidth || this.timelineWrapper.scrollWidth) / duration
@@ -265,6 +273,15 @@ class TimelinePlugin extends BasePlugin<TimelinePluginEvents, TimelinePluginOpti
265273
this.timelineWrapper.innerHTML = ''
266274
this.timelineWrapper.appendChild(timeline)
267275

276+
// Add single scroll listener for all notches (instead of one per notch)
277+
if (this.wavesurfer) {
278+
this.unsubscribeNotches.push(
279+
this.wavesurfer.on('scroll', (_start, _end, scrollLeft, scrollRight) => {
280+
this.updateVisibleNotches(scrollLeft, scrollRight, timeline)
281+
}),
282+
)
283+
}
284+
268285
this.emit('ready')
269286
}
270287
}

src/plugins/zoom.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,8 +157,8 @@ class ZoomPlugin extends BasePlugin<ZoomPluginEvents, ZoomPluginOptions> {
157157
}
158158

159159
destroy() {
160-
if (this.wrapper) {
161-
this.wrapper.removeEventListener('wheel', this.onWheel)
160+
if (this.container) {
161+
this.container.removeEventListener('wheel', this.onWheel)
162162
}
163163
super.destroy()
164164
}

0 commit comments

Comments
 (0)