diff options
| author | Carl Hetherington <cth@carlh.net> | 2020-01-28 23:23:47 +0100 |
|---|---|---|
| committer | Carl Hetherington <cth@carlh.net> | 2020-01-28 23:23:47 +0100 |
| commit | df48c75c38dd788835a93540aea243a2dac4bb10 (patch) | |
| tree | ddb89cf5bada00cfe4e77301c71123297bd24946 /src/lib | |
| parent | 95f4d8740de74cdecf6658ab2607c0af9732904e (diff) | |
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.
Diffstat (limited to 'src/lib')
| -rw-r--r-- | src/lib/player.cc | 10 | ||||
| -rw-r--r-- | src/lib/player.h | 4 |
2 files changed, 6 insertions, 8 deletions
diff --git a/src/lib/player.cc b/src/lib/player.cc index 657c1a8b4..62527e3eb 100644 --- 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 2fd7c8668..68531cfb3 100644 --- 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) */ |
