Move moduleFromJson from engine to plugin namespace. Move Engine mutex lock at the top of Engine::step().

include/engine/Engine.hpp

@@ -87,11 +87,5 @@ struct Engine {
 };
 
 
-/** Creates a Module from a JSON module object.
-Throws an Exception if the model is not found.
-*/
-Module* moduleFromJson(json_t* moduleJ);
-
-
 } // namespace engine
 } // namespace rack

include/plugin.hpp

@@ -34,6 +34,10 @@ void syncUpdates();
 bool isSyncing();
 Plugin* getPlugin(const std::string& pluginSlug);
 Model* getModel(const std::string& pluginSlug, const std::string& modelSlug);
+/** Creates a Module from a JSON module object.
+Throws an Exception if the model is not found.
+*/
+engine::Module* moduleFromJson(json_t* moduleJ);
 /** Checks that the slug contains only alphanumeric characters, "-", and "_" */
 bool isSlugValid(const std::string& slug);
 /** Returns a string containing only the valid slug characters. */

src/app/RackWidget.cpp

@@ -21,7 +21,7 @@ namespace app {
 
 /** Creates a new Module and ModuleWidget */
 ModuleWidget* moduleWidgetFromJson(json_t* moduleJ) {
-	engine::Module* module = engine::moduleFromJson(moduleJ);
+	engine::Module* module = plugin::moduleFromJson(moduleJ);
 	assert(module);
 
 	// Create ModuleWidget
@@ -301,7 +301,7 @@ void RackWidget::pastePresetClipboardAction() {
 	});
 
 	try {
-		engine::Module* module = engine::moduleFromJson(moduleJ);
+		engine::Module* module = plugin::moduleFromJson(moduleJ);
 		// Reset ID so the Engine automatically assigns a new one
 		module->id = -1;
 		APP->engine->addModule(module);

src/engine/Engine.cpp

@@ -422,6 +422,7 @@ void Engine::clear() {
 
 
 void Engine::step(int frames) {
+	std::lock_guard<std::recursive_mutex> lock(internal->mutex);
 	// Configure thread
 	initMXCSR();
 	random::init();
@@ -444,8 +445,6 @@ void Engine::step(int frames) {
 			Engine_relaunchWorkers(this, settings::threadCount);
 		}
 
-		std::lock_guard<std::recursive_mutex> lock(internal->mutex);
-
 		// Update expander pointers
 		for (Module* module : internal->modules) {
 			Engine_updateExpander(this, &module->leftExpander);
@@ -475,6 +474,7 @@ void Engine::setPrimaryModule(Module* module) {
 
 
 Module* Engine::getPrimaryModule() {
+	// No lock, for performance
 	return internal->primaryModule;
 }
 
@@ -486,17 +486,19 @@ void Engine::setPaused(bool paused) {
 
 
 bool Engine::isPaused() {
-	// No lock
+	// No lock, for performance
 	return internal->paused;
 }
 
 
 float Engine::getSampleRate() {
+	// No lock, for performance
 	return internal->sampleRate;
 }
 
 
 float Engine::getSampleTime() {
+	// No lock, for performance
 	return internal->sampleTime;
 }
 
@@ -507,13 +509,14 @@ void Engine::yieldWorkers() {
 
 
 uint64_t Engine::getFrame() {
+	// No lock, for performance
 	return internal->frame;
 }
 
 
 void Engine::addModule(Module* module) {
-	assert(module);
 	std::lock_guard<std::recursive_mutex> lock(internal->mutex);
+	assert(module);
 	// Check that the module is not already added
 	auto it = std::find(internal->modules.begin(), internal->modules.end(), module);
 	assert(it == internal->modules.end());
@@ -547,8 +550,8 @@ void Engine::addModule(Module* module) {
 
 
 void Engine::removeModule(Module* module) {
-	assert(module);
 	std::lock_guard<std::recursive_mutex> lock(internal->mutex);
+	assert(module);
 	// Check that the module actually exists
 	auto it = std::find(internal->modules.begin(), internal->modules.end(), module);
 	assert(it != internal->modules.end());
@@ -601,8 +604,8 @@ Module* Engine::getModule(int moduleId) {
 
 
 void Engine::resetModule(Module* module) {
-	assert(module);
 	std::lock_guard<std::recursive_mutex> lock(internal->mutex);
+	assert(module);
 
 	Module::ResetEvent eReset;
 	module->onReset(eReset);
@@ -610,8 +613,8 @@ void Engine::resetModule(Module* module) {
 
 
 void Engine::randomizeModule(Module* module) {
-	assert(module);
 	std::lock_guard<std::recursive_mutex> lock(internal->mutex);
+	assert(module);
 
 	Module::RandomizeEvent eRandomize;
 	module->onRandomize(eRandomize);
@@ -619,8 +622,8 @@ void Engine::randomizeModule(Module* module) {
 
 
 void Engine::disableModule(Module* module, bool disabled) {
-	assert(module);
 	std::lock_guard<std::recursive_mutex> lock(internal->mutex);
+	assert(module);
 	if (module->disabled == disabled)
 		return;
 	// Clear outputs and set to 1 channel
@@ -689,8 +692,8 @@ static void Engine_updateConnected(Engine* that) {
 
 
 void Engine::addCable(Cable* cable) {
-	assert(cable);
 	std::lock_guard<std::recursive_mutex> lock(internal->mutex);
+	assert(cable);
 	// Check cable properties
 	assert(cable->inputModule);
 	assert(cable->outputModule);
@@ -744,8 +747,8 @@ void Engine::addCable(Cable* cable) {
 
 
 void Engine::removeCable(Cable* cable) {
-	assert(cable);
 	std::lock_guard<std::recursive_mutex> lock(internal->mutex);
+	assert(cable);
 	// Check that the cable is already added
 	auto it = std::find(internal->cables.begin(), internal->cables.end(), cable);
 	assert(it != internal->cables.end());
@@ -791,7 +794,7 @@ Cable* Engine::getCable(int cableId) {
 
 
 void Engine::setParam(Module* module, int paramId, float value) {
-	// Don't lock because this is called too frequently.
+	// No lock, for performance
 	// If param is being smoothed, cancel smoothing.
 	if (internal->smoothModule == module && internal->smoothParamId == paramId) {
 		internal->smoothModule = NULL;
@@ -802,11 +805,13 @@ void Engine::setParam(Module* module, int paramId, float value) {
 
 
 float Engine::getParam(Module* module, int paramId) {
+	// No lock, for performance
 	return module->params[paramId].value;
 }
 
 
 void Engine::setSmoothParam(Module* module, int paramId, float value) {
+	// No lock, for performance
 	// If another param is being smoothed, jump value
 	if (internal->smoothModule && !(internal->smoothModule == module && internal->smoothParamId == paramId)) {
 		internal->smoothModule->params[internal->smoothParamId].value = internal->smoothValue;
@@ -819,6 +824,7 @@ void Engine::setSmoothParam(Module* module, int paramId, float value) {
 
 
 float Engine::getSmoothParam(Module* module, int paramId) {
+	// No lock, for performance
 	if (internal->smoothModule == module && internal->smoothParamId == paramId)
 		return internal->smoothValue;
 	return getParam(module, paramId);
@@ -868,8 +874,7 @@ void Engine::removeParamHandle(ParamHandle* paramHandle) {
 
 
 ParamHandle* Engine::getParamHandle(int moduleId, int paramId) {
-	// Don't lock because this method is called potentially thousands of times per screen frame.
-
+	// No lock, for performance
 	auto it = internal->paramHandleCache.find(std::make_tuple(moduleId, paramId));
 	if (it == internal->paramHandleCache.end())
 		return NULL;
@@ -878,6 +883,7 @@ ParamHandle* Engine::getParamHandle(int moduleId, int paramId) {
 
 
 ParamHandle* Engine::getParamHandle(Module* module, int paramId) {
+	// No lock, for performance
 	return getParamHandle(module->id, paramId);
 }
 
@@ -922,6 +928,7 @@ void Engine::updateParamHandle(ParamHandle* paramHandle, int moduleId, int param
 
 
 json_t* Engine::toJson() {
+	std::lock_guard<std::recursive_mutex> lock(internal->mutex);
 	json_t* rootJ = json_object();
 
 	// modules
@@ -947,6 +954,7 @@ json_t* Engine::toJson() {
 
 
 void Engine::fromJson(json_t* rootJ) {
+	std::lock_guard<std::recursive_mutex> lock(internal->mutex);
 	// modules
 	json_t* modulesJ = json_object_get(rootJ, "modules");
 	if (!modulesJ)
@@ -955,7 +963,7 @@ void Engine::fromJson(json_t* rootJ) {
 	json_t* moduleJ;
 	json_array_foreach(modulesJ, moduleIndex, moduleJ) {
 		try {
-			Module* module = moduleFromJson(moduleJ);
+			Module* module = plugin::moduleFromJson(moduleJ);
 
 			// Before 1.0, the module ID was the index in the "modules" array
 			if (APP->patch->isLegacy(2)) {
@@ -1009,32 +1017,5 @@ void EngineWorker::run() {
 }
 
 
-Module* moduleFromJson(json_t* moduleJ) {
-	// Get slugs
-	json_t* pluginSlugJ = json_object_get(moduleJ, "plugin");
-	if (!pluginSlugJ)
-		throw Exception("\"plugin\" property not found in module JSON");
-	std::string pluginSlug = json_string_value(pluginSlugJ);
-	pluginSlug = plugin::normalizeSlug(pluginSlug);
-
-	json_t* modelSlugJ = json_object_get(moduleJ, "model");
-	if (!modelSlugJ)
-		throw Exception("\"model\" property not found in module JSON");
-	std::string modelSlug = json_string_value(modelSlugJ);
-	modelSlug = plugin::normalizeSlug(modelSlug);
-
-	// Get Model
-	plugin::Model* model = plugin::getModel(pluginSlug, modelSlug);
-	if (!model)
-		throw Exception(string::f("Could not find module \"%s\" of plugin \"%s\"", modelSlug.c_str(), pluginSlug.c_str()));
-
-	// Create Module
-	Module* module = model->createModule();
-	assert(module);
-	module->fromJson(moduleJ);
-	return module;
-}
-
-
 } // namespace engine
 } // namespace rack

src/plugin.cpp

@@ -7,6 +7,7 @@
 #include <app/common.hpp>
 #include <plugin/callbacks.hpp>
 #include <settings.hpp>
+#include <engine/Module.hpp>
 
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -162,6 +163,7 @@ static Plugin* loadPlugin(std::string path) {
 	return plugin;
 }
 
+
 static void loadPlugins(std::string path) {
 	for (std::string pluginPath : system::getEntries(path)) {
 		if (!system::isDirectory(pluginPath))
@@ -172,6 +174,7 @@ static void loadPlugins(std::string path) {
 	}
 }
 
+
 static void extractPackages(std::string path) {
 	std::string message;
 
@@ -232,6 +235,7 @@ void init() {
 	}
 }
 
+
 void destroy() {
 	for (Plugin* plugin : plugins) {
 		// Free library handle
@@ -250,6 +254,7 @@ void destroy() {
 	plugins.clear();
 }
 
+
 void logIn(const std::string& email, const std::string& password) {
 	loginStatus = "Logging in...";
 	json_t* reqJ = json_object();
@@ -286,15 +291,18 @@ void logIn(const std::string& email, const std::string& password) {
 	queryUpdates();
 }
 
+
 void logOut() {
 	settings::token = "";
 	updates.clear();
 }
 
+
 bool isLoggedIn() {
 	return settings::token != "";
 }
 
+
 void queryUpdates() {
 	if (settings::token.empty())
 		return;
@@ -392,6 +400,7 @@ void queryUpdates() {
 	updateStatus = "";
 }
 
+
 bool hasUpdates() {
 	for (Update& update : updates) {
 		if (update.progress < 1.f)
@@ -400,9 +409,11 @@ bool hasUpdates() {
 	return false;
 }
 
+
 static bool isSyncingUpdate = false;
 static bool isSyncingUpdates = false;
 
+
 void syncUpdate(Update* update) {
 	isSyncingUpdate = true;
 	DEFER({
@@ -425,6 +436,7 @@ void syncUpdate(Update* update) {
 	}
 }
 
+
 void syncUpdates() {
 	isSyncingUpdates = true;
 	DEFER({
@@ -441,10 +453,12 @@ void syncUpdates() {
 	restartRequested = true;
 }
 
+
 bool isSyncing() {
 	return isSyncingUpdate || isSyncingUpdates;
 }
 
+
 Plugin* getPlugin(const std::string& pluginSlug) {
 	for (Plugin* plugin : plugins) {
 		if (plugin->slug == pluginSlug) {
@@ -454,6 +468,7 @@ Plugin* getPlugin(const std::string& pluginSlug) {
 	return NULL;
 }
 
+
 Model* getModel(const std::string& pluginSlug, const std::string& modelSlug) {
 	Plugin* plugin = getPlugin(pluginSlug);
 	if (!plugin)
@@ -464,6 +479,34 @@ Model* getModel(const std::string& pluginSlug, const std::string& modelSlug) {
 	return model;
 }
 
+
+engine::Module* moduleFromJson(json_t* moduleJ) {
+	// Get slugs
+	json_t* pluginSlugJ = json_object_get(moduleJ, "plugin");
+	if (!pluginSlugJ)
+		throw Exception("\"plugin\" property not found in module JSON");
+	std::string pluginSlug = json_string_value(pluginSlugJ);
+	pluginSlug = normalizeSlug(pluginSlug);
+
+	json_t* modelSlugJ = json_object_get(moduleJ, "model");
+	if (!modelSlugJ)
+		throw Exception("\"model\" property not found in module JSON");
+	std::string modelSlug = json_string_value(modelSlugJ);
+	modelSlug = normalizeSlug(modelSlug);
+
+	// Get Model
+	Model* model = getModel(pluginSlug, modelSlug);
+	if (!model)
+		throw Exception(string::f("Could not find module \"%s\" of plugin \"%s\"", modelSlug.c_str(), pluginSlug.c_str()));
+
+	// Create Module
+	engine::Module* module = model->createModule();
+	assert(module);
+	module->fromJson(moduleJ);
+	return module;
+}
+
+
 bool isSlugValid(const std::string& slug) {
 	for (char c : slug) {
 		if (!(std::isalnum(c) || c == '-' || c == '_'))
@@ -472,6 +515,7 @@ bool isSlugValid(const std::string& slug) {
 	return true;
 }
 
+
 std::string normalizeSlug(const std::string& slug) {
 	std::string s;
 	for (char c : slug) {