Too many drop-references handlers (#9363)

This fixes a random crash with stop-and-forget capture.

When aborting capture, the disk-writer can emit
 midi_write_source->drop_references ()
in the butler thread, which leads to a direct call to
Session::remove_source.

This can happen before or after Region::source_deleted
is called which is initiated from the same signal.

Furthermore it was possible that Region::source deleted
was called concurrently from the UI thread via SourceRemoved
for whole file regions, which lead to memory corruption.
This commit is contained in:
Robin Gareus 2023-06-09 16:10:28 +02:00
parent 03e0eea744
commit ad49de022a
Signed by: rgareus
GPG Key ID: A090BCE02CF57F04
5 changed files with 19 additions and 18 deletions

View File

@ -545,6 +545,8 @@ private:
void register_properties ();
void use_sources (SourceList const &);
std::atomic<int> _source_deleted;
};
} /* namespace ARDOUR */

View File

@ -116,11 +116,14 @@ AudioSource::AudioSource (Session& s, const XMLNode& node)
AudioSource::~AudioSource ()
{
/* shouldn't happen but make sure we don't leak file descriptors anyway */
#ifndef NDEBUG
/* shouldn't happen but make sure we don't leak file descriptors anyway,
* (it can happen with stop-and-forget capture)
*/
if (peak_leftover_cnt) {
cerr << "AudioSource destroyed with leftover peak data pending" << endl;
}
#endif
if (-1 != _peakfile_fd) {
close (_peakfile_fd);

View File

@ -246,7 +246,7 @@ Region::register_properties ()
Region::Region (Session& s, timepos_t const & start, timecnt_t const & length, const string& name, DataType type)
: SessionObject(s, name)
, _type (type)
, REGION_DEFAULT_STATE (start,length)
, REGION_DEFAULT_STATE (start,length)
, _last_length (length)
, _first_edit (EditChangesNothing)
, _layer (0)
@ -1552,6 +1552,9 @@ Region::size_equivalent (std::shared_ptr<const Region> other) const
void
Region::source_deleted (std::weak_ptr<Source>)
{
if (_source_deleted.fetch_add (1)) {
return;
}
drop_sources ();
if (!_session.deletion_in_progress()) {
@ -1957,6 +1960,7 @@ Region::drop_sources ()
void
Region::use_sources (SourceList const & s)
{
_source_deleted.store (0);
set<std::shared_ptr<Source> > unique_srcs;
for (SourceList::const_iterator i = s.begin (); i != s.end(); ++i) {

View File

@ -4666,8 +4666,6 @@ Session::remove_source (std::weak_ptr<Source> src, bool drop_references)
return;
}
assert (!source->used ());
if (!in_cleanup () && !loading ()) {
/* save state so we don't end up with a session file
* referring to non-existent sources.

View File

@ -96,6 +96,7 @@ Source::Source (Session& s, const XMLNode& node)
Source::~Source ()
{
DEBUG_TRACE (DEBUG::Destruction, string_compose ("Source %1 destructor %2\n", _name, this));
assert (!used ());
}
void
@ -448,20 +449,13 @@ Source::dec_use_count ()
{
#ifndef NDEBUG
int oldval = _use_count.fetch_sub (1);
if (oldval <= 0) {
cerr << "Bad use dec for " << name() << endl;
abort ();
}
assert (oldval > 0);
#else
_use_count.fetch_sub (1);
#endif
try {
std::shared_ptr<Source> sptr = shared_from_this();
} catch (...) {
/* no shared_ptr available, relax; */
if (oldval <= 0) {
cerr << "Bad use dec for " << name() << endl;
}
assert (oldval > 0);
#else
_use_count.fetch_sub (1);
#endif
}
bool