Bug fixes.

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

13 files changed, 108 insertions, 252 deletions

diff --git a/src/common/util/CountedPtr.h b/src/common/util/CountedPtr.h
index e3f34ec2..81604e45 100644
--- a/src/common/util/CountedPtr.h
+++ b/src/common/util/CountedPtr.h
@@ -21,219 +21,39 @@
 #include <cassert>
 #include <cstddef>
 
-// honestly, WTF?
-#include <boost/shared_ptr.hpp>
-
-#define CountedPtr boost::shared_ptr
-#define PtrCast    boost::dynamic_pointer_cast
-#if 0
-// FIXME
-#ifndef NDEBUG
-#define COUNTED_PTR_DEBUG
+#ifdef DEBUG
+#define BOOST_SP_ENABLE_DEBUG_HOOKS 1
 #include <iostream>
 #include <list>
 #include <algorithm>
-static std::list<void*> counted_ptr_counters;
-#endif
 
+static std::list<void*> counted_ptr_counters;
 
-/** Simple reference counted pointer.
- *
- * Allocates one counter on the heap on initial construction.  You can safely
- * cast a CountedPtr<X> to a CountedPtr<Y> iff Y is a base of X (eg the cast
- * will only compile if it is a valid up-cast).
- *
- * It is possible for this to be a NULL pointer, and a boolean conversion
- * operator is provided so you can test for this with "if (someCountedPtr)".
- * Dereferencing a NULL CountedPtr will result in a failed assertion if
- * debugging is enabled.
- *
- * FIXME: test this more thoroughly
- */
-template <class T>
-class CountedPtr
-{
-public:
+// Use debug hooks to ensure 2 shared_ptrs never point to the same thing
+namespace boost {
 	
-	// Declare some other type of CountedPtr as a friend (for casting)
-	template <class Y> friend class CountedPtr;
-
-
-	/** Allocate a new Counter (if p is non-NULL) */
-	CountedPtr(T* p)
-	: _counter(0)
-	{
-		if (p)
-			_counter = new Counter(p);
-	}
-
-	/** Make a NULL CountedPtr.
-	 * It would be best if this didn't exist, but it makes these storable
-	 * in STL containers :/
-	 */
-	CountedPtr()
-	: _counter(0)
-	{}
-
-	~CountedPtr()
-	{
-		release();
-	}
-
-	/** Copy a CountedPtr with the same type. */
-	CountedPtr(const CountedPtr& copy) 
-	: _counter(0)
-	{
-		assert(this != &copy);
-		
-		if (copy)
-			retain(copy._counter);
-
-		assert(_counter == copy._counter);
+	static void sp_scalar_constructor_hook(void* object, unsigned long cnt, void* ptr) {
+		assert(std::find(counted_ptr_counters.begin(), counted_ptr_counters.end(),
+				(void*)object) == counted_ptr_counters.end());
+		counted_ptr_counters.push_back(object);
+		//std::cerr << "Creating " << typeid(object).name() << " Pointer to "
+			//<< object << std::endl;
 	}
 	
-	/** Copy a CountedPtr to a valid base class.
-	 */
-	template <class Y>
-	CountedPtr(const CountedPtr<Y>& y)
-	: _counter(0)
-	{
-		assert(this != (CountedPtr*)&y);
-
-		// Fail if this is not a valid cast
-		if (y) {
-			T* const casted_y = dynamic_cast<T* const>(y._counter->ptr);
-
-			if (casted_y) {
-				//release(); // FIXME: leak?
-				retain((Counter*)y._counter);
-				assert(_counter == (Counter*)y._counter);
-			}
-		}
-
-		assert( ! _counter || _counter == (Counter*)y._counter);
+	static void sp_scalar_destructor_hook(void* object, unsigned long cnt, void* ptr) {
+		counted_ptr_counters.remove(object);
+		//std::cerr << "Destroying " << typeid(object).name() << " Pointer to "
+			//<< object << std::endl;
 	}
 
-	/** Assign to the value of a CountedPtr of the same type. */
-	CountedPtr& operator=(const CountedPtr& copy)
-	{
-		if (this != &copy) {
-			assert( ! _counter || _counter != copy._counter);
-			release();
-			retain(copy._counter);
-		}
-		assert(_counter == copy._counter);
-		return *this;
-	}
+}
+#endif // DEBUG
 
-	/** Assign to the value of a CountedPtr of a different type. */
-	template <class Y>
-	CountedPtr& operator=(const CountedPtr<Y>& y)
-	{
-		if (this != (CountedPtr*)&y) {
-			assert(_counter != y._counter);
-			release();
-			retain(y._counter);
-		}
-		assert(_counter == y._counter);
-		return *this;
-	}
-
-	inline bool operator==(const CountedPtr& p) const
-	{
-		return (_counter == p._counter);
-	}
 
-	inline bool operator!=(const CountedPtr& p) const
-	{
-		return (_counter != p._counter);
-	}
-
-	/** Allow testing for NULL nicely like a real pointer */
-	operator bool() const
-	{
-		return (_counter && _counter->ptr);
-	}
-
-	inline T& operator*() const
-	{
-		assert(_counter);
-		assert(_counter->count > 0);
-		assert(_counter->ptr);
-		return *_counter->ptr;
-	}
-
-	inline T* operator->() const
-	{
-		assert(_counter);
-		assert(_counter->count > 0);
-		assert(_counter->ptr);
-		return _counter->ptr;
-	}
-
-	inline T* get() const { return _counter ? _counter->ptr : 0; }
-
-	bool unique() const { return (_counter ? _counter->count == 1 : true); }
-
-private:
-	/** Stored on the heap and referred to by all existing CountedPtr's to
-	 * the object */
-	class Counter
-	{
-	public:
-		Counter(T* p)
-		: ptr(p)
-		, count(1)
-		{
-			assert(p);
-#ifdef COUNTED_PTR_DEBUG
-			assert(std::find(counted_ptr_counters.begin(), counted_ptr_counters.end(), (void*)p)
-					== counted_ptr_counters.end());
-			counted_ptr_counters.push_back(p);
-			std::cerr << "Creating " << typeid(T).name() << " Counter " << this << std::endl;
-#endif
-		}
-		
-		~Counter()
-		{
-			// for debugging
-			assert(count == 0);
-			count = 0;
-		}
-
-		T* const        ptr;
-		volatile size_t count;
-
-	private:
-		// Prevent copies (undefined)
-		Counter(const Counter& copy);
-		Counter& operator=(const Counter& copy);
-	};
-
-	/** Increment the count */
-	void retain(Counter* c) 
-	{	
-		assert( ! _counter || _counter == c);
-		_counter = c;
-		if (_counter)
-			++(c->count);
-	}
-	
-	/** Decrement the count, delete if we're the last reference */
-	void release()
-	{	
-		if (_counter) {
-			if (--(_counter->count) == 0) {
-				delete _counter->ptr;
-				delete _counter;
-			}
-			_counter = 0;
-		}
-	}
-	
+#include <boost/shared_ptr.hpp>
 
-	Counter* _counter;
-};
-#endif
+#define CountedPtr boost::shared_ptr
+#define PtrCast    boost::dynamic_pointer_cast
 
 #endif // COUNTED_PTR_H
+
diff --git a/src/libs/client/NodeModel.cpp b/src/libs/client/NodeModel.cpp
index 05b7b43e..80f8f03a 100644
--- a/src/libs/client/NodeModel.cpp
+++ b/src/libs/client/NodeModel.cpp
@@ -44,8 +44,15 @@ NodeModel::NodeModel(const string& plugin_uri, const Path& path)
 
 NodeModel::~NodeModel()
 {
-	/*for (PortModelList::iterator i = m_ports.begin(); i != m_ports.end(); ++i)
-		delete(*i);*/
+	clear();
+	m_controller.reset();
+}
+
+
+void
+NodeModel::remove_port(CountedPtr<PortModel> port)
+{
+	m_ports.remove(port);
 }
 
 
@@ -64,11 +71,7 @@ NodeModel::remove_port(const string& port_path)
 void
 NodeModel::clear()
 {
-	/*for (PortModelList::iterator i = m_ports.begin(); i != m_ports.end(); ++i)
-		delete (*i);*/
-
 	m_ports.clear();
-
 	assert(m_ports.empty());
 }
 
@@ -100,6 +103,18 @@ NodeModel::add_child(CountedPtr<ObjectModel> c)
 
 
 void
+NodeModel::remove_child(CountedPtr<ObjectModel> c)
+{
+	assert(c->path().is_child_of(m_path));
+	assert(c->parent().get() == this);
+
+	CountedPtr<PortModel> pm = PtrCast<PortModel>(c);
+	assert(pm);
+	remove_port(pm);
+}
+
+
+void
 NodeModel::add_port(CountedPtr<PortModel> pm)
 {
 	assert(pm);
diff --git a/src/libs/client/NodeModel.h b/src/libs/client/NodeModel.h
index f0bede51..60973d95 100644
--- a/src/libs/client/NodeModel.h
+++ b/src/libs/client/NodeModel.h
@@ -49,9 +49,11 @@ public:
 	virtual ~NodeModel();
 	
 	void add_child(CountedPtr<ObjectModel> c);
+	void remove_child(CountedPtr<ObjectModel> c);
 
 	CountedPtr<PortModel> get_port(const string& port_name);
 	void       add_port(CountedPtr<PortModel> pm);
+	void       remove_port(CountedPtr<PortModel> pm);
 	void       remove_port(const string& port_path);
 	
 	virtual void clear();
diff --git a/src/libs/client/ObjectModel.h b/src/libs/client/ObjectModel.h
index 9ee4f8c4..744d78a0 100644
--- a/src/libs/client/ObjectModel.h
+++ b/src/libs/client/ObjectModel.h
@@ -60,6 +60,7 @@ public:
 	virtual void  set_parent(CountedPtr<ObjectModel> p) { m_parent = p; }
 	
 	virtual void add_child(CountedPtr<ObjectModel> c) = 0;
+	virtual void remove_child(CountedPtr<ObjectModel> c) = 0;
 
 	CountedPtr<ObjectController> controller() const { return m_controller; }
 	
diff --git a/src/libs/client/PatchLibrarian.h b/src/libs/client/PatchLibrarian.h
index a900b22f..d73912f1 100644
--- a/src/libs/client/PatchLibrarian.h
+++ b/src/libs/client/PatchLibrarian.h
@@ -48,7 +48,7 @@ public:
 	// FIXME: return booleans and set an errstr that can be checked or something?
 	
 	PatchLibrarian(CountedPtr<ModelEngineInterface> engine)
-	: _patch_search_path("."), _engine(_engine)
+	: _patch_search_path("."), _engine(engine)
 	{
 		assert(_engine);
 	}
diff --git a/src/libs/client/PatchModel.cpp b/src/libs/client/PatchModel.cpp
index 36e829f3..68a55b40 100644
--- a/src/libs/client/PatchModel.cpp
+++ b/src/libs/client/PatchModel.cpp
@@ -69,6 +69,26 @@ PatchModel::add_child(CountedPtr<ObjectModel> c)
 }
 
 
+void
+PatchModel::remove_child(CountedPtr<ObjectModel> c)
+{
+	assert(c->path().is_child_of(m_path));
+	assert(c->parent().get() == this);
+
+	CountedPtr<PortModel> pm = PtrCast<PortModel>(c);
+	if (pm) {
+		remove_port(pm);
+		return;
+	}
+	
+	CountedPtr<NodeModel> nm = PtrCast<NodeModel>(c);
+	if (nm) {
+		remove_node(nm);
+		return;
+	}
+}
+
+
 CountedPtr<NodeModel>
 PatchModel::get_node(const string& name)
 {
@@ -99,6 +119,25 @@ PatchModel::add_node(CountedPtr<NodeModel> nm)
 
 
 void
+PatchModel::remove_node(CountedPtr<NodeModel> nm)
+{
+	assert(nm->path().is_child_of(m_path));
+	assert(nm->parent().get() == this);
+
+	NodeModelMap::iterator i = m_nodes.find(nm->path().name());
+	if (i != m_nodes.end()) {
+		assert(i->second == nm);
+		m_nodes.erase(i);
+		removed_node_sig.emit(nm->path().name());
+		return;
+	}
+	
+	cerr << "[PatchModel::remove_node] " << m_path
+		<< ": failed to find node " << nm->path().name() << endl;
+}
+
+
+void
 PatchModel::remove_node(const string& name)
 {
 	assert(name.find("/") == string::npos);
diff --git a/src/libs/client/PatchModel.h b/src/libs/client/PatchModel.h
index db444de2..6ca8ed8f 100644
--- a/src/libs/client/PatchModel.h
+++ b/src/libs/client/PatchModel.h
@@ -51,10 +51,12 @@ public:
 	virtual void set_path(const Path& path);
 	
 	void add_child(CountedPtr<ObjectModel> c);
+	void remove_child(CountedPtr<ObjectModel> c);
 
 	CountedPtr<NodeModel> get_node(const string& node_name);
 	void                  add_node(CountedPtr<NodeModel> nm);
 	void                  remove_node(const string& name);
+	void                  remove_node(CountedPtr<NodeModel> nm);
 
 	void rename_node(const Path& old_path, const Path& new_path);
 	void rename_node_port(const Path& old_path, const Path& new_path);
diff --git a/src/libs/client/PortModel.h b/src/libs/client/PortModel.h
index 7b84c95a..1f816748 100644
--- a/src/libs/client/PortModel.h
+++ b/src/libs/client/PortModel.h
@@ -70,7 +70,8 @@ public:
 	{
 	}
 	
-	void add_child(CountedPtr<ObjectModel> c) { throw; }
+	void add_child(CountedPtr<ObjectModel> c)    { throw; }
+	void remove_child(CountedPtr<ObjectModel> c) { throw; }
 	
 	inline float min_val() const      { return m_min_val; }
 	inline float user_min() const     { return atof(get_metadata("min").c_str()); } // FIXME: haaack
diff --git a/src/libs/client/Store.cpp b/src/libs/client/Store.cpp
index 36e815d8..c0bc1a31 100644
--- a/src/libs/client/Store.cpp
+++ b/src/libs/client/Store.cpp
@@ -153,7 +153,18 @@ Store::remove_object(const Path& path)
 		CountedPtr<ObjectModel> result = (*i).second;
 		m_objects.erase(i);
 		//cout << "[Store] Removed " << path << endl;
+
+		if (result)
+			result->destroyed_sig.emit();
+
+		if (result->path() != "/") {
+			CountedPtr<ObjectModel> parent = this->object(result->path().parent());
+			if (parent) {
+				parent->remove_child(result);
+			}
+		}
 		return result;
+
 	} else {
 		cerr << "[Store] Unable to find object " << path << " to remove." << endl;
 		return CountedPtr<ObjectModel>();
@@ -200,32 +211,7 @@ Store::add_plugin(CountedPtr<PluginModel> pm)
 void
 Store::destruction_event(const Path& path)
 {
-	// Hopefully the compiler will optimize all these const pointers into one...
-	
-	CountedPtr<ObjectModel> obj_ptr = remove_object(path);
-	ObjectModel* const object = obj_ptr.get();
-
-	// FIXME: Why does this need to be specific?  Just make a remove_child
-	// for everything
-	
-	// Succeeds for (Plugin) Nodes and Patches
-	NodeModel* const node = dynamic_cast<NodeModel*>(object);
-	if (node) {
-		cerr << "Node\n";
-		PatchModel* const parent = dynamic_cast<PatchModel* const>(object->parent().get());
-		if (parent)
-			parent->remove_node(node->path().name());
-	}
-
-	PortModel* const port = dynamic_cast<PortModel*>(object);
-	if (port) {
-		NodeModel* const parent = dynamic_cast<NodeModel* const>(object->parent().get());
-		assert(parent);
-		parent->remove_port(port->path().name());
-	}
-
-	if (object)
-		object->destroyed_sig.emit();
+	remove_object(path);
 }
 
 void
diff --git a/src/progs/ingenuity/NodeController.cpp b/src/progs/ingenuity/NodeController.cpp
index 677bc8b5..641d975e 100644
--- a/src/progs/ingenuity/NodeController.cpp
+++ b/src/progs/ingenuity/NodeController.cpp
@@ -95,6 +95,7 @@ NodeController::NodeController(CountedPtr<NodeModel> model)
 
 NodeController::~NodeController()
 {
+	cerr << "~NodeController()\n";
 	destroy_module();
 }
 
@@ -102,7 +103,9 @@ NodeController::~NodeController()
 void
 NodeController::destroy()
 {
-	delete this;
+	cerr << "FIXME: NODE DESTROYED\n";
+	destroy_module(); // cuts reference
+	//delete this;
 }
 
 
diff --git a/src/progs/ingenuity/PatchController.cpp b/src/progs/ingenuity/PatchController.cpp
index 8dee4cd2..f02aa773 100644
--- a/src/progs/ingenuity/PatchController.cpp
+++ b/src/progs/ingenuity/PatchController.cpp
@@ -153,13 +153,6 @@ PatchController::destroy()
 
 
 void
-PatchController::destroy()
-{
-	delete this;
-}
-
-
-void
 PatchController::metadata_update(const string& key, const string& value)
 {
 	NodeController::metadata_update(key, value);
@@ -179,9 +172,9 @@ PatchController::set_path(const Path& new_path)
 	for (NodeModelMap::const_iterator i = patch_model()->nodes().begin();
 			i != patch_model()->nodes().end(); ++i) {
 		const NodeModel* const nm = (*i).second.get();
-		assert(nm );
+		assert(nm);
 		CountedPtr<NodeController> nc = PtrCast<NodeController>(nm->controller());
-		assert(nc );
+		assert(nc);
 		nc->set_path(new_path.base() + nc->node_model()->path().name());
 	}
 	
@@ -376,7 +369,7 @@ PatchController::add_node(CountedPtr<NodeModel> node)
 
 	CountedPtr<NodeController> nc = PtrCast<NodeController>(ControllerFactory::get_controller(node));
 	assert(nc);
-	assert(node->controller() == nc);
+	assert(node->controller() == nc); // lifeline reference
 
 	if (m_patch_view) {
 		double x, y;
diff --git a/src/progs/ingenuity/PatchController.h b/src/progs/ingenuity/PatchController.h
index 2ee01738..e9c0d0b6 100644
--- a/src/progs/ingenuity/PatchController.h
+++ b/src/progs/ingenuity/PatchController.h
@@ -99,8 +99,6 @@ private:
 	friend class ControllerFactory;
 	PatchController(CountedPtr<PatchModel> model);
 	
-	void destroy();
-
 	void add_node(CountedPtr<NodeModel> object);
 	
 	PatchPropertiesWindow* m_properties_window;
diff --git a/src/progs/ingenuity/WindowFactory.cpp b/src/progs/ingenuity/WindowFactory.cpp
index 6a913663..c7d79aca 100644
--- a/src/progs/ingenuity/WindowFactory.cpp
+++ b/src/progs/ingenuity/WindowFactory.cpp
@@ -69,8 +69,6 @@ WindowFactory::create_new(CountedPtr<PatchController> patch)
 	win->signal_delete_event().connect(sigc::bind<0>(
 		sigc::mem_fun(this, &WindowFactory::remove), win));
 
-	cerr << "Created window - count: " << _windows.size() << endl;
-
 	return win;
 }
 
@@ -100,8 +98,6 @@ WindowFactory::remove(PatchWindow* win, GdkEventAny* ignored)
 
 	delete win;
 	
-	cerr << "Removed window " << win << "- count: " << _windows.size() << endl;
-	
 	return true;
 }