Skip to content

_performNotes uses raw setTimeout for effect cleanup instead of ManagedTimer, leaving stale cleanup callbacks after Stop #7400

@sahu-virendra-1908

Description

@sahu-virendra-1908

Description

While reviewing the audio effects cleanup flow in Music Blocks, I noticed that _performNotes() in:

  • js/utils/synthutils.js

creates temporary Tone.js effect nodes (Vibrato, Tremolo, Phaser, Chorus, Distortion, etc.) and schedules their cleanup using raw setTimeout() calls.

Current implementation:

setTimeout(
    () => {
        effectsToDispose.forEach(effect => effect.dispose());
        temp_filters.forEach(filter => filter.dispose());

        // Re-establish dry path
        synth.toDestination();
    },
    beatValue * 1000 + 500
);

This cleanup path bypasses the project's centralized ManagedTimer system.

As a result, when playback is stopped through doStopTurtles() and _timerManager.clearAll() runs, these raw cleanup timeouts remain active because they are not tracked by ManagedTimer.

If playback is stopped before all notes complete, pending cleanup callbacks may still execute later against already-disposed synth/effect objects from the previous session.

This can lead to:

  • stale cleanup callbacks firing after Stop
  • cleanup logic running on already-disposed Tone.js nodes
  • retained closures/timers during long playback sessions
  • inconsistent cleanup behavior compared to other ManagedTimer-based systems
  • unexpected audio behavior between playback sessions

Expected Behavior

Effect cleanup timers should participate in the same lifecycle management system as other playback timers.

Stopping playback should:

  • cancel or safely coordinate pending cleanup callbacks
  • avoid callbacks executing against disposed Tone.js nodes
  • fully clean up playback-related timers/effects
  • prevent stale cleanup callbacks from previous sessions

Proof / Observations

The current cleanup logic uses unmanaged raw timers:

setTimeout(...)

instead of _timerManager.setGuardedTimeout() or another ManagedTimer-backed mechanism.

The project already uses centralized timer cleanup through:

_timerManager.clearAll()

but these cleanup timeouts are not registered there.

How to Reproduce

  1. Open Music Blocks

  2. Create a Timbre block using effects such as:

    • Vibrato
    • Tremolo
    • Phaser
    • Chorus
  3. Add many notes using that timbre

  4. Press Play

  5. Press Stop before all notes finish

  6. Start playback again

  7. Inspect pending callbacks/timers using DevTools

Suggested Direction

Replace raw setTimeout() usage with a ManagedTimer-backed timeout so cleanup callbacks participate in the same playback lifecycle management system.

Example direction:

this._timerManager.setGuardedTimeout(
    () => {
        effectsToDispose.forEach(effect => {
            try {
                if (effect) effect.dispose();
            } catch (e) {}
        });

        temp_filters.forEach(filter => {
            try {
                if (filter) filter.dispose();
            } catch (e) {}
        });

        if (synth && typeof synth.toDestination === "function") {
            try {
                synth.toDestination();
            } catch (e) {}
        }
    },
    beatValue * 1000 + 500,
    () => false
);

Checklist

  • I have read and followed the project's code of conduct.
  • I have searched for similar issues before creating this one.
  • I have provided all the necessary information to understand and reproduce the issue.
  • I am willing to contribute to the resolution of this issue.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    Status

    Todo

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions