Strip - fixed crash when removing modules added by STRIP in plugin-version of Rack

Author: stoermelder

CHANGELOG.md

@@ -18,6 +18,8 @@
     - Fixed saving of SysEx data logging setting
 - Module [PANIC ROOM](./docs/panicroom/PanicRoom.md)
     - Added options to limit the number of allowed modules and cables
+- Module [STRIP](./docs/strip/Strip.md)
+    - Fixed crash when removing modules added by STRIP in plugin-version of Rack
 - Module [TRANSIT](./docs/transit/Transit.md)
     - Fade CV input is now additive to per-slot fade time (previously CV was only additive to the global _FADE_ knob)
     - Added Output-mode "Tipsy" for sending the snapshot text label (for modules with Tipsy-support like [TTY](https://library.vcvrack.com/StochasticTelegraph/TTY))

docs/strip/Strip.md

@@ -105,4 +105,6 @@ STRIP-BAY is a companion module for STRIP: It is used to name some "patching-poi
 - v2.2.0
     - Remember and recall file-dialog folder locations for vcvss and vcvs files
 - v2.3.1
-    - Fixed changed behavior on excluded/included parameters for randomization introduced in v2.3.0
+    - Fixed changed behavior on excluded/included parameters for randomization introduced in v2.3.0
+- v2.5.0
+    - Fixed crash when removing modules added by STRIP in plugin-version of Rack

src/modules/strip/Strip.hpp

@@ -1,6 +1,7 @@
 #pragma once
 #include "../../plugin.hpp"
 #include "../../utils/StripIdFixModule.hpp"
+#include "../../utils/vcv_files.hpp"
 #include <osdialog.h>
 #include <plugin.hpp>
 
@@ -679,6 +680,9 @@ struct StripWidgetBase : ThemedModuleWidget<MODULE> {
 						StripIdFixModule* m = dynamic_cast<StripIdFixModule*>(mw->module);
 						if (m) m->idFixDataFromJson(modules);
 
+						// Drop engine-runtime binding fields, like Rack's own preset loader.
+						vcv::jsonStripIds(moduleJ);
+
 						mw->fromJson(moduleJ);
 
 						h->newModuleJ = mw->toJson();
@@ -706,6 +710,9 @@ struct StripWidgetBase : ThemedModuleWidget<MODULE> {
 						StripIdFixModule* m = dynamic_cast<StripIdFixModule*>(mw->module);
 						if (m) m->idFixDataFromJson(modules);
 
+						// Drop engine-runtime binding fields, like Rack's own preset loader.
+						vcv::jsonStripIds(moduleJ);
+
 						mw->fromJson(moduleJ);
 
 						h->newModuleJ = mw->toJson();
@@ -738,6 +745,9 @@ struct StripWidgetBase : ThemedModuleWidget<MODULE> {
 				StripIdFixModule* m = dynamic_cast<StripIdFixModule*>(mw->module);
 				if (m) m->idFixDataFromJson(modules);
 
+				// Drop engine-runtime binding fields, like Rack's own preset loader.
+				vcv::jsonStripIds(moduleJ);
+
 				mw->fromJson(moduleJ);
 
 				h->newModuleJ = mw->toJson();

src/utils/vcv_files.hpp

@@ -9,6 +9,26 @@ namespace vcv {
 
 static const char SELECTION_FILTERS[] = "VCV Rack module selection (.vcvs):vcvs";
 
+/**
+ * Removes engine-runtime binding fields from a module's JSON before it is applied
+ * to an already-created module with ModuleWidget::fromJson(). Mirrors Rack's own
+ * engine::Module::jsonStripIds(), which every Rack preset/clipboard load calls
+ * before fromJson(). Restoring these fields would transplant another instance's
+ * bindings onto the new module; in particular a stale "automId" desyncs the module
+ * from the engine's host-automation map (automModuleMap) and causes a use-after-free
+ * in getAutomParamQuantity() on the next module removal.
+ * NB: unlike Rack's version, "id" is intentionally kept - callers read it for id
+ * remapping (StripIdFixModule) and rely on it.
+ */
+#if defined(__GNUC__) || defined(__clang__)
+__attribute__((unused))
+#endif
+static void jsonStripIds(json_t* moduleJ) {
+    json_object_del(moduleJ, "leftModuleId");
+    json_object_del(moduleJ, "rightModuleId");
+    json_object_del(moduleJ, "automId");
+}
+
 /**
  * Creates a module from JSON data, also returns the previous id of the module.
  * @param moduleJ JSON representation of the module
@@ -282,6 +302,9 @@ static std::vector<history::Action*>* vcvsFromJson_presets(json_t* rootJ, std::m
             StripIdFixModule* m = dynamic_cast<StripIdFixModule*>(mw->module);
             if (m) m->idFixDataFromJson(modules);
 
+            // Drop engine-runtime binding fields, like Rack's own preset loader.
+            jsonStripIds(moduleJ);
+
             mw->fromJson(moduleJ);
 
             h->newModuleJ = mw->toJson();