VCV Rack 2 crashes - SOLVED

Thank you all :pray:.

To recover my VCV file and my VCV Rack :slight_smile:

  • I suppressed the plateau module in my library (on line library)
  • I suppressed the related directory on my PC
  • I started VCV Rack.
  • It just told my a module was missing. And it works!

I’ll report a bug… Alain.

It’s one of these:

void OnePoleLPFilter::setCutoffFreq(double cutoffFreq)
//...
    assert(cutoffFreq > 0.0);
    assert(cutoffFreq <= _maxCutoffFreq);

The question is how the cutoffFreq is going out of range, or the _maxCutoffFreq is getting set badly (it’s derived from sampleRate: sampleRate / 2.0 - 1.0).

Just browsing the code randomly, it looks like this might happen if you have a sample rate < 20khz Default cutoff it sets is double inputHighCut = 10000.0;, so if you’re sample rate is < 20k this will assert initializing the cutoff with this default. But, I haven’t attempted to reproduce that.

So maybe don’t try to use Plateau to do lofi. (that’s really just a weird joke).

2 Likes

Editorializing: the rack developers guide should have a 300 point bold face statement at the top saying “asserts are on in production”.

2 Likes

Unless your makefile overrides it so the aren’t. That’s what I always did. Happens all the time where I work. “what idiots decided to make an assert be a crash in release?”

asserts turned on in production are good.

It’s better to crash than have undefined behavior. Very often that undefined behavior will crash anyway, or cause other damage (in some software, these can become a security exploit). These crashes or bad behavior later happen in a way that is far harder to debug after-the-fact from logs. These very easily become eternal mysteries that don’t get fixed. The assert is at a known location for a known bad condition you didn’t code for, and need to fix if you ever run into it.

assert says: This is something that must be true for successful operation. It’s a precondition of your contract. If you have something you want to detect that isn’t at that level, then you want a debug trace/warning or conditional breakpoint rather than an assert.

1 Like

But I do agree that the developer’s guide should tell you that asserts are on in production. Even better if it explains the philosophy for it, because it can be an unexpected (and sometimes unpleasant) surprise for many that have different programming backgrounds.

Yeah, yeah, I know all that stuff. It’s just a matter of opinion. Smart ppl will never agree. And you are wrong, btw,

Might very well be related to this one (@valley.audio):

If so a fix should appear in the library pretty soon.

I wanted to provoke a discussion and hear the reason why it perhaps should be something else, rather than simply a dismissal.

A normal assert is a bit too broad of a hammer.

Doing surprising things in a framework is not a great idea, especially when you do not document such framework, but at least we have the source and the community to make it livable. It is a fact that standard VCV Rack builds from the open source are basically debug builds with optimization turned on, which is unfamiliar to many devs. It’s not an uncommon practice for internal software, where crashing with a readable stack trace shortens the time to fix an issue. For retail consumer products, that’s not really viable.

What’s needed is a better granularity: the normal assert that goes away in release builds as most are familiar with, and the ship_assert which does not, for those hard-core cases where it’s better to crash than soldier on and hope for the best. This is what Windows does. The ship-assert in the Windows kernel produces a blue screen (as do certain hardware faults). ship-asserts in other layers will do other things (like stop and restart a service).

This thread, where someone can quickly debug an issue in unfamiliar code shows there is some value for shipping (some kind of) asserts. Without the crash and trace, you’d just have “something sounds wrong” or “it doesn’t work”.

1 Like

BSOD is a good example. Is there anything a plugin could do that would warrant a BSOD? Probably not.

Often as programmers we think “I really want to know if this goes wrong. I’ll put in an assert that stays in the release build.” But as product people we usually don’t want that to happen.

For example, a few years ago my boss came to me and said “we are getting 1000 of these failures to load a document every day. Please fix it.” It turned out that it was hitting an assertion in a second party serialization framework. So we asked them to fix it - they fixed the assertion so it would not fire spuriously. The second time this happened we told them to take out all release assertions. Problem solved.

This is complicated a bit by the fact that VCV does not have debug and release builds. Many programmers have been using a system like Visual Studio where there are debug and release builds, and the release build does not have assertions. Unless a module author is really looking at this they may not even realize that assertions will be live in the “shipping version”.

In this specific instance is it good there is an assertion in there? judgement call. I would prefer that VCV not crash, and that my plugin just not send audio (or whatever this filter would do in that condition). I have the exact same assertion in many of my filters, as they would put out NaN in that case. But I don’t have an assertions in the VCV library version. I would prefer that a bug in my plugin does not crash VCV. But of course it’s a plausible choice to put assertions in your shipping plugin. Hopefully people have really thought it out and a) know that’s what they are doing. and b) have really considered whether crashing VCV is worth it.

1 Like

I understand your argument Paul. My two arguments for assert being off in production are:

  1. that’s the de-facto contract for assert. release builds define NDEBUG most places. So at least a screaming big warning that assert is not safe in rack. For instance, this code is very useful
   assert(param[FOO].getValue() < 3);
   if (param[FOO].getValue() < 3)
   {
      outputs[OUT].setVoltage(0);
      return;
   }

I bet if you asked 100 C++ devs what that would do with the param valued 1 in a production build, 99 would say “outputs 0”, and 1 would say “outputs 0 as long as your release build defines NDEBUG according to the assert spec”

  1. as a result, idiomatically you may put relatively expensive checks in an assert knowing they are gone at release time. So for instance, you may assert something every sample which you know to be true but leave the assert in there for a debug session.

therefore assert being on in production gives a counter-intuitive result that your code can be slower and can std::terminate

If the error when std::terminal was better for the users (like, it saved your patch and showed the message) that would be fine! but it isn’t.

Next time I update my plugs all add a $<$<CONFIG:Release>:-DNDEBUG> if I don’t have it already.

1 Like

Ah, okay, so it looks like these random crashes I’ve been having reports on (and failing to replicate) are my assertions still being present in my Rack module code?

I’m so used to how Xcode and Visual Studio operate, I’ve gotten complacent with knowing my assertions in the projects will be stripped out when compiling a Release build.

Edit: Just realised that if assertions are being tripped, then obviously there’s something up. Urgh, I’m sick of whenever I make a seemingly useful improvement to my code, I end up breaking things anyway, even though they pass my own tests.

2 Likes

Yeah

Add ndebug to your make file

I think I’ll add an assert false to surge startup just to make sure I don’t forget! I can comment it out when debugging

1 Like

This is a case of the Rack build setup violating the principle of least surprise.

This discussion made me realize I should fix the makefile for my Generic Blank repo to line up with this common understanding for debug/release builds. It’s currently the stock Rack module makefile. I’d be happy to take PRs if you have a good model for rack plugin builds.

(Generic Blank is my alternaive to the Rack tutorial, which IMO leads people down a bad path in a couple of ways.)

With qnos help I moved all my things to cmake where it is easy (and where I can get reliable debug builds and step debug in CLion). But is t it really just flags+= -DNDEBUG=1 in make land?

I think you really want separate build targets for debug and release. Just setting that flag will disable the asserts for all builds.

Totally agree. Can regular rack makefile do that today?

In cmake I can do it easily like i mentioned.though. So will when I open up. Y rack projects again :slight_smile:

It should be a simple matter to add targets.

I rewrote the manual for NMAKE back in the early 90’s but I’ve forgotten almost everything.

I know zip about CMake, and it’s not the default setup for Rack. In the few CMAKE projects I’ve seen, I find tons of generated files littering the folders with impenetrable gobbledegook in them.

1 Like

from my ancient make file:

# Macro to use on any target where we don't normally want asserts
ASSERTOFF = -D NDEBUG

# Make _ASSERT=true will nullify our ASSERTOFF flag, thus allowing them
ifdef _ASSERT
ASSERTOFF =
endif

# This turns asserts off for make (plugin), not for test or perf
$(TARGET) : FLAGS += $(ASSERTOFF)
3 Likes

Done for the report (a little bit late). Will tel you!