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

15 files changed, 107 insertions, 73 deletions

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<DriverPort*>::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 <string>
 #include <boost/utility.hpp>
+#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<DriverPort*>::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<JackAudioPort*>(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<DriverPort*>::Node*
 JackAudioDriver::remove_port(const Path& path)
 {
 	assert(ThreadManager::current_thread_id() == THREAD_PROCESS);
 
 	for (Raul::List<JackAudioPort*>::iterator i = _ports.begin(); i != _ports.end(); ++i)
 		if ((*i)->patch_port()->path() == path)
-			return _ports.erase(i)->elem(); // FIXME: LEAK
+			return (Raul::List<DriverPort*>::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<DriverPort*>::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<JackMidiPort*>::iterator i = _in_ports.begin(); i != _in_ports.end(); ++i)
-		(*i)->pre_process(context);
+	for (Raul::List<JackMidiPort*>::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<JackMidiPort*>::iterator i = _out_ports.begin(); i != _out_ports.end(); ++i)
-		(*i)->post_process(context);
+	for (Raul::List<JackMidiPort*>::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<JackMidiPort*>(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<DriverPort*>::Node*
 JackMidiDriver::remove_port(const Path& path)
 {
 	assert(ThreadManager::current_thread_id() == THREAD_PROCESS);
 
-	// FIXME: duplex?
-
-	for (Raul::List<JackMidiPort*>::iterator i = _in_ports.begin(); i != _in_ports.end(); ++i)
+	for (Raul::List<JackMidiPort*>::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<JackMidiPort*>::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<DriverPort*>::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<JackMidiPort*>::iterator i = _in_ports.begin(); i != _in_ports.end(); ++i)
-		if ((*i)->patch_port()->path() == path)
-			return (*i);
-
-	for (Raul::List<JackMidiPort*>::iterator i = _out_ports.begin(); i != _out_ports.end(); ++i)
+	for (Raul::List<JackMidiPort*>::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<DriverPort*>::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<JackMidiPort*> _in_ports;
-	Raul::List<JackMidiPort*> _out_ports;
+	Raul::List<JackMidiPort*> _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<DriverPort*>::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<DriverPort*>::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> 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<PortImpl> port = PtrCast<PortImpl>(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<DriverPort*>::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<DriverPort*>::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<PatchImpl>    _patch;
-	DriverPort*             _driver_port;
 	bool                    _process;
 	Raul::Array<PortImpl*>* _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<NodeImpl>                          _node;  ///< Non-NULL iff a node
 	SharedPtr<PortImpl>                          _port;  ///< Non-NULL iff a port
-	DriverPort*                                  _driver_port;
+	Raul::List<DriverPort*>::Node*               _driver_port;
 	PatchImpl::Nodes::Node*                      _patch_node_listnode;
 	Raul::List<PortImpl*>::Node*                 _patch_port_listnode;
 	Raul::Array<PortImpl*>*                      _ports_array; ///< New (external) ports for Patch
diff --git a/src/gui/PatchCanvas.cpp b/src/gui/PatchCanvas.cpp
index 8a580b88..0360d989 100644
--- a/src/gui/PatchCanvas.cpp
+++ b/src/gui/PatchCanvas.cpp
@@ -395,11 +395,11 @@ PatchCanvas::remove_port(SharedPtr<PortModel> pm)
 
 	// Port on this patch
 	if (i != _views.end()) {
-		_views.erase(i);
 		bool ret = remove_item(i->second);
 		if (!ret)
 			cerr << "WARNING: Failed to remove port item: " << pm->path() << endl;
 		i->second.reset();
+		_views.erase(i);
 	
 	} else {
 		SharedPtr<NodeModule> module = PtrCast<NodeModule>(_views[pm->parent()]);