Bug: static variables initializing things in audio time - simultaneously

[codecollider]

Hi,

there's an issue with the jc303 plugin in Renoise ans most likely other DAWs when more than one instances are used which sometimes results in wrong settings of parameters.

I copy paste the text from taktik (Renoise developer) here, so you can have a look:

This is a bug in the plugin that happens when multiple instances of the plugin are involved. Loading it with only one audio CPU in Renoise will work fine, but that’s not a fix.

A quick look at their code shows that there’s at least one problem here. They are using static variables to initialize things in audio time - simultaneously:

`

  case   TRIANGLE:  fillWithTriangle();    break;
  case   SQUARE:    fillWithSquare();      break;
  case   SAW:       fillWithSaw();         break;
  case   SQUARE303: fillWithSquare303();   break;
  case   SAW303:    fillWithSaw303();      break;

  default :  fillWithSine();
  }
}

**void MipMappedWaveTable::generateMipMap()**
{
  static double spectrum[tableLength];
  //static int    position, offset;
  static int t, i; // indices for the table and position

  //position = 0;             // begin of the 1st table (index 0)
  //offset   = tableLength+4; // offset between tow tables, the 4 is the number
  // of additional samples used for interpolation

  // copy the prototypeTable into the 1st table of the mipmap (this actually makes the

`

You can’t use static variables in that context. This code can run asynchronously in audio time from multiple threads at the same time. So multiple plugin instances will be using the same temporary state at the same time. This will break sooner or later.

The crucial line is " void MipMappedWaveTable::generateMipMap()"

[midilab]

Hi @codecollider ,

Thanks for reporting this bug.

This part of the code is from Open303 DSP source code wich is not maintained by jc303, i will bring this concern to the main developer of Open303 and see what he says.

This project just use Open303 as library so we have to deal with this on the main Open303 project: https://github.com/RobinSchmidt/Open303

I will try to bridge this bug with the main developer of Open303 and get back to you, fixing on the main source code will fix it on the JC303 too.

[RobinSchmidt]

Oh! Interesting! If I remember correctly, this "spectrum" array is used as an internal buffer to hold the spectrum of the prototype waveform and then do the spectral truncation before the iFFT to produce lowpassed, mip-mapped versions of the waveform for anti-aliasing. It's basically a temporary workspace for the algorithm that produces the mip-map. I think my rationale in declaring it static was precisely to avoid memory allocations that I apparently deemed unnecessary at that time. Would it help to just remove the "static" qualifiers? That would imply that we would use a fresh (stack-allocated) array in each call. I actually do not like such "big" stack allocations, though. I might think about giving the class a heap-allocated buffer for this temporary "spectrum" (e.g. use std::vector or something). Or wait - looking at the code - the class already has a lot of buffers as old-school C-arrays. I might add a temporary "spectrum" buffer as another such C-array to the class. I really need to make this change in my main codebase, too. This MipMappedWaveTable class is also used in Straightliner, for example. Good catch! I should really review my whole codebase for similar things!

So - yeah - I'd say, for a quick fix, maybe just try to remove those problematic "static" qualifiers. I do not think that the buffers (of 2048 doubles) should get us anywhere near "stack-overflow" territory in any context. ...but yeah - I guess the better fix would be to introduce another class member for the "spectrum" buffer. Although that would increase the memory footprint of the object...so maybe a stack-allocated buffer would be preferable, after all? I'm not sure.