Fix broken iterator use (memory errors on erase) in ResourceImpl::set_property.

Load all required modules before starting jack (Jack went crazy sometimes otherwise, particularly with ingen running in gdb).
Fix insane non-const reference return to Resource::set_property.
Exit cleanly on interrupt (Ctrl-C) and terminate (kill).


git-svn-id: http://svn.drobilla.net/lad/trunk/ingen@3480 a436a847-0d15-0410-975c-d299462d15a1

11 files changed, 76 insertions, 49 deletions

diff --git a/src/client/ObjectModel.cpp b/src/client/ObjectModel.cpp
index a0235bb6..940bd51e 100644
--- a/src/client/ObjectModel.cpp
+++ b/src/client/ObjectModel.cpp
@@ -47,11 +47,11 @@ ObjectModel::~ObjectModel()
 {
 }
 
-Raul::Atom&
+const Raul::Atom&
 ObjectModel::set_property(const Raul::URI& key, const Raul::Atom& value,
                           Resource::Graph ctx)
 {
-	Raul::Atom& my_value = ResourceImpl::set_property(key, value, ctx);
+	const Raul::Atom& my_value = ResourceImpl::set_property(key, value, ctx);
 	_signal_property.emit(key, my_value);
 	return my_value;
 }
diff --git a/src/client/PluginModel.cpp b/src/client/PluginModel.cpp
index c9395ccd..694e5ce0 100644
--- a/src/client/PluginModel.cpp
+++ b/src/client/PluginModel.cpp
@@ -94,7 +94,7 @@ PluginModel::get_property(const URI& key) const
 	}
 
 	if (_lilv_plugin) {
-		boost::optional<Raul::Atom&> ret;
+		boost::optional<const Raul::Atom&> ret;
 		LilvNode*  lv2_pred = lilv_new_uri(_lilv_world, key.str().c_str());
 		LilvNodes* values   = lilv_plugin_get_value(_lilv_plugin, lv2_pred);
 		lilv_node_free(lv2_pred);
diff --git a/src/client/PortModel.cpp b/src/client/PortModel.cpp
index fe85e108..8404d60e 100644
--- a/src/client/PortModel.cpp
+++ b/src/client/PortModel.cpp
@@ -22,12 +22,12 @@
 namespace Ingen {
 namespace Client {
 
-Raul::Atom&
+const Raul::Atom&
 PortModel::set_property(const Raul::URI&  uri,
                         const Raul::Atom& value,
                         Resource::Graph   ctx)
 {
-	Raul::Atom& ret = ObjectModel::set_property(uri, value, ctx);
+	const Raul::Atom& ret = ObjectModel::set_property(uri, value, ctx);
 	if (uri == _uris.ingen_value)
 		this->value(value);
 	return ret;
diff --git a/src/gui/App.cpp b/src/gui/App.cpp
index 35d31b3e..055b4ef7 100644
--- a/src/gui/App.cpp
+++ b/src/gui/App.cpp
@@ -133,21 +133,22 @@ App::init(Ingen::Shared::World* world)
 		"widget \"*ingen_embedded_node_gui_container*\" style \"ingen_embedded_node_gui_style\"\n";
 
 	Gtk::RC::parse_string(rc_style);
-
-	App::instance().connect_window()->start(world);
-
-	// Run main iterations here until we're attached to the engine
-	// (otherwise with 'ingen -egl' we'll get a bunch of notifications about load immediately
-	// before even knowing about the root patch or plugins)
-	while (!App::instance().connect_window()->attached())
-		if (_main->iteration())
-			break;
 }
 
 void
 App::run()
 {
-	assert(_main);
+	App& me = App::instance();
+
+	me._connect_window->start(me.world());
+
+	// Run main iterations here until we're attached to the engine. Otherwise
+	// with 'ingen -egl' we'd get a bunch of notifications about load
+	// immediately before even knowing about the root patch or plugins)
+	while (!me._connect_window->attached())
+		if (me._main->iteration())
+			break;
+
 	_main->run();
 	LOG(info) << "Exiting" << endl;
 }
diff --git a/src/gui/ingen_gui.cpp b/src/gui/ingen_gui.cpp
index 83f4029d..88d27b39 100644
--- a/src/gui/ingen_gui.cpp
+++ b/src/gui/ingen_gui.cpp
@@ -21,6 +21,8 @@
 struct IngenGUIModule : public Ingen::Shared::Module {
 	void load(Ingen::Shared::World* world) {
 		Ingen::GUI::App::init(world);
+	}
+	void run(Ingen::Shared::World* world) {
 		Ingen::GUI::App::run();
 	}
 };
diff --git a/src/ingen/main.cpp b/src/ingen/main.cpp
index 2910f9eb..aae428c8 100644
--- a/src/ingen/main.cpp
+++ b/src/ingen/main.cpp
@@ -59,13 +59,18 @@ static const timespec main_rate = { 0, 125000000 }; // 1/8 second
 Ingen::Shared::World* world = NULL;
 
 void
-ingen_interrupt(int)
+ingen_interrupt(int signal)
 {
-	cout << "ingen: Interrupted" << endl;
-	if (world->local_engine())
-		world->local_engine()->quit();
-	delete world;
-	exit(EXIT_FAILURE);
+	if (signal == SIGTERM) {
+		cerr << "ingen: Terminated" << endl;
+		delete world;
+		exit(EXIT_FAILURE);
+	} else {
+		cout << "ingen: Interrupted" << endl;
+		if (world && world->local_engine()) {
+			world->local_engine()->quit();
+		}
+	}
 }
 
 void
@@ -110,7 +115,7 @@ main(int argc, char** argv)
 	g_type_init();
 #endif
 
-	Ingen::Shared::World* world = new Ingen::Shared::World(&conf, argc, argv);
+	world = new Ingen::Shared::World(&conf, argc, argv);
 
 	if (conf.option("uuid").get_string()) {
 		world->set_jack_uuid(conf.option("uuid").get_string());
@@ -148,20 +153,29 @@ main(int argc, char** argv)
 		          (string("Unable to create interface to `") + uri + "'").c_str());
 	}
 
-	// Activate the engine, if we have one
-	if (world->local_engine()) {
-		ingen_try(world->load_module("jack"),
-		          "Unable to load jack module");
+	world->set_engine(engine_interface);
+
+	// Load necessary modules before activating engine (and Jack driver)
+
+	if (conf.option("load").is_valid()) {
+		ingen_try(world->load_module("serialisation"),
+		          "Unable to load serialisation module");
 	}
 
-	world->set_engine(engine_interface);
+	if (conf.option("gui").get_bool()) {
+		ingen_try(world->load_module("gui"),
+		          "Unable to load GUI module");
+	}
 
+	// Activate the engine, if we have one
 	if (world->local_engine()) {
+		ingen_try(world->load_module("jack"),
+		          "Unable to load jack module");
 		world->local_engine()->activate();
 	}
 
 	// Load a patch
-	if (conf.option("load").is_valid() && engine_interface) {
+	if (conf.option("load").is_valid()) {
 		boost::optional<Path>   parent;
 		boost::optional<Symbol> symbol;
 		const Raul::Atom&       path_option = conf.option("path");
@@ -178,9 +192,6 @@ main(int argc, char** argv)
 			}
 		}
 
-		ingen_try(world->load_module("serialisation"),
-		          "Unable to load serialisation module");
-
 		ingen_try(world->parser(),
 		          "Unable to create parser");
 
@@ -199,13 +210,10 @@ main(int argc, char** argv)
 			world, engine_interface.get(), uri, parent, symbol);
 	}
 
-	// Load GUI
-	if (conf.option("gui").get_bool())
-		ingen_try(world->load_module("gui"),
-		          "Unable to load GUI module");
-
-	// Run a script
-	if (conf.option("run").is_valid()) {
+	if (conf.option("gui").get_bool()) {
+		world->run_module("gui");
+	} else if (conf.option("run").is_valid()) {
+		// Run a script
 #ifdef WITH_BINDINGS
 		ingen_try(world->load_module("bindings"),
 		          "Unable to load bindings module");
@@ -215,15 +223,13 @@ main(int argc, char** argv)
 		cerr << "This build of ingen does not support scripting." << endl;
 #endif
 
-	// Run main loop
 	} else if (world->local_engine() && !conf.option("gui").get_bool()) {
+		// Run main loop
+
 		// Set up signal handlers that will set quit_flag on interrupt
 		signal(SIGINT, ingen_interrupt);
 		signal(SIGTERM, ingen_interrupt);
 
-		// Activate the engine
-		world->local_engine()->activate();
-
 		// Run engine main loop until interrupt
 		while (world->local_engine()->main_iteration()) {
 			nanosleep(&main_rate, NULL);
diff --git a/src/shared/Module.hpp b/src/shared/Module.hpp
index 60acf1ff..a34db75f 100644
--- a/src/shared/Module.hpp
+++ b/src/shared/Module.hpp
@@ -38,6 +38,7 @@ struct Module {
 	}
 
 	virtual void load(Ingen::Shared::World* world) = 0;
+	virtual void run(Ingen::Shared::World* world) {}
 
 	SharedPtr<Glib::Module> library;
 };
diff --git a/src/shared/ResourceImpl.cpp b/src/shared/ResourceImpl.cpp
index 14227ccf..729a8152 100644
--- a/src/shared/ResourceImpl.cpp
+++ b/src/shared/ResourceImpl.cpp
@@ -39,17 +39,19 @@ ResourceImpl::add_property(const Raul::URI& uri, const Raul::Atom& value)
 	_properties.insert(make_pair(uri, value));
 }
 
-Raul::Atom&
+const Raul::Atom&
 ResourceImpl::set_property(const Raul::URI& uri, const Raul::Atom& value,
                            Resource::Graph ctx)
 {
 	// Erase existing property in this context
 	for (Properties::iterator i = _properties.find(uri);
-	     (i != _properties.end()) && (i->first == uri);
-	     ++i) {
+	     (i != _properties.end()) && (i->first == uri);) {
+		Properties::iterator next = i;
+		++next;
 		if (i->second.context() == ctx && i->second == value) {
 			_properties.erase(i);
 		}
+		i = next;
 	}
 
 	// Insert new property
@@ -84,7 +86,7 @@ ResourceImpl::has_property(const Raul::URI& uri, const Raul::Atom& value) const
 	return false;
 }
 
-Raul::Atom&
+const Raul::Atom&
 ResourceImpl::set_property(const Raul::URI& uri, const Raul::Atom& value) const
 {
 	return const_cast<ResourceImpl*>(this)->set_property(uri, value);
diff --git a/src/shared/ResourceImpl.hpp b/src/shared/ResourceImpl.hpp
index 6afc567d..0d96d840 100644
--- a/src/shared/ResourceImpl.hpp
+++ b/src/shared/ResourceImpl.hpp
@@ -46,7 +46,7 @@ public:
 	Properties properties(Resource::Graph ctx) const;
 
 	const Raul::Atom& get_property(const Raul::URI& uri) const;
-	Raul::Atom&       set_property(const Raul::URI& uri, const Raul::Atom& value,
+	const Raul::Atom& set_property(const Raul::URI& uri, const Raul::Atom& value,
 	                               Resource::Graph ctx=Resource::DEFAULT);
 	void              remove_property(const Raul::URI& uri, const Raul::Atom& value);
 	bool              has_property(const Raul::URI& uri, const Raul::Atom& value) const;
@@ -69,7 +69,7 @@ public:
 			bool& port, bool& is_output, PortType& data_type);
 
 protected:
-	Raul::Atom& set_property(const Raul::URI& uri, const Raul::Atom& value) const;
+	const Raul::Atom& set_property(const Raul::URI& uri, const Raul::Atom& value) const;
 
 	LV2URIMap& _uris;
 
diff --git a/src/shared/World.cpp b/src/shared/World.cpp
index c583f2fa..eb60a1a5 100644
--- a/src/shared/World.cpp
+++ b/src/shared/World.cpp
@@ -210,7 +210,7 @@ SharedPtr<LV2URIMap> World::uris()      { return _impl->uris; }
 
 /** Load an Ingen module.
  * @return true on success, false on failure
-m */
+ */
 bool
 World::load_module(const char* name)
 {
@@ -228,6 +228,19 @@ World::load_module(const char* name)
 	}
 }
 
+bool
+World::run_module(const char* name)
+{
+	Pimpl::Modules::iterator i = _impl->modules.find(name);
+	if (i == _impl->modules.end()) {
+		LOG(error) << "Attempt to run unloaded module `" << name << "'" << endl;
+		return false;
+	}
+
+	i->second->run(this);
+	return true;
+}
+	
 /** Unload all loaded Ingen modules.
  */
 void
diff --git a/src/shared/World.hpp b/src/shared/World.hpp
index 6aacea36..8b0a65da 100644
--- a/src/shared/World.hpp
+++ b/src/shared/World.hpp
@@ -60,6 +60,8 @@ public:
 	virtual ~World();
 
 	virtual bool load_module(const char* name);
+	virtual bool run_module(const char* name);
+
 	virtual void unload_modules();
 
 	typedef SharedPtr<ServerInterface> (*InterfaceFactory)(