From 16c866f220847ae23012318d2c1a5023076ab5fa Mon Sep 17 00:00:00 2001 From: David Robillard Date: Mon, 11 Sep 2006 19:57:59 +0000 Subject: Bug fixes. git-svn-id: http://svn.drobilla.net/lad/ingen@129 a436a847-0d15-0410-975c-d299462d15a1 --- src/common/util/CountedPtr.h | 222 +++----------------------------- src/libs/client/NodeModel.cpp | 27 +++- src/libs/client/NodeModel.h | 2 + src/libs/client/ObjectModel.h | 1 + src/libs/client/PatchLibrarian.h | 2 +- src/libs/client/PatchModel.cpp | 39 ++++++ src/libs/client/PatchModel.h | 2 + src/libs/client/PortModel.h | 3 +- src/libs/client/Store.cpp | 38 ++---- src/progs/ingenuity/NodeController.cpp | 5 +- src/progs/ingenuity/PatchController.cpp | 13 +- src/progs/ingenuity/PatchController.h | 2 - src/progs/ingenuity/WindowFactory.cpp | 4 - 13 files changed, 108 insertions(+), 252 deletions(-) (limited to 'src') 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 #include -// honestly, WTF? -#include - -#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 #include #include -static std::list counted_ptr_counters; -#endif +static std::list counted_ptr_counters; -/** Simple reference counted pointer. - * - * Allocates one counter on the heap on initial construction. You can safely - * cast a CountedPtr to a CountedPtr 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 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 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 != ©); - - 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 - CountedPtr(const CountedPtr& y) - : _counter(0) - { - assert(this != (CountedPtr*)&y); - - // Fail if this is not a valid cast - if (y) { - T* const casted_y = dynamic_cast(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 != ©) { - 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 - CountedPtr& operator=(const CountedPtr& 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 - 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 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()); } @@ -99,6 +102,18 @@ NodeModel::add_child(CountedPtr c) } +void +NodeModel::remove_child(CountedPtr c) +{ + assert(c->path().is_child_of(m_path)); + assert(c->parent().get() == this); + + CountedPtr pm = PtrCast(c); + assert(pm); + remove_port(pm); +} + + void NodeModel::add_port(CountedPtr 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 c); + void remove_child(CountedPtr c); CountedPtr get_port(const string& port_name); void add_port(CountedPtr pm); + void remove_port(CountedPtr 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 p) { m_parent = p; } virtual void add_child(CountedPtr c) = 0; + virtual void remove_child(CountedPtr c) = 0; CountedPtr 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 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 c) } +void +PatchModel::remove_child(CountedPtr c) +{ + assert(c->path().is_child_of(m_path)); + assert(c->parent().get() == this); + + CountedPtr pm = PtrCast(c); + if (pm) { + remove_port(pm); + return; + } + + CountedPtr nm = PtrCast(c); + if (nm) { + remove_node(nm); + return; + } +} + + CountedPtr PatchModel::get_node(const string& name) { @@ -98,6 +118,25 @@ PatchModel::add_node(CountedPtr nm) } +void +PatchModel::remove_node(CountedPtr 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) { 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 c); + void remove_child(CountedPtr c); CountedPtr get_node(const string& node_name); void add_node(CountedPtr nm); void remove_node(const string& name); + void remove_node(CountedPtr 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 c) { throw; } + void add_child(CountedPtr c) { throw; } + void remove_child(CountedPtr 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 result = (*i).second; m_objects.erase(i); //cout << "[Store] Removed " << path << endl; + + if (result) + result->destroyed_sig.emit(); + + if (result->path() != "/") { + CountedPtr 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(); @@ -200,32 +211,7 @@ Store::add_plugin(CountedPtr pm) void Store::destruction_event(const Path& path) { - // Hopefully the compiler will optimize all these const pointers into one... - - CountedPtr 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(object); - if (node) { - cerr << "Node\n"; - PatchModel* const parent = dynamic_cast(object->parent().get()); - if (parent) - parent->remove_node(node->path().name()); - } - - PortModel* const port = dynamic_cast(object); - if (port) { - NodeModel* const parent = dynamic_cast(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 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 @@ -152,13 +152,6 @@ PatchController::destroy() #endif -void -PatchController::destroy() -{ - delete this; -} - - void PatchController::metadata_update(const string& key, const string& 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 nc = PtrCast(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 node) CountedPtr nc = PtrCast(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 model); - void destroy(); - void add_node(CountedPtr 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 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; } -- cgit v1.2.1