Skip to content

Fix effect cleanup race condition and unmanaged timers in synthutils #7457

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sahu-virendra-1908 wants to merge 3 commits into
base: master
Choose a base branch
from
+81 −89
Open

Fix effect cleanup race condition and unmanaged timers in synthutils #7457

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
31d49da
fix: prevent recursive traversal from cyclic block graphs
sahu-virendra-1908 May 19, 2026
826ea7f
style: format blocks.js with prettier
sahu-virendra-1908 May 19, 2026
adbd2ea
Fix synth epoch cleanup handling
sahu-virendra-1908 Jun 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 48 additions & 55 deletions js/blocks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@
* @returns {void}
*/
this.extract = () => {
if (this.activeBlock != null) {

Check warning on line 414 in js/blocks.js

View workflow job for this annotation

GitHub Actions / Lint and format-check changed JS files 

Expected '!==' and instead saw '!='
/** Don't extract silence blocks. */
if (this.blockList[this.activeBlock].name !== "rest2") {
this._extractBlock(this.activeBlock, true);
Expand Down Expand Up @@ -439,7 +439,7 @@

let lastConnection = last(blkObj.connections);

if (firstConnection != null) {

Check warning on line 442 in js/blocks.js

View workflow job for this annotation

GitHub Actions / Lint and format-check changed JS files 

Expected '!==' and instead saw '!='
connectionIdx = this.blockList[firstConnection].connections.indexOf(blk);
} else {
connectionIdx = null;
Expand All @@ -447,7 +447,7 @@

blkObj.connections[0] = null;

if (lastConnection != null) {

Check warning on line 450 in js/blocks.js

View workflow job for this annotation

GitHub Actions / Lint and format-check changed JS files 

Expected '!==' and instead saw '!='
/** Is it a hidden block? Keep it attached. */
if (
this.blockList[lastConnection].name === "hidden" ||
Expand All @@ -462,19 +462,19 @@
blkObj.connections[blkObj.connections.length - 1] = null;
}

if (lastConnection != null) {

Check warning on line 465 in js/blocks.js

View workflow job for this annotation

GitHub Actions / Lint and format-check changed JS files 

Expected '!==' and instead saw '!='
this.blockList[lastConnection].connections[0] = firstConnection;
}
}

if (firstConnection != null) {

Check warning on line 470 in js/blocks.js

View workflow job for this annotation

GitHub Actions / Lint and format-check changed JS files 

Expected '!==' and instead saw '!='
this.blockList[firstConnection].connections[connectionIdx] = lastConnection;
}

this.moveStackRelative(blk, 4 * STANDARDBLOCKHEIGHT, 0);
this.blockMoved(blk);

if (adjustDock && firstConnection != null) {

Check warning on line 477 in js/blocks.js

View workflow job for this annotation

GitHub Actions / Lint and format-check changed JS files 

Expected '!==' and instead saw '!='
this.adjustDocks(firstConnection, true);
if (clampList.length > 0) {
this.clampBlocksToCheck = clampList;
Expand All @@ -482,7 +482,7 @@
}
}
} else {
if (firstConnection != null) {

Check warning on line 485 in js/blocks.js

View workflow job for this annotation

GitHub Actions / Lint and format-check changed JS files 

Expected '!==' and instead saw '!='
connectionIdx = this.blockList[firstConnection].connections.indexOf(blk);
this.blockList[firstConnection].connections[connectionIdx] = null;
blkObj.connections[0] = null;
Expand Down Expand Up @@ -664,7 +664,7 @@

that._sizeCounter = 0;
let childFlowSize = 1;
if (c > 0 && myBlock.connections[c] != null) {

Check warning on line 667 in js/blocks.js

View workflow job for this annotation

GitHub Actions / Lint and format-check changed JS files 

Expected '!==' and instead saw '!='
this._sizeCounter = 0;
childFlowSize = Math.max(that._getStackSize(myBlock.connections[c]), 1);
}
Expand Down Expand Up @@ -747,7 +747,7 @@
for (let i = 0; i < slotList.length; i++) {
const c = myBlock.connections[ci + i];
let size = 1; /** Minimum size */
if (c != null) {

Check warning on line 750 in js/blocks.js

View workflow job for this annotation

GitHub Actions / Lint and format-check changed JS files 

Expected '!==' and instead saw '!='
size = Math.max(this._getBlockSize(c), 1);
}

Expand Down Expand Up @@ -781,7 +781,7 @@
/** Determine the size of the first argument. */
const c = myBlock.connections[1];
let firstArgumentSize = 1; /** Minimum size */
if (c != null) {

Check warning on line 784 in js/blocks.js

View workflow job for this annotation

GitHub Actions / Lint and format-check changed JS files 

Expected '!==' and instead saw '!='
firstArgumentSize = Math.max(this._getBlockSize(c), 1);
}

Expand Down Expand Up @@ -3872,50 +3872,30 @@
* @private
* @returns {void}
*/
this._calculateDragGroup = blk => {
this.dragLoopCounter += 1;
if (this.dragLoopCounter > this.blockList.length) {
console.debug(
"Maximum loop counter exceeded in calculateDragGroup... this is bad. " + blk
);
return;
}

if (blk == null) {
console.debug("null block passed to calculateDragGroup");
return;
}

const myBlock = this.blockList[blk];
/** If this happens, something is really broken. */
if (myBlock == null) {
console.debug("null block encountered... this is bad. " + blk);
return;
}

/** As before, does these ever happen? */
if (myBlock.connections == null) {
this.dragGroup = [blk];
return;
}

/** Some malformed blocks might have no connections. */
if (myBlock.connections.length === 0) {
this.dragGroup = [blk];
return;
}

this.dragGroup.push(blk);

for (let c = 1; c < myBlock.connections.length; c++) {
const cblk = myBlock.connections[c];
if (cblk != null) {
/** Recurse */
this._calculateDragGroup(cblk);
this._calculateDragGroup = startBlk => {
// Iterative BFS — eliminates recursive stack overflow risk
// for deeply chained or cyclic block structures.
const visited = new Set();
const queue = [startBlk];
while (queue.length > 0) {
const blk = queue.shift();
if (blk == null || visited.has(blk)) continue;
visited.add(blk);
const myBlock = this.blockList[blk];
if (myBlock == null) continue;
if (myBlock.connections == null || myBlock.connections.length === 0) {
this.dragGroup.push(blk);
continue;
}
this.dragGroup.push(blk);
for (let c = 1; c < myBlock.connections.length; c++) {
const cblk = myBlock.connections[c];
if (cblk != null && !visited.has(cblk)) {
queue.push(cblk);
}
}
}
};

/**
* Set protoblock visibility on the Action palette.
* @param - state
Expand Down Expand Up @@ -5573,20 +5553,33 @@
);
}

/** Check for blocks connected to themselves, */
/** and for action blocks not connected to text blocks. */
for (let b = 0; b < blockObjs.length; b++) {
const blkData = blockObjs[b];

for (const c in blkData[4]) {
if (blkData[4][c] === blkData[0]) {
console.debug("Circular connection in block data: " + blkData);

console.debug("Punting loading of new blocks!");

console.debug(blockObjs);
return;
}
// Check for BOTH self-loops AND multi-block cycles using DFS
const adjacency = new Map();
for (const bd of blockObjs) {
if (Array.isArray(bd[4])) {
adjacency.set(
bd[0],
bd[4].filter(c => c !== null && c !== undefined)
);
}
}
const _visited = new Set();
const _recStack = new Set();
const _hasCycle = node => {
if (_recStack.has(node)) return true;
if (_visited.has(node)) return false;
_visited.add(node);
_recStack.add(node);
for (const neighbor of adjacency.get(node) || []) {
if (_hasCycle(neighbor)) return true;
}
_recStack.delete(node);
return false;
};
for (const [node] of adjacency) {
if (_hasCycle(node)) {
console.error("Cycle detected in block connection data — refusing to load.");
return;
}
}

Expand Down
67 changes: 33 additions & 34 deletions js/utils/synthutils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1845,6 +1845,7 @@ function Synth() {
);
chainNodes.push(vibrato);
effectsToDispose.push(vibrato);
this._trackPendingEffect(vibrato);
}

if (paramsEffects.doDistortion) {
Expand Down Expand Up @@ -1960,46 +1961,44 @@ function Synth() {
}
}

// Schedule cleanup after the note duration.
// A 500 ms safety buffer is added beyond the note duration to prevent
// premature disposal caused by audio-clock drift or scheduler jitter,
// which would otherwise produce crackling artefacts in long sessions.
setTimeout(
const capturedEpoch = this._instrumentEpoch;
this._timerManager.setGuardedTimeout(
() => {
try {
// Dispose of effects
// Check epoch: if instruments were disposed, skip cleanup on dead synth
if (this._instrumentEpoch !== capturedEpoch) {
// Instruments disposed — still need to dispose the Tone effect nodes
effectsToDispose.forEach(effect => {
if (effect && typeof effect.dispose === "function") {
effect.dispose();
}
try {
if (effect) effect.dispose();
} catch (e) {}
});

// Dispose of filters
if (temp_filters.length > 0) {
temp_filters.forEach(filter => {
if (filter && typeof filter.dispose === "function") {
filter.dispose();
}
});
}

// Re-establish the dry path only when no other effects chain
// is still active on this synth; otherwise the direct
// connection would bypass the in-flight chain.
const remaining = (_effectsInFlight.get(synth) || 1) - 1;
_effectsInFlight.set(synth, remaining);
if (
remaining === 0 &&
synth &&
typeof synth.toDestination === "function"
) {
temp_filters.forEach(filter => {
try {
if (filter) filter.dispose();
} catch (e) {}
});
return;
}
effectsToDispose.forEach(effect => {
try {
if (effect) effect.dispose();
} catch (e) {}
});
temp_filters.forEach(filter => {
try {
if (filter) filter.dispose();
} catch (e) {}
});
const remaining = (_effectsInFlight.get(synth) || 1) - 1;
_effectsInFlight.set(synth, remaining);
if (remaining === 0 && synth && typeof synth.toDestination === "function") {
try {
synth.toDestination();
}
} catch (e) {
console.debug("Error disposing effects:", e);
} catch (e) {}
}
},
beatValue * 1000 + 500
beatValue * 1000 + 500,
() => false // never auto-abort: cleanup must always run eventually
);
}
} catch (e) {
Expand Down
Loading