My plug-in crashed on starting or closing GUI - usually once per 100 or 200 launches. It was not easy to find the reason because the crash happened rarely. Finally I found the reason and I want to share it to prevent others from repeating my mistake.
My ProcessBlock() function contained the following innocent looking string:
if (GetUI()) uPeakMeterHoriz->SetValueFromUserInput(vumeter_level);
The problem of this string lies in the fact that the GetUI() function may return “true” before initialization of all ‘graphical’ controls or after destructing them. Consequently it cannot be used to reliably determine whether or not a graphical control is ready to receive a command. The ProcessBlock() function already runs when we start the graphical interface or close it, so ProcessBlock() may sometimes send a command to a graphical control when GetUI() returns “true” but the graphical control is not ready to receive the command.
I solved this problem as follows:
uPeakMeterHoriz = NULL; //added to the beginning of the constructor
uPeakMeterHoriz = NULL; //added to the OnUIClose() function
if (GetUI() && uPeakMeterHoriz != NULL) uPeakMeterHoriz->SetValueFromUserInput(vumeter_level); //changed in the ProcessBlock() function
I ran into this problem a long time ago and solved it in a slightly different way. I assumed the DAW constructed the plugin - UI and all - and THEN loaded the values. But I found that some DAWs construct the plugin in random order and I couldn’t rely on the controls being there before the code tried to update them. At least that’s what I recall. I think Ableton or FL Studio is where I came across the issue.
I solved it (right or wrong) by adding a global bool variable I call “controlsready” that I initialize to FALSE but then set to TRUE as the very last line of the mLayoutFunc. I then check for “controlsready” in OnParameterChangeUI before updating any controls:
if (GetUI() && controlsready)
It may seem obscure or unnecessary but it fixed the problem and there’s no harm in it AFAIK.
Your method is more convenient because it can work with any control, but you may want to additionally override the OnUIClose() function and add there the controlsready=FALSE; statement. This method helped me to prevent rare crashes of Reaper on closing plug-in’s UI (Windows 10).
I may be misunderstanding your specific situation, but as far as I know it is generally not okay to access user interface controls from anywhere inside the ProcessBlock function.
That is because the ProcessBlock function runs on the audio thread, whereas creation and updating of the user interface happens on another thread. This separation is necessary for the performance and latency demands of audio code.
Since user interface and audio run on different threads, it is necessary to use a proper synchronization mechanism to ensure that communication between them doesn’t result in race conditions, which might cause all sorts of rare and weird bugs, not only limited to the crashes you encounter.
If I recall correctly, some iPlug example projects come with example code that shows one way to do this.
I solve it a bit differently in my own code:
Data from audio processing that needs to be communicated to the user interface (eg. a volume level) is stored in a struct that lives on my plugin class (the one deriving from Plugin), so that the data is guaranteed to exist during both ProcessBlock and any UI code being executed.
And, very important, every access (from both ProcessBlock and UI code!) is guarded by locking an std::mutex.
I also can’t recommend using any global variables to solve this problem. Remember that a DAW may host multiple instances of your plugin in a single process, so a global variable will be shared among them.
Putting any state needed for synchronization/communication between audio and UI on your Plugin class should be the most sensible solution.
You are right, the ProcessBlock() is not an ideal place for using mycontrol->SetValueFromUserInput() or mycontrol->SetValueFromDelegate(). Could you suggest a better place? I need to change the graphical position of a few controls from within the ProcessBlock() function.
The IPlugControls example demonstrates the use of the OnIdle() function for this purpose. The problem of the OnIdle() function lies in the fact that it starts working before the creation of the graphical controls. Using mycontrol->SetValueFromUserInput() in the OnIdle() function means using a control which doesn’t exist yet. This may result in the plug-in crash.
If you or anyone else can suggest a better solution, I would be grateful. At this moment I use the described solution because I don’t see better alternatives.
Yes, the OnIdle() function is called on the UI thread so that’s where I do all of my UI updating.
It can get called even when the UI and any controls don’t exist yet, so first I simply check for existence of the UI using GetUI() and for validity of all the controls I created.
Since it runs on the UI thread, it’s perfectly valid to check these things there.
The most problematic part of your method is checking the validity of the control. A simple expression like if(uPeakMeterHoriz != nullptr) doesn’t work because IPlug2 doesn’t reset controls after their destruction. There is a time interval when uPeakMeterHoriz is already destroyed but the expression (GetUI() && uPeakMeterHoriz != nullptr) is true. It may cause the plug-in crash on closing the GUI. Have you found the solution to this problem?
I solve this problem by means of resetting the control in the OnUIClose() function. This is a “dirty” method, but it helps to avoid rare plug-in crashes on closing the GUI. Is there a better solution?
Hmmm.
You’re right that iPlug2 doesn’t reset controls after their destruction (it can’t, really), so doing that in the OnUIClose function should just work.
I think this is the relevant code in the framework (comments added by me):
void IGEditorDelegate::CloseWindow()
{
if (!mClosing)
{
mClosing = true;
IEditorDelegate::CloseWindow(); // will also call OnUIClose on your plugin
if (mGraphics)
{
mLastWidth = mGraphics->Width();
mLastHeight = mGraphics->Height();
mLastScale = mGraphics->GetDrawScale();
mGraphics->CloseWindow(); // will also destroy all controls
mGraphics = nullptr; // finally resets pointer (is what GetUI() returns)
}
mClosing = false;
}
}
Since it’s all happening in this one function, your code running exclusively on the UI thread (ie in OnIdle, OnUIClose, ..) shouldn’t be able to observe a situation where your control is already destroyed, yet GetUI() still returns a non-null pointer.
Note that the audio thread could observe this in rare cases, but it’s anyway a mistake for audio thread code to directly access UI like that.
I would say resetting your control pointers in OnUIClose() is perfectly fine!
Thank you for your comment, it makes the situation very clear.
I eventually settled on the following solution: bool controls_ready; //added to the private part of the plug-in definition controls_ready = false; //added to the beginning of the plug-in constructor controls_ready = true; // added as the very last line of the mLayoutFunc controls_ready = false; //added to the OnUIClose() function if (GetUI() && controls_ready) uPeakMeterHoriz->SetValueFromDelegate(vumeter_level); //added to the OnIdle() function
So far everything works fine.