From 795e5d916707da498acc7079ae1fb10aa6af6973 Mon Sep 17 00:00:00 2001
From: Carl Hetherington <cth@carlh.net>
Date: Mon, 20 Jun 2016 15:02:15 +0100
Subject: Encoder shutdown fixes.

Two fixes here; prevent the servers-list-changed callback being
called when Encoder is being destroyed, and stop ~Encoder throwing
exceptions.

I'm not sure if the catch (...) in ~Encoder will hide problems
that we should be handling, but I think by the time ~Encoder
is happening we'll already have seen any exceptions that we
need to report.
---
 src/lib/encoder.cc | 33 ++++++++++++++++++++++++++++++---
 src/lib/encoder.h  | 10 +++++++---
 2 files changed, 37 insertions(+), 6 deletions(-)

(limited to 'src')

diff --git a/src/lib/encoder.cc b/src/lib/encoder.cc
index 8f2aa58f7..9ec817604 100644
--- a/src/lib/encoder.cc
+++ b/src/lib/encoder.cc
@@ -65,14 +65,37 @@ Encoder::Encoder (shared_ptr<const Film> film, shared_ptr<Writer> writer)
 
 Encoder::~Encoder ()
 {
-	terminate_threads ();
+	try {
+		terminate_threads ();
+	} catch (...) {
+		/* Destructors must not throw exceptions; anything bad
+		   happening now is too late to worry about anyway,
+		   I think.
+		*/
+	}
 }
 
 void
 Encoder::begin ()
 {
 	if (!EncodeServerFinder::instance()->disabled ()) {
-		_server_found_connection = EncodeServerFinder::instance()->ServersListChanged.connect (boost::bind (&Encoder::servers_list_changed, this));
+		weak_ptr<Encoder> wp = shared_from_this ();
+		_server_found_connection = EncodeServerFinder::instance()->ServersListChanged.connect (
+			boost::bind (&Encoder::call_servers_list_changed, wp)
+			);
+	}
+}
+
+/* We don't want the servers-list-changed callback trying to do things
+   during destruction of Encoder, and I think this is the neatest way
+   to achieve that.
+*/
+void
+Encoder::call_servers_list_changed (weak_ptr<Encoder> encoder)
+{
+	shared_ptr<Encoder> e = encoder.lock ();
+	if (e) {
+		e->servers_list_changed ();
 	}
 }
 
@@ -238,7 +261,11 @@ Encoder::terminate_threads ()
 		LOG_GENERAL ("Terminating thread %1 of %2", n + 1, _threads.size ());
 		(*i)->interrupt ();
 		DCPOMATIC_ASSERT ((*i)->joinable ());
-		(*i)->join ();
+		try {
+			(*i)->join ();
+		} catch (boost::thread_interrupted& e) {
+			/* This is to be expected */
+		}
 		delete *i;
 		LOG_GENERAL_NC ("Thread terminated");
 		++n;
diff --git a/src/lib/encoder.h b/src/lib/encoder.h
index b188e3be3..5816fe12f 100644
--- a/src/lib/encoder.h
+++ b/src/lib/encoder.h
@@ -1,5 +1,5 @@
 /*
-    Copyright (C) 2012-2015 Carl Hetherington <cth@carlh.net>
+    Copyright (C) 2012-2016 Carl Hetherington <cth@carlh.net>
 
     This file is part of DCP-o-matic.
 
@@ -34,6 +34,7 @@
 #include <boost/thread.hpp>
 #include <boost/optional.hpp>
 #include <boost/signals2.hpp>
+#include <boost/enable_shared_from_this.hpp>
 #include <list>
 #include <stdint.h>
 
@@ -51,7 +52,7 @@ class PlayerVideo;
  *  the work around threads and encoding servers.
  */
 
-class Encoder : public boost::noncopyable, public ExceptionStore
+class Encoder : public boost::noncopyable, public ExceptionStore, public boost::enable_shared_from_this<Encoder>
 {
 public:
 	Encoder (boost::shared_ptr<const Film>, boost::shared_ptr<Writer>);
@@ -69,13 +70,16 @@ public:
 	float current_encoding_rate () const;
 	int video_frames_enqueued () const;
 
+	void servers_list_changed ();
+
 private:
 
+	static void call_servers_list_changed (boost::weak_ptr<Encoder> encoder);
+
 	void frame_done ();
 
 	void encoder_thread (boost::optional<EncodeServerDescription>);
 	void terminate_threads ();
-	void servers_list_changed ();
 
 	/** Film that we are encoding */
 	boost::shared_ptr<const Film> _film;
-- 
cgit v1.2.3