Fix deadlock during content examination.

Before this fix, the following situation could happen in threads
A and B:

A: Some DONE signal happens; this triggers setup_pieces which
   takes a lock on the player mutex.

B: FFmpegContent::examine takes a lock on the content mutex.
B: FFmpegContent::examine adds a stream
B: That causes STREAMS PENDING to be emitted.
B: This tries to take a lock on the player mutex so it can update _suspended

A: setup_pieces tries to access some content information, hence
   tries to take a lock on the content mutex.

Now B is holding the CL and awaiting the PL and A is holding
the PL and awaiting the CL.

It feels like the root cause of this is that while setup_pieces
is happening another change (which would itself cause setup_pieces)
is announced, and this isn't dealt with properly.

There are two steps here; _suspended is protected with an atomic
rather than using _mutex, and also it can cope with being updated
recursively.

diff --git a/src/lib/player.cc b/src/lib/player.cc
index 657c1a8b4ed9e2d168a2ffdbc156b46704824545..62527e3ebbf3b60408bd2cc967ffb568f529ddf8 100644 (file)
--- a/src/lib/player.cc
+++ b/src/lib/player.cc
@@ -87,7 +87,7 @@ int const PlayerProperty::DCP_DECODE_REDUCTION = 704;
 Player::Player (shared_ptr<const Film> film, shared_ptr<const Playlist> playlist)
        : _film (film)
        , _playlist (playlist)
-       , _suspended (false)
+       , _suspended (0)
        , _ignore_video (false)
        , _ignore_audio (false)
        , _ignore_text (false)
@@ -252,19 +252,17 @@ void
 Player::playlist_content_change (ChangeType type, int property, bool frequent)
 {
        if (type == CHANGE_TYPE_PENDING) {
-               boost::mutex::scoped_lock lm (_mutex);
                /* The player content is probably about to change, so we can't carry on
                   until that has happened and we've rebuilt our pieces.  Stop pass()
                   and seek() from working until then.
                */
-               _suspended = true;
+               ++_suspended;
        } else if (type == CHANGE_TYPE_DONE) {
                /* A change in our content has gone through.  Re-build our pieces. */
                setup_pieces ();
-               _suspended = false;
+               --_suspended;
        } else if (type == CHANGE_TYPE_CANCELLED) {
-               boost::mutex::scoped_lock lm (_mutex);
-               _suspended = false;
+               --_suspended;
        }
 
        Change (type, property, frequent);

diff --git a/src/lib/player.h b/src/lib/player.h
index 2fd7c8668a914125dc6308968393d3bf1d79c806..68531cfb377da8a5bbc7fb390f0108d06639aca9 100644 (file)
--- a/src/lib/player.h
+++ b/src/lib/player.h
@@ -149,8 +149,8 @@ private:
        boost::shared_ptr<const Film> _film;
        boost::shared_ptr<const Playlist> _playlist;
 
-       /** true if we are suspended (i.e. pass() and seek() do nothing) */
-       bool _suspended;
+       /** > 0 if we are suspended (i.e. pass() and seek() do nothing) */
+       boost::atomic<int> _suspended;
        std::list<boost::shared_ptr<Piece> > _pieces;
 
        /** Size of the image in the DCP (e.g. 1990x1080 for flat) */