Skip to content

Commit f1306f1

Browse files
tom7sophiapoirier
authored andcommitted
Fix new vst crash on editor close introduced in r1277. I didn't fully study this but I think we should just turn off midi learning as one of the first orders of business since it makes so many calls. Fixed some typos too.
1 parent 78d33e1 commit f1306f1

File tree

4 files changed

+32
-7
lines changed

4 files changed

+32
-7
lines changed

dfx-library/dfxplugin-vst.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,7 @@ void DfxPlugin::getParameterName(VstInt32 index, char* name)
356356
}
357357

358358
//-----------------------------------------------------------------------------
359-
// numerical display of each parameter's gradiations
359+
// numerical display of each parameter's gradations
360360
void DfxPlugin::getParameterDisplay(VstInt32 index, char* text)
361361
{
362362
if (!text)

dfx-library/dfxplugin.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ void DfxPlugin::do_PreDestructor()
272272
#if TARGET_PLUGIN_HAS_GUI
273273
// The destructor of AudioEffect will delete editor if it's non-null, but
274274
// it looks like DfxGuiEditor wants to be able to still access the effect
275-
// during its own destructor. Maybe it's just wong that it's doing that,
275+
// during its own destructor. Maybe it's just wrong that it's doing that,
276276
// but if not, then we should destroy the editor now before the effect
277277
// instance becomes invalid.
278278
if (auto* e = editor)

dfxgui/dfxguieditor.cpp

+7-5
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,12 @@ void DfxGuiEditor::close()
178178
{
179179
mJustOpened = false;
180180

181+
#if TARGET_PLUGIN_USES_MIDI
182+
// This uses the effect instance and forwards messages all around, so get this done
183+
// before we invalidate anything...
184+
setmidilearning(false);
185+
#endif
186+
181187
CloseEditor();
182188

183189
frame->unregisterMouseObserver(this);
@@ -194,11 +200,7 @@ void DfxGuiEditor::close()
194200
{
195201
frame_temp->forget();
196202
}
197-
198-
#if TARGET_PLUGIN_USES_MIDI
199-
setmidilearning(false);
200-
#endif
201-
203+
202204
TARGET_API_EDITOR_BASE_CLASS::close();
203205
}
204206

notes/notes2020.txt

+23
Original file line numberDiff line numberDiff line change
@@ -598,3 +598,26 @@ problems still:
598598
second click opens another plugin window. I can actually get as many as I want this way. Doesn't
599599
seem like a practical issue without the modals, but maybe we should prevent it from happening?)
600600

601+
602+
== crash on plugin close ==
603+
604+
Bisected. This was introduced in r1277 (thankfully it is not the windows update or something like that...!)
605+
606+
Possible causes?
607+
- some possibly substantive changes in DfxPlugin::getProgramName
608+
- maxparamstrlen + 1 changes in getParameterDisplay could be related to fixed buffer I just noted?
609+
(probably not called on close though?)
610+
- there are some std::find calls etc. that I didn't verify but they didn't seem related
611+
- removed bogus call to setmidilearning(false) in ~DfxGuiEditor(). This is certainly proximal, but how
612+
would *removing* this call cause problems?
613+
... aha, it was moved to the close() call, so this could be it. seems confirmed by tracing.
614+
I guess this must be from some call to close() other than in the destructor, or because we need
615+
to run the base class close() first.
616+
And actually this is a *different* problem than before. This is not getting called during the plugin
617+
destructor (after all we're just closing the editor window), it's just a normal call to close(). But
618+
it looks like something earlier in close() is invalidating something (mControlsList perhaps?) that
619+
setmidilearning (which kicks off a cascade of calls, including some overridable ones) is implicitly
620+
relying on. Seems appropriate to just turn off midi learning as one of the first steps of closing.
621+
Moving the call up fixes the issue so that's good enough for me.
622+
623+

0 commit comments

Comments
 (0)