Merge pull request 'Bug Fix: Fix race condition between worklet termination and port messages' (#1897) from glossing/diedack into main

Reviewed-on: https://codeberg.org/uzu/strudel/pulls/1897

packages/superdough/nodePools.mjs

@@ -7,11 +7,13 @@ This program is free software: you can redistribute it and/or modify it under th
 
 const nodePools = new Map();
 const POOL_KEY = Symbol('nodePoolKey');
-const IS_WORKLET_DEAD = Symbol('nodePoolIsWorkletDead');
-const MAX_POOL_SIZE = 64;
 
 export const isPoolable = (node) => !!node[POOL_KEY];
 
+const getNodeTime = (node) => {
+  return node.context?.currentTime ?? 0;
+};
+
 const getParams = (node) => {
   const params = new Set();
   node.parameters?.forEach((param) => params.add(param));
@@ -38,34 +40,43 @@ export const releaseNodeToPool = (node) => {
     // not reusable
     return;
   }
-  if (node[IS_WORKLET_DEAD]) {
-    // Worklet already terminated, don't pool it
-    return;
-  }
   const key = node[POOL_KEY];
   if (key == null) return;
-  const now = node.context?.currentTime ?? 0;
+  const now = getNodeTime(node);
   getParams(node).forEach((param) => param.cancelScheduledValues(now));
   const pool = nodePools.get(key) ?? [];
-  if (pool.length < MAX_POOL_SIZE) {
-    pool.push(new WeakRef(node));
-    nodePools.set(key, pool);
-  }
+  pool.push(new WeakRef(node));
+  nodePools.set(key, pool);
 };
 
-export const markWorkletAsDead = (worklet) => (worklet[IS_WORKLET_DEAD] = true);
+// Audio worklets are given a grace period to survive (`return true`) after
+// being released. This concludes at time `end + 0.5`. We test here whether we are
+// within some safe distance of that (`end + 0.45`) and if so, permit the node to be
+// released. This helps to prevent race conditions between node termination and node
+// re-use
+const isNodeAlive = (node) => {
+  // Skip check if node is not a worklet
+  if (!(node instanceof AudioWorkletNode)) return true;
+  const now = getNodeTime(node);
+  const end = node?.parameters?.get('end').value ?? 0;
+  return now < end + 0.45;
+};
 
 // Attempt to get node from the pool. If this fails, fall back
 // to building it with the factory
 export const getNodeFromPool = (key, factory) => {
   const pool = nodePools.get(key) ?? [];
   let node;
+  let found = false;
   while (pool.length) {
     const ref = pool.pop();
     node = ref?.deref();
-    if (node != null && !node[IS_WORKLET_DEAD]) break;
+    if (node != null && isNodeAlive(node)) {
+      found = true;
+      break;
+    }
   }
-  if (node == null || node[IS_WORKLET_DEAD]) {
+  if (!found) {
     node = factory();
   }
   node[POOL_KEY] = key;

packages/superdough/synth.mjs

@@ -18,7 +18,7 @@ import {
 } from './helpers.mjs';
 import { logger } from './logger.mjs';
 import { getNoiseMix, getNoiseOscillator } from './noise.mjs';
-import { getNodeFromPool, markWorkletAsDead, releaseNodeToPool } from './nodePools.mjs';
+import { getNodeFromPool, releaseNodeToPool } from './nodePools.mjs';
 
 const waveforms = ['triangle', 'square', 'sawtooth', 'sine', 'user', 'one'];
 const waveformAliases = [
@@ -185,10 +185,6 @@ export function registerSynthSounds() {
         param.setValueAtTime(target, now);
       });
       o.port.postMessage({ type: 'initialize' });
-      o.port.onmessage = (e) => {
-        if (e.data.type === 'died') markWorkletAsDead(o);
-        o.port.onmessage = null;
-      };
       const gainAdjustment = 1 / Math.sqrt(voices);
       getPitchEnvelope(o.parameters.get('detune'), value, begin, holdend);
       const vibratoHandle = getVibratoOscillator(o.parameters.get('detune'), value, begin);

packages/superdough/wavetable.mjs

@@ -11,7 +11,7 @@ import {
   webAudioTimeout,
   releaseAudioNode,
 } from './helpers.mjs';
-import { getNodeFromPool, markWorkletAsDead, releaseNodeToPool } from './nodePools.mjs';
+import { getNodeFromPool, releaseNodeToPool } from './nodePools.mjs';
 import { logger } from './logger.mjs';
 
 export const Warpmode = Object.freeze({
@@ -251,10 +251,6 @@ export async function onTriggerSynth(t, value, onended, tables, cps, frameLen) {
     param.setValueAtTime(target, now);
   });
   source.port.postMessage({ type: 'initialize', payload });
-  source.port.onmessage = (e) => {
-    if (e.data.type === 'died') markWorkletAsDead(source);
-    source.port.onmessage = null;
-  };
   if (ac.currentTime > t) {
     logger(`[wavetable] still loading sound "${s}:${n}"`, 'highlight');
     return;

packages/superdough/worklets.mjs

@@ -463,7 +463,6 @@ registerProcessor('distort-processor', DistortProcessor);
 class SuperSawOscillatorProcessor extends AudioWorkletProcessor {
   constructor() {
     super();
-    this.isAlive = true; // used internally to prevent multiple death messages
     this.port.onmessage = (e) => {
       const { type, payload } = e.data || {};
       if (type === 'initialize') {
@@ -479,16 +478,16 @@ class SuperSawOscillatorProcessor extends AudioWorkletProcessor {
     return [
       {
         name: 'begin',
-        defaultValue: 0,
+        defaultValue: -1,
         max: Number.POSITIVE_INFINITY,
-        min: 0,
+        min: -1,
       },
 
       {
         name: 'end',
-        defaultValue: 0,
+        defaultValue: -1,
         max: Number.POSITIVE_INFINITY,
-        min: 0,
+        min: -1,
       },
 
       {
@@ -523,16 +522,17 @@ class SuperSawOscillatorProcessor extends AudioWorkletProcessor {
     ];
   }
   process(_input, outputs, params) {
-    if (currentTime >= params.end[0] + 0.5) {
-      // Outside of grace period - should terminate
-      if (this.isAlive) {
-        this.port.postMessage({ type: 'died' });
-        this.isAlive = false;
-      }
+    const begin = params.begin[0];
+    const end = params.end[0];
+    const beginDefined = begin >= 0;
+    const endDefined = end >= 0;
+    // We give a 0.5s grace period (for node pooling) before termination
+    const shouldTerminate = endDefined && currentTime >= end + 0.5;
+    const ended = endDefined && currentTime >= end;
+    const notStarted = currentTime <= begin;
+    if (shouldTerminate) {
       return false;
-    }
-    if (currentTime >= params.end[0] || currentTime <= params.begin[0]) {
-      // Inside of grace period or not yet started
+    } else if (ended || notStarted || !beginDefined) {
       return true;
     }
     const output = outputs[0];
@@ -1148,8 +1148,8 @@ const tablesCache = {};
 class WavetableOscillatorProcessor extends AudioWorkletProcessor {
   static get parameterDescriptors() {
     return [
-      { name: 'begin', defaultValue: 0, min: 0, max: Number.POSITIVE_INFINITY },
-      { name: 'end', defaultValue: 0, min: 0, max: Number.POSITIVE_INFINITY },
+      { name: 'begin', defaultValue: -1, min: -1, max: Number.POSITIVE_INFINITY },
+      { name: 'end', defaultValue: -1, min: -1, max: Number.POSITIVE_INFINITY },
       { name: 'frequency', defaultValue: 440, min: Number.EPSILON },
       { name: 'detune', defaultValue: 0 },
       { name: 'freqspread', defaultValue: 0.18, min: 0 },
@@ -1164,7 +1164,6 @@ class WavetableOscillatorProcessor extends AudioWorkletProcessor {
 
   constructor(options) {
     super(options);
-    this.isAlive = true; // used internally to prevent multiple death messages
     this.port.onmessage = (e) => {
       const { type, payload } = e.data || {};
       if (type === 'initialize') {
@@ -1332,16 +1331,17 @@ class WavetableOscillatorProcessor extends AudioWorkletProcessor {
   }
 
   process(_inputs, outputs, parameters) {
-    if (currentTime >= parameters.end[0] + 0.5) {
-      // Outside of grace period - should terminate
-      if (this.isAlive) {
-        this.port.postMessage({ type: 'died' });
-        this.isAlive = false;
-      }
+    const begin = parameters.begin[0];
+    const end = parameters.end[0];
+    const beginDefined = begin >= 0;
+    const endDefined = end >= 0;
+    // We give a 0.5s grace period (for node pooling) before termination
+    const shouldTerminate = endDefined && currentTime >= end + 0.5;
+    const ended = endDefined && currentTime >= end;
+    const notStarted = currentTime <= begin;
+    if (shouldTerminate) {
       return false;
-    }
-    if (currentTime >= parameters.end[0] || currentTime <= parameters.begin[0]) {
-      // Inside of grace period or not yet started
+    } else if (ended || notStarted || !beginDefined) {
       return true;
     }
     const outL = outputs[0][0];