From 76f6fcfad68d88728bb1a04b193029aa9e46e976 Mon Sep 17 00:00:00 2001 From: David Robillard Date: Sat, 20 Dec 2008 20:28:04 +0000 Subject: Fix deregistration of Jack ports, associated memory leaks. Hopefully a fix for #294 and #305). git-svn-id: http://svn.drobilla.net/lad/trunk/ingen@1877 a436a847-0d15-0410-975c-d299462d15a1 --- src/engine/AudioDriver.hpp | 11 +++---- src/engine/Driver.hpp | 10 +++++-- src/engine/DuplexPort.cpp | 14 ++++----- src/engine/JackAudioDriver.cpp | 19 ++++++++---- src/engine/JackAudioDriver.hpp | 6 ++-- src/engine/JackMidiDriver.cpp | 55 +++++++++++++++++------------------ src/engine/JackMidiDriver.hpp | 11 +++---- src/engine/MidiDriver.hpp | 5 ++-- src/engine/OSCDriver.hpp | 5 ++-- src/engine/PortImpl.cpp | 5 ++-- src/engine/events/ClearPatchEvent.cpp | 24 ++++++++++----- src/engine/events/ClearPatchEvent.hpp | 1 - src/engine/events/DestroyEvent.cpp | 10 ++++++- src/engine/events/DestroyEvent.hpp | 2 +- 14 files changed, 106 insertions(+), 72 deletions(-) (limited to 'src/engine') diff --git a/src/engine/AudioDriver.hpp b/src/engine/AudioDriver.hpp index a478f945..fb3b5320 100644 --- a/src/engine/AudioDriver.hpp +++ b/src/engine/AudioDriver.hpp @@ -45,12 +45,13 @@ public: virtual void set_root_patch(PatchImpl* patch) = 0; virtual PatchImpl* root_patch() = 0; - virtual void add_port(DriverPort* port) = 0; - virtual DriverPort* remove_port(const Raul::Path& path) = 0; + virtual void add_port(DriverPort* port) = 0; - virtual SampleCount buffer_size() const = 0; - virtual SampleCount sample_rate() const = 0; - virtual SampleCount frame_time() const = 0; + virtual Raul::List::Node* remove_port(const Raul::Path& path) = 0; + + virtual SampleCount buffer_size() const = 0; + virtual SampleCount sample_rate() const = 0; + virtual SampleCount frame_time() const = 0; virtual bool is_realtime() const = 0; diff --git a/src/engine/Driver.hpp b/src/engine/Driver.hpp index a11fffba..a2ca6b15 100644 --- a/src/engine/Driver.hpp +++ b/src/engine/Driver.hpp @@ -20,6 +20,7 @@ #include #include +#include "raul/Deletable.hpp" #include "interface/DataType.hpp" #include "DuplexPort.hpp" @@ -38,12 +39,14 @@ class DuplexPort; * * \ingroup engine */ -class DriverPort : boost::noncopyable { +class DriverPort : boost::noncopyable, public Raul::Deletable { public: virtual ~DriverPort() {} /** Set the name of the system port */ virtual void set_name(const std::string& name) = 0; + + virtual void unregister() = 0; bool is_input() const { return _patch_port->is_input(); } DuplexPort* patch_port() const { return _patch_port; } @@ -86,8 +89,9 @@ public: virtual DriverPort* driver_port(const Raul::Path& path) = 0; - virtual void add_port(DriverPort* port) = 0; - virtual DriverPort* remove_port(const Raul::Path& path) = 0; + virtual void add_port(DriverPort* port) = 0; + + virtual Raul::List::Node* remove_port(const Raul::Path& path) = 0; protected: DataType _type; diff --git a/src/engine/DuplexPort.cpp b/src/engine/DuplexPort.cpp index ec521e82..3ede6969 100644 --- a/src/engine/DuplexPort.cpp +++ b/src/engine/DuplexPort.cpp @@ -33,13 +33,13 @@ namespace Ingen { DuplexPort::DuplexPort( NodeImpl* parent, - const string& name, - uint32_t index, - uint32_t poly, - DataType type, - const Raul::Atom& value, - size_t buffer_size, - bool is_output) + const string& name, + uint32_t index, + uint32_t poly, + DataType type, + const Raul::Atom& value, + size_t buffer_size, + bool is_output) : PortImpl(parent, name, index, poly, type, value, buffer_size) , InputPort(parent, name, index, poly, type, value, buffer_size) , OutputPort(parent, name, index, poly, type, value, buffer_size) diff --git a/src/engine/JackAudioDriver.cpp b/src/engine/JackAudioDriver.cpp index f7507541..e5170127 100644 --- a/src/engine/JackAudioDriver.cpp +++ b/src/engine/JackAudioDriver.cpp @@ -70,7 +70,17 @@ JackAudioPort::JackAudioPort(JackAudioDriver* driver, DuplexPort* patch_port) JackAudioPort::~JackAudioPort() { - jack_port_unregister(_driver->jack_client(), _jack_port); + assert(_jack_port == NULL); +} + + +void +JackAudioPort::unregister() +{ + assert(_jack_port); + if (jack_port_unregister(_driver->jack_client(), _jack_port)) + cerr << "[JackMidiPort] ERROR: Unable to unregister port" << endl; + _jack_port = NULL; } @@ -200,7 +210,6 @@ void JackAudioDriver::add_port(DriverPort* port) { assert(ThreadManager::current_thread_id() == THREAD_PROCESS); - assert(dynamic_cast(port)); _ports.push_back((JackAudioPort*)port); } @@ -214,16 +223,16 @@ JackAudioDriver::add_port(DriverPort* port) * * It is the callers responsibility to delete the returned port. */ -DriverPort* +Raul::List::Node* JackAudioDriver::remove_port(const Path& path) { assert(ThreadManager::current_thread_id() == THREAD_PROCESS); for (Raul::List::iterator i = _ports.begin(); i != _ports.end(); ++i) if ((*i)->patch_port()->path() == path) - return _ports.erase(i)->elem(); // FIXME: LEAK + return (Raul::List::Node*)(_ports.erase(i)); - cerr << "[JackAudioDriver::remove_port] WARNING: Unable to find Jack port " << path << endl; + cerr << "[JackAudioDriver::remove_port] WARNING: Unable to find port " << path << endl; return NULL; } diff --git a/src/engine/JackAudioDriver.hpp b/src/engine/JackAudioDriver.hpp index 57c7f74e..a7f4c2ff 100644 --- a/src/engine/JackAudioDriver.hpp +++ b/src/engine/JackAudioDriver.hpp @@ -48,6 +48,8 @@ public: JackAudioPort(JackAudioDriver* driver, DuplexPort* patch_port); ~JackAudioPort(); + void unregister(); + void set_name(const std::string& name) { jack_port_set_name(_jack_port, name.c_str()); }; void prepare_buffer(jack_nframes_t nframes); @@ -89,10 +91,10 @@ public: DriverPort* create_port(DuplexPort* patch_port); void add_port(DriverPort* port); - DriverPort* remove_port(const Raul::Path& path); - DriverPort* driver_port(const Raul::Path& path); + Raul::List::Node* remove_port(const Raul::Path& path); + PatchImpl* root_patch() { return _root_patch; } void set_root_patch(PatchImpl* patch) { _root_patch = patch; } diff --git a/src/engine/JackMidiDriver.cpp b/src/engine/JackMidiDriver.cpp index d95f30ff..9f2f25d5 100644 --- a/src/engine/JackMidiDriver.cpp +++ b/src/engine/JackMidiDriver.cpp @@ -63,7 +63,18 @@ JackMidiPort::JackMidiPort(JackMidiDriver* driver, DuplexPort* patch_port) JackMidiPort::~JackMidiPort() { - jack_port_unregister(_driver->jack_client(), _jack_port); + assert(_jack_port == NULL); +} + + +void +JackMidiPort::unregister() +{ + assert(_jack_port); + if (jack_port_unregister(_driver->jack_client(), _jack_port)) + cerr << "[JackMidiPort] ERROR: Unable to unregister port" << endl; + + _jack_port = NULL; } @@ -187,8 +198,9 @@ JackMidiDriver::deactivate() void JackMidiDriver::pre_process(ProcessContext& context) { - for (Raul::List::iterator i = _in_ports.begin(); i != _in_ports.end(); ++i) - (*i)->pre_process(context); + for (Raul::List::iterator i = _ports.begin(); i != _ports.end(); ++i) + if ((*i)->is_input()) + (*i)->pre_process(context); } @@ -197,8 +209,9 @@ JackMidiDriver::pre_process(ProcessContext& context) void JackMidiDriver::post_process(ProcessContext& context) { - for (Raul::List::iterator i = _out_ports.begin(); i != _out_ports.end(); ++i) - (*i)->post_process(context); + for (Raul::List::iterator i = _ports.begin(); i != _ports.end(); ++i) + if (!(*i)->is_input()) + (*i)->post_process(context); } @@ -214,15 +227,11 @@ JackMidiDriver::add_port(DriverPort* port) { assert(ThreadManager::current_thread_id() == THREAD_PROCESS); assert(dynamic_cast(port)); - - if (port->is_input()) - _in_ports.push_back((JackMidiPort*)port); - else - _out_ports.push_back((JackMidiPort*)port); + _ports.push_back((JackMidiPort*)port); } -/** Remove an Jack MIDI port. +/** Remove a Jack MIDI port. * * Realtime safe. This is to be called at the beginning of a process cycle to * remove the port from the lists read by the audio thread, so the port @@ -230,22 +239,16 @@ JackMidiDriver::add_port(DriverPort* port) * * It is the callers responsibility to delete the returned port. */ -DriverPort* +Raul::List::Node* JackMidiDriver::remove_port(const Path& path) { assert(ThreadManager::current_thread_id() == THREAD_PROCESS); - // FIXME: duplex? - - for (Raul::List::iterator i = _in_ports.begin(); i != _in_ports.end(); ++i) + for (Raul::List::iterator i = _ports.begin(); i != _ports.end(); ++i) if ((*i)->patch_port()->path() == path) - return _in_ports.erase(i)->elem(); // FIXME: LEAK - - for (Raul::List::iterator i = _out_ports.begin(); i != _out_ports.end(); ++i) - if ((*i)->patch_port()->path() == path) - return _out_ports.erase(i)->elem(); // FIXME: LEAK - - cerr << "[JackMidiDriver::remove_input] WARNING: Unable to find Jack port " << path << endl; + return (Raul::List::Node*)_ports.erase(i); + + cerr << "[JackMidiDriver::remove_port] WARNING: Unable to find port " << path << endl; return NULL; } @@ -255,13 +258,7 @@ JackMidiDriver::driver_port(const Path& path) { assert(ThreadManager::current_thread_id() == THREAD_PROCESS); - // FIXME: duplex? - - for (Raul::List::iterator i = _in_ports.begin(); i != _in_ports.end(); ++i) - if ((*i)->patch_port()->path() == path) - return (*i); - - for (Raul::List::iterator i = _out_ports.begin(); i != _out_ports.end(); ++i) + for (Raul::List::iterator i = _ports.begin(); i != _ports.end(); ++i) if ((*i)->patch_port()->path() == path) return (*i); diff --git a/src/engine/JackMidiDriver.hpp b/src/engine/JackMidiDriver.hpp index 02bcd5c1..af885766 100644 --- a/src/engine/JackMidiDriver.hpp +++ b/src/engine/JackMidiDriver.hpp @@ -41,6 +41,8 @@ public: JackMidiPort(JackMidiDriver* driver, DuplexPort* port); virtual ~JackMidiPort(); + void unregister(); + void pre_process(ProcessContext& context); void post_process(ProcessContext& context); @@ -80,18 +82,17 @@ public: { return new JackMidiPort(this, patch_port); } void add_port(DriverPort* port); - DriverPort* remove_port(const Raul::Path& path); - DriverPort* driver_port(const Raul::Path& path); + + Raul::List::Node* remove_port(const Raul::Path& path); - jack_client_t* jack_client() { return _client; } + jack_client_t* jack_client() { return _client; } private: Engine& _engine; uint32_t _midi_event_type; - Raul::List _in_ports; - Raul::List _out_ports; + Raul::List _ports; friend class JackMidiPort; diff --git a/src/engine/MidiDriver.hpp b/src/engine/MidiDriver.hpp index 8bc07ff4..75da9db5 100644 --- a/src/engine/MidiDriver.hpp +++ b/src/engine/MidiDriver.hpp @@ -85,8 +85,9 @@ public: DriverPort* new_port(DuplexPort* patch_port) { return NULL; } - void add_port(DriverPort* port) {} - DriverPort* remove_port(const Raul::Path& path) { return NULL; } + void add_port(DriverPort* port) {} + + Raul::List::Node* remove_port(const Raul::Path& path) { return NULL; } void pre_process(ProcessContext& context) {} void post_process(ProcessContext& context) {} diff --git a/src/engine/OSCDriver.hpp b/src/engine/OSCDriver.hpp index 06125217..a7c84d37 100644 --- a/src/engine/OSCDriver.hpp +++ b/src/engine/OSCDriver.hpp @@ -69,8 +69,9 @@ public: DriverPort* create_port(DuplexPort* patch_port) { return NULL; } - void add_port(DriverPort* port) {} - DriverPort* remove_port(const Raul::Path& path) { return NULL; } + void add_port(DriverPort* port) {} + + Raul::List::Node* remove_port(const Raul::Path& path) { return NULL; } void prepare_block(const SampleCount block_start, const SampleCount block_end) {} }; diff --git a/src/engine/PortImpl.cpp b/src/engine/PortImpl.cpp index 1e137b75..26fe07d0 100644 --- a/src/engine/PortImpl.cpp +++ b/src/engine/PortImpl.cpp @@ -72,8 +72,9 @@ PortImpl::PortImpl(NodeImpl* const node, PortImpl::~PortImpl() { - for (uint32_t i=0; i < _poly; ++i) - delete _buffers->at(i); + if (!_fixed_buffers) + for (uint32_t i=0; i < _poly; ++i) + delete _buffers->at(i); delete _buffers; } diff --git a/src/engine/events/ClearPatchEvent.cpp b/src/engine/events/ClearPatchEvent.cpp index 7dfdbc3e..51e83154 100644 --- a/src/engine/events/ClearPatchEvent.cpp +++ b/src/engine/events/ClearPatchEvent.cpp @@ -36,7 +36,6 @@ namespace Ingen { ClearPatchEvent::ClearPatchEvent(Engine& engine, SharedPtr responder, FrameTime time, QueuedEventSource* source, const string& patch_path) : QueuedEvent(engine, responder, time, true, source) , _patch_path(patch_path) - , _driver_port(NULL) , _process(false) , _ports_array(NULL) , _compiled_patch(NULL) @@ -88,12 +87,24 @@ ClearPatchEvent::execute(ProcessContext& context) // Remove driver ports, if necessary if (_patch->parent() == NULL) { - for (EngineStore::Objects::iterator i = _removed_table->begin(); i != _removed_table->end(); ++i) { + for (EngineStore::Objects::iterator i = _removed_table->begin(); + i != _removed_table->end(); ++i) { SharedPtr port = PtrCast(i->second); - if (port && port->type() == DataType::AUDIO) - _driver_port = _engine.audio_driver()->remove_port(port->path()); - else if (port && port->type() == DataType::EVENT) - _driver_port = _engine.midi_driver()->remove_port(port->path()); + if (port && port->type() == DataType::AUDIO) { + List::Node* ln + = _engine.audio_driver()->remove_port(port->path()); + assert(ln); + ln->elem()->unregister(); + _engine.maid()->push(ln->elem()); + _engine.maid()->push(ln); + } else if (port && port->type() == DataType::EVENT) { + List::Node* ln + = _engine.midi_driver()->remove_port(port->path()); + assert(ln); + ln->elem()->unregister(); + _engine.maid()->push(ln->elem()); + _engine.maid()->push(ln); + } } } } @@ -105,7 +116,6 @@ ClearPatchEvent::post_process() { if (_patch != NULL) { delete _ports_array; - delete _driver_port; // Restore patch's run state if (_process) diff --git a/src/engine/events/ClearPatchEvent.hpp b/src/engine/events/ClearPatchEvent.hpp index 7fe76c65..ed91208f 100644 --- a/src/engine/events/ClearPatchEvent.hpp +++ b/src/engine/events/ClearPatchEvent.hpp @@ -50,7 +50,6 @@ public: private: const string _patch_path; SharedPtr _patch; - DriverPort* _driver_port; bool _process; Raul::Array* _ports_array; ///< New (external) ports for Patch CompiledPatch* _compiled_patch; ///< Patch's new process order diff --git a/src/engine/events/DestroyEvent.cpp b/src/engine/events/DestroyEvent.cpp index c1ee15e7..52c19945 100644 --- a/src/engine/events/DestroyEvent.cpp +++ b/src/engine/events/DestroyEvent.cpp @@ -128,6 +128,7 @@ DestroyEvent::execute(ProcessContext& context) if (_node->parent_patch()->compiled_patch()) _engine.maid()->push(_node->parent_patch()->compiled_patch()); + _node->parent_patch()->compiled_patch(_compiled_patch); } else if (_patch_port_listnode) { @@ -151,6 +152,10 @@ DestroyEvent::execute(ProcessContext& context) _driver_port = _engine.audio_driver()->remove_port(_port->path()); else if (_port->type() == DataType::EVENT) _driver_port = _engine.midi_driver()->remove_port(_port->path()); + + // Apparently this needs to be called in post_process?? + //if (_driver_port) + // _driver_port->elem()->unregister(); } } @@ -194,7 +199,10 @@ DestroyEvent::post_process() _responder->respond_error("Unable to destroy object"); } - delete _driver_port; + if (_driver_port) { + _driver_port->elem()->unregister(); + _engine.maid()->push(_driver_port); + } } diff --git a/src/engine/events/DestroyEvent.hpp b/src/engine/events/DestroyEvent.hpp index c260a480..59ff80ee 100644 --- a/src/engine/events/DestroyEvent.hpp +++ b/src/engine/events/DestroyEvent.hpp @@ -61,7 +61,7 @@ private: EngineStore::iterator _store_iterator; SharedPtr _node; ///< Non-NULL iff a node SharedPtr _port; ///< Non-NULL iff a port - DriverPort* _driver_port; + Raul::List::Node* _driver_port; PatchImpl::Nodes::Node* _patch_node_listnode; Raul::List::Node* _patch_port_listnode; Raul::Array* _ports_array; ///< New (external) ports for Patch -- cgit v1.2.1