OSC is now driven by an event loop; fix up lifetime mgmt of Glib::Source to workaround bug in Glib

git-svn-id: svn://localhost/ardour2/branches/3.0@6329 d708f5d6-7413-0410-9779-e7cbd77b26cf

gtk2_ardour/main.cc

@@ -393,7 +393,7 @@ int main (int argc, char *argv[])
 	ui = 0;
 
 	ARDOUR::cleanup ();
-	// pthread_cancel ();
+	pthread_cancel_all ();
 
 #ifdef HAVE_LV2
 	close_external_ui_windows();

libs/ardour/ardour/midi_ui.h

@@ -38,7 +38,7 @@ class MidiControlUI : public AbstractUI<MidiUIRequest>
 	void do_request (MidiUIRequest*);
 	
   private:
-	typedef std::list<Glib::RefPtr<Glib::IOSource> > PortSources;
+	typedef std::list<GSource*> PortSources;
 	PortSources port_sources;
 	ARDOUR::Session& _session;
 	

libs/ardour/control_protocol_manager.cc

@@ -121,6 +121,7 @@ ControlProtocolManager::instantiate (ControlProtocolInfo& cpi)
 		return 0;
 	}
 
+
 	Glib::Mutex::Lock lm (protocols_lock);
 	control_protocols.push_back (cpi.protocol);
 
@@ -152,13 +153,6 @@ ControlProtocolManager::teardown (ControlProtocolInfo& cpi)
 		} else {
 			cerr << "Programming error: ControlProtocolManager::teardown() called for " << cpi.name << ", but it was not found in control_protocols" << endl;
 		}
-
-		list<ControlProtocolInfo*>::iterator p2 = find (control_protocol_info.begin(), control_protocol_info.end(), &cpi);
-		if (p2 != control_protocol_info.end()) {
-			control_protocol_info.erase (p2);
-		} else {
-			cerr << "Programming error: ControlProtocolManager::teardown() called for " << cpi.name << ", but it was not found in control_protocol_info" << endl;
-		}
 	}
 
 	cpi.protocol = 0;

libs/ardour/midi_ui.cc

@@ -64,6 +64,10 @@ MidiControlUI::do_request (MidiUIRequest* req)
 	} else if (req->type == CallSlot) {
 
 		req->the_slot ();
+
+	} else if (req->type == Quit) {
+
+		BaseUI::quit ();
 	}
 }
 
@@ -102,8 +106,8 @@ void
 MidiControlUI::clear_ports ()
 {
 	for (PortSources::iterator i = port_sources.begin(); i != port_sources.end(); ++i) {
-		/* remove existing sources from the event loop */
+		g_source_destroy (*i);
-		(*i)->destroy ();
+		g_source_unref (*i);
 	}
 
 	port_sources.clear ();
@@ -120,13 +124,15 @@ MidiControlUI::reset_ports ()
 		int fd;
 		if ((fd = (*i)->selectable ()) >= 0) {
 			Glib::RefPtr<IOSource> psrc = IOSource::create (fd, IO_IN|IO_HUP|IO_ERR);
-			psrc->connect (bind (mem_fun (*this, &MidiControlUI::midi_input_handler), (*i)));
-			port_sources.push_back (psrc);
-		} 
-	}
 
-	for (PortSources::iterator i = port_sources.begin(); i != port_sources.end(); ++i) {
+			psrc->connect (bind (mem_fun (*this, &MidiControlUI::midi_input_handler), (*i)));
-		(*i)->attach (_main_loop->get_context());
+			psrc->attach (_main_loop->get_context());
+
+			// glibmm hack: for now, store only the GSource*
+
+			port_sources.push_back (psrc->gobj());
+			g_source_ref (psrc->gobj());
+		} 
 	}
 }
 

libs/gtkmm2ext/gtk_ui.cc

@@ -50,7 +50,6 @@ using std::map;
 UI       *UI::theGtkUI = 0;
 
 BaseUI::RequestType Gtkmm2ext::ErrorMessage = BaseUI::new_request_type();
-BaseUI::RequestType Gtkmm2ext::Quit = BaseUI::new_request_type();
 BaseUI::RequestType Gtkmm2ext::TouchDisplay = BaseUI::new_request_type();
 BaseUI::RequestType Gtkmm2ext::StateChange = BaseUI::new_request_type();
 BaseUI::RequestType Gtkmm2ext::SetTip = BaseUI::new_request_type();

libs/gtkmm2ext/gtkmm2ext/gtk_ui.h

@@ -51,7 +51,6 @@ namespace Gtkmm2ext {
 class TextViewer;
 
 extern BaseUI::RequestType ErrorMessage;
-extern BaseUI::RequestType Quit;
 extern BaseUI::RequestType CallSlot;
 extern BaseUI::RequestType TouchDisplay;
 extern BaseUI::RequestType StateChange;

libs/pbd/base_ui.cc

@@ -37,11 +37,14 @@ using namespace Glib;
 	
 uint64_t BaseUI::rt_bit = 1;
 BaseUI::RequestType BaseUI::CallSlot = BaseUI::new_request_type();
+BaseUI::RequestType BaseUI::Quit = BaseUI::new_request_type();
 
 BaseUI::BaseUI (const string& str)
 	: run_loop_thread (0)
 	, _name (str)
 {
+	cerr << "New BUI called " << _name << " @ " << this << endl;
+
 	base_ui_instance = this;
 
 	request_channel.ios()->connect (sigc::mem_fun (*this, &BaseUI::request_handler));
@@ -77,19 +80,24 @@ void
 BaseUI::run ()
 {
 	/* to be called by UI's that need/want their own distinct, self-created event loop thread.
-	   Derived classes should have set up a handler for IO on request_channel.ios()
 	*/
 
 	_main_loop = MainLoop::create (MainContext::create());
 	request_channel.ios()->attach (_main_loop->get_context());
+
+	/* glibmm hack - drop the refptr to the IOSource now before it can hurt */
+	request_channel.drop_ios ();
+
 	run_loop_thread = Thread::create (mem_fun (*this, &BaseUI::main_thread), true);
 }
 
 void
 BaseUI::quit ()
 {
-	_main_loop->quit ();
+	if (_main_loop->is_running()) {
-	run_loop_thread->join ();
+		_main_loop->quit ();
+		run_loop_thread->join ();
+	}
 }
 
 bool

libs/pbd/crossthread.cc

@@ -32,6 +32,7 @@ using namespace Glib;
 
 CrossThreadChannel::CrossThreadChannel ()
 {
+	_ios = 0;
 	fds[0] = -1;
 	fds[1] = -1;
 
@@ -49,12 +50,12 @@ CrossThreadChannel::CrossThreadChannel ()
 		error << "cannot set non-blocking mode for x-thread pipe (write) (%2)" << ::strerror (errno) << ')' << endmsg;
 		return;
 	}
-
 }
 
 CrossThreadChannel::~CrossThreadChannel ()
 {
-	_ios->destroy ();
+	/* glibmm hack */
+	drop_ios ();
 
 	if (fds[0] >= 0) {
 		close (fds[0]);
@@ -78,11 +79,18 @@ RefPtr<IOSource>
 CrossThreadChannel::ios () 
 {
 	if (!_ios) {
-		_ios = IOSource::create (fds[0], IOCondition(IO_IN|IO_PRI|IO_ERR|IO_HUP|IO_NVAL));
+		_ios = new RefPtr<IOSource> (IOSource::create (fds[0], IOCondition(IO_IN|IO_PRI|IO_ERR|IO_HUP|IO_NVAL)));
 	}
-	return _ios;
+	return *_ios;
 }
-	
+
+void
+CrossThreadChannel::drop_ios ()
+{
+	delete _ios;
+	_ios = 0;
+}
+
 void
 CrossThreadChannel::drain ()
 {

libs/pbd/pbd/abstract_ui.cc

@@ -17,7 +17,7 @@ AbstractUI<RequestObject>::AbstractUI (const string& name)
 }
 
 template <typename RequestObject> void
-AbstractUI<RequestObject>::register_thread (string target_gui, pthread_t thread_id, string /*thread_name*/, uint32_t num_requests)
+AbstractUI<RequestObject>::register_thread (string target_gui, pthread_t thread_id, string thread_name, uint32_t num_requests)
 {
 	if (target_gui != name()) {
 		return;

libs/pbd/pbd/base_ui.h

@@ -57,6 +57,7 @@ class BaseUI : virtual public sigc::trackable {
 
 	static RequestType new_request_type();
 	static RequestType CallSlot;
+	static RequestType Quit;
 
 	void run ();
 	void quit ();

libs/pbd/pbd/crossthread.h

@@ -33,11 +33,21 @@ class CrossThreadChannel {
 	void drain ();
 	static void drain (int fd);
 
+	/* glibmm 2.22 and earlier has a terrifying bug that will
+	   cause crashes whenever a Source is removed from
+	   a MainContext (including the destruction of the MainContext),
+	   because the Source is destroyed "out from under the nose of" 
+	   the RefPtr. I (Paul) have fixed this (https://bugzilla.gnome.org/show_bug.cgi?id=561885)
+	   but in the meantime, we need a hack to get around the issue.
+	*/
+
 	Glib::RefPtr<Glib::IOSource> ios();
+	void drop_ios ();
+
 	bool ok() const { return fds[0] >= 0 && fds[1] >= 0; }
 
   private:
-	Glib::RefPtr<Glib::IOSource> _ios; // lazily constructed
+	Glib::RefPtr<Glib::IOSource>* _ios; // lazily constructed
 	int fds[2];
 };
 

libs/pbd/pbd/pthread_utils.h

@@ -29,6 +29,7 @@
 
 int  pthread_create_and_store (std::string name, pthread_t  *thread, void * (*start_routine)(void *), void * arg);
 void pthread_cancel_one (pthread_t thread);
+void pthread_cancel_all ();
 void pthread_kill_all (int signum);
 void pthread_exit_pbd (void* status);
 std::string pthread_name ();

libs/pbd/pthread_utils.cc

@@ -123,6 +123,19 @@ pthread_kill_all (int signum)
 	pthread_mutex_unlock (&thread_map_lock);
 }
 
+void
+pthread_cancel_all () 
+{	
+	pthread_mutex_lock (&thread_map_lock);
+	for (ThreadMap::iterator i = all_threads.begin(); i != all_threads.end(); ++i) {
+		if (i->second != pthread_self()) {
+			pthread_cancel (i->second);
+		}
+	}
+	all_threads.clear();
+	pthread_mutex_unlock (&thread_map_lock);
+}
+
 void
 pthread_cancel_one (pthread_t thread) 
 {	

libs/surfaces/control_protocol/control_protocol/control_protocol.h

@@ -34,7 +34,7 @@ namespace ARDOUR {
 class Route;
 class Session;
 
-class ControlProtocol : public sigc::trackable, public PBD::Stateful, public BasicUI {
+class ControlProtocol : virtual public sigc::trackable, public PBD::Stateful, public BasicUI {
   public:
 	ControlProtocol (Session&, std::string name);
 	virtual ~ControlProtocol();

libs/surfaces/osc/interface.cc

@@ -27,7 +27,7 @@ ControlProtocol*
 new_osc_protocol (ControlProtocolDescriptor* /*descriptor*/, Session* s)
 {
 	OSC* osc = new OSC (*s, Config->get_osc_port());
-		
+	
 	osc->set_active (true);
 
 	return osc;

libs/surfaces/osc/osc.cc

@@ -49,8 +49,11 @@
 using namespace ARDOUR;
 using namespace sigc;
 using namespace std;
+using namespace Glib;
 
 
+#include "pbd/abstract_ui.cc" // instantiate template
+
 #ifdef DEBUG
 static void error_callback(int num, const char *m, const char *path)
 {
@@ -65,25 +68,22 @@ static void error_callback(int, const char *, const char *)
 
 OSC::OSC (Session& s, uint32_t port)
 	: ControlProtocol (s, "OSC")
+	, AbstractUI<OSCUIRequest> ("osc")
 	, _port(port)
 {
 	_shutdown = false;
 	_osc_server = 0;
 	_osc_unix_server = 0;
-	_osc_thread = 0;
 	_namespace_root = "/ardour";
 	_send_route_changes = true;
 
+	/* glibmm hack */
+	local_server = 0;
+	remote_server = 0;
+
 	// "Application Hooks"
 	session_loaded (s);
-	session->Exported.connect( mem_fun( *this, &OSC::session_exported ) );
+	session->Exported.connect (mem_fun (*this, &OSC::session_exported));
-
-	/* catch up with existing routes */
-
-	boost::shared_ptr<RouteList> rl = session->get_routes ();
-	route_added (*(rl.get()));
-
-	// session->RouteAdded.connect (mem_fun (*this, &OSC::route_added));
 }
 
 OSC::~OSC()
@@ -91,6 +91,19 @@ OSC::~OSC()
 	stop ();
 }
 
+void
+OSC::do_request (OSCUIRequest* req)
+{
+	if (req->type == CallSlot) {
+
+		call_slot (req->the_slot);
+
+	} else if (req->type == Quit) {
+
+		stop ();
+	}
+}
+
 int
 OSC::set_active (bool yn)
 {
@@ -187,35 +200,79 @@ OSC::start ()
 	register_callbacks();
 	
 	// lo_server_thread_add_method(_sthread, NULL, NULL, OSC::_dummy_handler, this);
-		
+
-	if (!init_osc_thread()) {
+	/* startup the event loop thread */
-		return -1;
+
-	}
+	BaseUI::run ();
+
 	return 0;
 }
 
+void
+OSC::thread_init ()
+{
+	if (_osc_unix_server) {
+		Glib::RefPtr<IOSource> src = IOSource::create (lo_server_get_socket_fd (_osc_unix_server), IO_IN|IO_HUP|IO_ERR);
+		src->connect (bind (sigc::mem_fun (*this, &OSC::osc_input_handler), _osc_unix_server));
+		src->attach (_main_loop->get_context());
+		local_server = src->gobj();
+		g_source_ref (local_server);
+	}
+
+	if (_osc_server) {
+		Glib::RefPtr<IOSource> src  = IOSource::create (lo_server_get_socket_fd (_osc_server), IO_IN|IO_HUP|IO_ERR);
+		src->connect (bind (sigc::mem_fun (*this, &OSC::osc_input_handler), _osc_server));
+		src->attach (_main_loop->get_context());
+		remote_server = src->gobj();
+		g_source_ref (remote_server);
+	}
+}
+
 int
 OSC::stop ()
 {	
-	if (_osc_server == 0) {
+	/* stop main loop */
-		/* already stopped */
+
-		return 0;
+	if (local_server) {
+		g_source_destroy (local_server);
+		g_source_unref (local_server);
+		local_server = 0;
 	}
 
-	// stop server thread
+	if (remote_server) {
-	terminate_osc_thread();
+		g_source_destroy (remote_server);
+		g_source_unref (remote_server);
+		remote_server = 0;
+	}
 
-	lo_server_free (_osc_server);
+	BaseUI::quit ();
-	_osc_server = 0;
+
+	if (_osc_server) {
+		int fd = lo_server_get_socket_fd(_osc_server);
+		if (fd >=0) {
+			close(fd);
+		}
+		lo_server_free (_osc_server);
+		_osc_server = 0;
+	}
+
+	if (_osc_unix_server) {
+		int fd = lo_server_get_socket_fd(_osc_unix_server);
+		if (fd >=0) {
+			close(fd);
+		}
+		lo_server_free (_osc_unix_server);
+		_osc_unix_server = 0;
+	}
 	
 	if (!_osc_unix_socket_path.empty()) {
-		// unlink it
+		unlink (_osc_unix_socket_path.c_str());
-		unlink(_osc_unix_socket_path.c_str());
 	}
 	
-	if (!  _osc_url_file.empty() ) {
+	if (!_osc_url_file.empty() ) {
-		unlink(_osc_url_file.c_str() );
+		unlink (_osc_url_file.c_str() );
 	}
+
 	return 0;
 }
 
@@ -268,7 +325,9 @@ OSC::register_callbacks()
 		REGISTER_CALLBACK (serv, "/ardour/routes/gainabs", "if", route_set_gain_abs);
 		REGISTER_CALLBACK (serv, "/ardour/routes/gaindB", "if", route_set_gain_dB);
 
+		
 #if 0
+		/* still not-really-standardized query interface */
 		REGISTER_CALLBACK (serv, "/ardour/*/#current_value", "", current_value);
 		REGISTER_CALLBACK (serv, "/ardour/set", "", set);
 #endif
@@ -284,56 +343,19 @@ OSC::register_callbacks()
 }
 
 bool
-OSC::init_osc_thread ()
+OSC::osc_input_handler (IOCondition ioc, lo_server srv)
 {
-	// create new thread to run server
+	if (ioc & ~IO_IN) {
-	if (pipe (_request_pipe)) {
-		cerr << "Cannot create osc request signal pipe" <<  strerror (errno) << endl;
 		return false;
 	}
 
-	if (fcntl (_request_pipe[0], F_SETFL, O_NONBLOCK)) {
+	if (ioc & IO_IN) {
-		cerr << "osc: cannot set O_NONBLOCK on signal read pipe " << strerror (errno) << endl;
+		lo_server_recv (srv);
-		return false;
 	}
 
-	if (fcntl (_request_pipe[1], F_SETFL, O_NONBLOCK)) {
-		cerr << "osc: cannot set O_NONBLOCK on signal write pipe " << strerror (errno) << endl;
-		return false;
-	}
-	
-	pthread_create_and_store (X_("OSC"), &_osc_thread, &OSC::_osc_receiver, this);
-
-	if (!_osc_thread) {
-		return false;
-	}
-
-	//pthread_detach (_osc_thread);
 	return true;
 }
 
-void
-OSC::terminate_osc_thread ()
-{
-	void* status;
-
-	_shutdown = true;
-	
-	poke_osc_thread ();
-
-	pthread_join (_osc_thread, &status);
-}
-
-void
-OSC::poke_osc_thread ()
-{
-	char c;
-
-	if (write (_request_pipe[1], &c, 1) != 1) {
-		cerr << "cannot send signal to osc thread! " <<  strerror (errno) << endl;
-	}
-}
-
 std::string
 OSC::get_server_url()
 {
@@ -365,107 +387,6 @@ OSC::get_unix_server_url()
 }
 
 
-/* server thread */
-
-void *
-OSC::_osc_receiver(void * arg)
-{
-	static_cast<OSC*>(arg)->register_thread (X_("OSC"));
-	static_cast<OSC*>(arg)->osc_receiver();
-	return 0;
-}
-
-void
-OSC::osc_receiver()
-{
-	struct pollfd pfd[3];
-	int fds[3];
-	lo_server srvs[3];
-	int nfds = 0;
-	int timeout = -1;
-	int ret;
-	
-	fds[0] = _request_pipe[0];
-	nfds++;
-	
-	if (_osc_server && lo_server_get_socket_fd(_osc_server) >= 0) {
-		fds[nfds] = lo_server_get_socket_fd(_osc_server);
-		srvs[nfds] = _osc_server;
-		nfds++;
-	}
-
-	if (_osc_unix_server && lo_server_get_socket_fd(_osc_unix_server) >= 0) {
-		fds[nfds] = lo_server_get_socket_fd(_osc_unix_server);
-		srvs[nfds] = _osc_unix_server;
-		nfds++;
-	}
-	
-	
-	while (!_shutdown) {
-
-		for (int i=0; i < nfds; ++i) {
-			pfd[i].fd = fds[i];
-			pfd[i].events = POLLIN|POLLPRI|POLLHUP|POLLERR;
-			pfd[i].revents = 0;
-		}
-		
-	again:
-		//cerr << "poll on " << nfds << " for " << timeout << endl;
-		if ((ret = poll (pfd, nfds, timeout)) < 0) {
-			if (errno == EINTR) {
-				/* gdb at work, perhaps */
-				goto again;
-			}
-			
-			cerr << "OSC thread poll failed: " <<  strerror (errno) << endl;
-			
-			break;
-		}
-
-		//cerr << "poll returned " << ret << "  pfd[0].revents = " << pfd[0].revents << "  pfd[1].revents = " << pfd[1].revents << endl;
-		
-		if (_shutdown) {
-			break;
-		}
-		
-		if ((pfd[0].revents & ~POLLIN)) {
-			cerr << "OSC: error polling extra port" << endl;
-			break;
-		}
-		
-		for (int i=1; i < nfds; ++i) {
-			if (pfd[i].revents & POLLIN)
-			{
-				// this invokes callbacks
-				// cerr << "invoking recv on " << pfd[i].fd << endl;
-				lo_server_recv(srvs[i]);
-			}
-		}
-
-	}
-
-	//cerr << "SL engine shutdown" << endl;
-	
-	if (_osc_server) {
-		int fd = lo_server_get_socket_fd(_osc_server);
-		if (fd >=0) {
-			// hack around
-			close(fd);
-		}
-		lo_server_free (_osc_server);
-		_osc_server = 0;
-	}
-
-	if (_osc_unix_server) {
-		cerr << "freeing unix server" << endl;
-		lo_server_free (_osc_unix_server);
-		_osc_unix_server = 0;
-	}
-	
-	close(_request_pipe[0]);
-	close(_request_pipe[1]);
-}
-
 void
 OSC::current_value_query (const char* path, size_t len, lo_arg **argv, int argc, lo_message msg)
 {
@@ -600,11 +521,6 @@ OSC::catchall (const char *path, const char *types, lo_arg **argv, int argc, lo_
 	return ret;
 }
 
-void
-OSC::route_added (RouteList&)
-{
-}
-
 void
 OSC::listen_to_route (boost::shared_ptr<Route> route, lo_address addr)
 {

libs/surfaces/osc/osc.h

@@ -29,8 +29,12 @@
 
 #include <lo/lo.h>
 
+#include <glibmm/main.h>
+
 #include <sigc++/sigc++.h>
 
+#include "pbd/abstract_ui.h"
+
 #include "ardour/types.h"
 #include "control_protocol/control_protocol.h"
 
@@ -41,7 +45,18 @@ class Session;
 class Route;
 }
 	
-class OSC : public ARDOUR::ControlProtocol
+/* this is mostly a placeholder because I suspect that at some
+   point we will want to add more members to accomodate
+   certain types of requests to the MIDI UI
+*/
+
+struct OSCUIRequest : public BaseUI::BaseRequestObject {
+  public:
+	OSCUIRequest () {}
+	~OSCUIRequest() {}
+};
+
+class OSC : public ARDOUR::ControlProtocol, public AbstractUI<OSCUIRequest>
 {
   public:
 	OSC (ARDOUR::Session&, uint32_t port);
@@ -60,6 +75,15 @@ class OSC : public ARDOUR::ControlProtocol
 	int start ();
 	int stop ();
 
+  protected:
+        void thread_init ();
+	void do_request (OSCUIRequest*);
+
+	GSource* local_server;
+	GSource* remote_server;
+	
+	bool osc_input_handler (Glib::IOCondition, lo_server);
+
   private:
 	uint32_t _port;
 	volatile bool _ok;
@@ -70,16 +94,6 @@ class OSC : public ARDOUR::ControlProtocol
 	std::string _osc_url_file;
 	std::string _namespace_root;
 	bool _send_route_changes;
-	pthread_t _osc_thread;
-	int _request_pipe[2];
-
-	static void * _osc_receiver(void * arg);
-	void osc_receiver();
-	void send(); // This should accept an OSC payload
-
-	bool init_osc_thread ();
-	void terminate_osc_thread ();
-	void poke_osc_thread ();
 
 	void register_callbacks ();
 

libs/surfaces/osc/wscript

@@ -32,7 +32,7 @@ def build(bld):
 	obj.name         = 'libardour_osc'
 	obj.target       = 'osc'
 	obj.uselib       = ' LO '
-	obj.uselib_local = 'libardour libardour_cp'
+	obj.uselib_local = 'libardour libardour_cp libpbd'
 	obj.vnum         = LIBARDOUR_OSC_LIB_VERSION
 	obj.install_path = os.path.join(bld.env['LIBDIR'], 'ardour3', 'surfaces')