Fix deadlock when changing CPL in the player (#1827).
authorCarl Hetherington <cth@carlh.net>
Sat, 17 Oct 2020 20:23:12 +0000 (22:23 +0200)
committerCarl Hetherington <cth@carlh.net>
Sat, 17 Oct 2020 20:23:12 +0000 (22:23 +0200)
TextContent::set_dcp_track can end up requesting a view update, which
involves calls to methods in Content which lock the Content::_mutex.
Do these calls without a lock on that mutex held.

Also, it looks like we would append to texts on every call to
examine().  Fix that so that we replace the texts list on each
examine() call.

src/lib/dcp_content.cc

index 0bef73f77d48cf1ce2e40b217af5bd206db07d28..f507784208444f4c152253aa1909769a708ad32e 100644 (file)
@@ -202,7 +202,6 @@ DCPContent::examine (shared_ptr<const Film> film, shared_ptr<Job> job)
        bool const needed_assets = needs_assets ();
        bool const needed_kdm = needs_kdm ();
        string const old_name = name ();
-       int const old_texts = text.size ();
 
        ChangeSignaller<Content> cc_texts (this, DCPContentProperty::TEXTS);
        ChangeSignaller<Content> cc_assets (this, DCPContentProperty::NEEDS_ASSETS);
@@ -249,20 +248,21 @@ DCPContent::examine (shared_ptr<const Film> film, shared_ptr<Job> job)
                atmos->set_length (examiner->atmos_length());
        }
 
-       int texts = 0;
+       list<shared_ptr<TextContent> > new_text;
+       for (int i = 0; i < TEXT_COUNT; ++i) {
+               for (int j = 0; j < examiner->text_count(static_cast<TextType>(i)); ++j) {
+                       shared_ptr<TextContent> c(new TextContent(this, static_cast<TextType>(i), static_cast<TextType>(i)));
+                       if (i == TEXT_CLOSED_CAPTION) {
+                               c->set_dcp_track (examiner->dcp_text_track(j));
+                       }
+                       new_text.push_back (c);
+               }
+       }
+
        {
                boost::mutex::scoped_lock lm (_mutex);
+               text = new_text;
                _name = examiner->name ();
-               for (int i = 0; i < TEXT_COUNT; ++i) {
-                       for (int j = 0; j < examiner->text_count(static_cast<TextType>(i)); ++j) {
-                               shared_ptr<TextContent> c(new TextContent(this, static_cast<TextType>(i), static_cast<TextType>(i)));
-                               if (i == TEXT_CLOSED_CAPTION) {
-                                       c->set_dcp_track (examiner->dcp_text_track(j));
-                               }
-                               text.push_back (c);
-                       }
-               }
-               texts = text.size ();
                _encrypted = examiner->encrypted ();
                _needs_assets = examiner->needs_assets ();
                _kdm_valid = examiner->kdm_valid ();
@@ -279,10 +279,6 @@ DCPContent::examine (shared_ptr<const Film> film, shared_ptr<Job> job)
                _content_versions = examiner->content_versions ();
        }
 
-       if (old_texts == texts) {
-               cc_texts.abort ();
-       }
-
        if (needed_assets == needs_assets()) {
                cc_assets.abort ();
        }