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.
Backported from
df48c75c38dd788835a93540aea243a2dac4bb10 in v2.15.x
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)
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);
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) */