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.

diff --git a/src/lib/encoder.cc b/src/lib/encoder.cc
index 8f2aa58f70ebbe53db9fb1ef3bd3418d23af1a68..9ec817604a34e09ca98dc0c8dfb24a5f1606a441 100644 (file)
--- 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 b188e3be304ac4f8d3a04985cdd7218758d203f7..5816fe12f64c88c69d0c19ef47ece2b2744fe8e5 100644 (file)
--- 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;