ardour/ardour
ardour/ardour
13
1
Fork 0
Code Pull Requests Releases Activity

Fix race condition when deleting tracks

Browse Source 
In rare cases it can happen that the GUI thread results in
a call to DropReferences(), while the backend (RCU) still has a
reference to the track and processes the track.

However the call to DropReferences, DiskIO processor will
have cleared the pointer to _track, leading to segfaults when
the processor runs.

Since the DIO processor is owned by the track, one cannot directly
pass a shared_ptr<Track>. That would keep the Track around forever.

However the DIO processor cannot exist without a track passing
a reference is acceptable.
This commit is contained in:
Robin Gareus 2021-02-14 21:43:06 +01:00
parent 3e9f142f1b
commit acae86781b
Signed by: rgareus
GPG Key ID: A090BCE02CF57F04
8 changed files with 34 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
libs/ardour/ardour/disk_io.h
View File

@ -59,12 +59,9 @@ public:
static const std::string state_node_name;
DiskIOProcessor (Session&, const std::string& name, Flag f);
DiskIOProcessor (Session&, Track&, const std::string& name, Flag f);
virtual ~DiskIOProcessor ();
void set_track (boost::shared_ptr<Track>);
void drop_track ();
static void set_buffering_parameters (BufferingPreset bp);
int set_block_size (pframes_t);
@ -121,7 +118,7 @@ protected:
bool in_set_state;
samplepos_t playback_sample;
bool _need_butler;
boost::shared_ptr<Track> _track;
Track& _track;
void init ();

2
libs/ardour/ardour/disk_reader.h
View File

@ -39,7 +39,7 @@ template <typename T> class MidiRingBuffer;
class LIBARDOUR_API DiskReader : public DiskIOProcessor
{
public:
DiskReader (Session&, std::string const& name, DiskIOProcessor::Flag f = DiskIOProcessor::Flag (0));
DiskReader (Session&, Track&, std::string const& name, DiskIOProcessor::Flag f = DiskIOProcessor::Flag (0));
~DiskReader ();
bool set_name (std::string const& str);

2
libs/ardour/ardour/disk_writer.h
View File

@ -35,7 +35,7 @@ class MidiSource;
class LIBARDOUR_API DiskWriter : public DiskIOProcessor
{
public:
DiskWriter (Session&, std::string const& name,
DiskWriter (Session&, Track&, std::string const& name,
DiskIOProcessor::Flag f = DiskIOProcessor::Flag (0));
~DiskWriter ();

7
libs/ardour/ardour/track.h
View File

@ -128,6 +128,13 @@ public:
void set_block_size (pframes_t);
/* used by DiskReader request_overwrite_buffer(), to create
* a SessionEvent with weak_ptr<> reference
*/
boost::shared_ptr<Track> shared_ptr () {
return boost::dynamic_pointer_cast<Track> (shared_from_this());
}
boost::shared_ptr<Playlist> playlist ();
void request_input_monitoring (bool);
void ensure_input_monitoring (bool);

19
libs/ardour/disk_io.cc
View File

@ -48,13 +48,14 @@ const string DiskIOProcessor::state_node_name = X_("DiskIOProcessor");
// PBD::Signal0<void> DiskIOProcessor::DiskOverrun;
// PBD::Signal0<void> DiskIOProcessor::DiskUnderrun;
DiskIOProcessor::DiskIOProcessor (Session& s, string const & str, Flag f)
DiskIOProcessor::DiskIOProcessor (Session& s, Track& t, string const & str, Flag f)
: Processor (s, str)
, _flags (f)
, _slaved (false)
, in_set_state (false)
, playback_sample (0)
, _need_butler (false)
, _track (t)
, channels (new ChannelList)
, _midi_buf (0)
, _samples_written_to_ringbuffer (0)
@ -341,22 +342,6 @@ DiskIOProcessor::ChannelInfo::~ChannelInfo ()
capture_transition_buf = 0;
}
void
DiskIOProcessor::drop_track ()
{
_track.reset ();
}
void
DiskIOProcessor::set_track (boost::shared_ptr<Track> t)
{
_track = t;
if (_track) {
_track->DropReferences.connect_same_thread (*this, boost::bind (&DiskIOProcessor::drop_track, this));
}
}
/** Get the start, end, and length of a location "atomically".
*
* Note: Locations don't get deleted, so all we care about when I say "atomic"

24
libs/ardour/disk_reader.cc
View File

@ -57,8 +57,8 @@ DiskReader::Declicker DiskReader::loop_declick_in;
DiskReader::Declicker DiskReader::loop_declick_out;
samplecnt_t DiskReader::loop_fade_length (0);
DiskReader::DiskReader (Session& s, string const& str, DiskIOProcessor::Flag f)
: DiskIOProcessor (s, str, f)
DiskReader::DiskReader (Session& s, Track& t, string const& str, DiskIOProcessor::Flag f)
: DiskIOProcessor (s, t, str, f)
, overwrite_sample (0)
, run_must_resolve (false)
, _declick_amp (s.nominal_sample_rate ())
@ -197,7 +197,7 @@ void
DiskReader::realtime_locate (bool for_loop_end)
{
if (!for_loop_end) {
boost::shared_ptr<MidiTrack> mt = boost::dynamic_pointer_cast<MidiTrack> (_track);
MidiTrack* mt = dynamic_cast<MidiTrack*> (&_track);
_tracker.resolve_notes (mt->immediate_events (), 0);
}
}
@ -239,7 +239,7 @@ DiskReader::adjust_buffering ()
void
DiskReader::playlist_modified ()
{
_session.request_overwrite_buffer (_track, PlaylistModified);
_session.request_overwrite_buffer (_track.shared_ptr (), PlaylistModified);
}
int
@ -260,7 +260,7 @@ DiskReader::use_playlist (DataType dt, boost::shared_ptr<Playlist> playlist)
* take care of the buffer refill. */
if (!(g_atomic_int_get (&_pending_overwrite) & PlaylistChanged) || prior_playlist) {
_session.request_overwrite_buffer (_track, PlaylistChanged);
_session.request_overwrite_buffer (_track.shared_ptr (), PlaylistChanged);
}
return 0;
@ -273,7 +273,7 @@ DiskReader::run (BufferSet& bufs, samplepos_t start_sample, samplepos_t end_samp
boost::shared_ptr<ChannelList> c = channels.reader ();
ChannelList::iterator chan;
sampleoffset_t disk_samples_to_consume;
MonitorState ms = _track->monitoring_state ();
MonitorState ms = _track.monitoring_state ();
const bool midi_only = (c->empty() || !_playlists[DataType::AUDIO]);
if (_active) {
@ -544,7 +544,7 @@ DiskReader::configuration_changed ()
return;
}
}
_session.request_overwrite_buffer (_track, LoopDisabled);
_session.request_overwrite_buffer (_track.shared_ptr (), LoopDisabled);
}
bool
@ -765,8 +765,8 @@ DiskReader::overwrite_existing_midi ()
RTMidiBuffer* mbuf = rt_midibuffer ();
if (mbuf) {
boost::shared_ptr<MidiTrack> mt = boost::dynamic_pointer_cast<MidiTrack> (_track);
MidiChannelFilter* filter = mt ? &mt->playback_filter () : 0;
MidiTrack* mt = dynamic_cast<MidiTrack*> (&_track);
MidiChannelFilter* filter = mt ? &mt->playback_filter () : 0;
PBD::Timing minsert;
minsert.start ();
@ -1358,7 +1358,7 @@ DiskReader::playlist_ranges_moved (list<Evoral::RangeMove<samplepos_t> > const&
return;
}
if (!_track || Config->get_automation_follows_regions () == false) {
if (Config->get_automation_follows_regions () == false) {
return;
}
@ -1371,7 +1371,7 @@ DiskReader::playlist_ranges_moved (list<Evoral::RangeMove<samplepos_t> > const&
}
/* move panner automation */
boost::shared_ptr<Pannable> pannable = _track->pannable ();
boost::shared_ptr<Pannable> pannable = _track.pannable ();
Evoral::ControlSet::Controls& c (pannable->controls ());
for (Evoral::ControlSet::Controls::iterator ci = c.begin (); ci != c.end (); ++ci) {
@ -1391,7 +1391,7 @@ DiskReader::playlist_ranges_moved (list<Evoral::RangeMove<samplepos_t> > const&
}
}
/* move processor automation */
_track->foreach_processor (boost::bind (&DiskReader::move_processor_automation, this, _1, movements_samples));
_track.foreach_processor (boost::bind (&DiskReader::move_processor_automation, this, _1, movements_samples));
}
void

16
libs/ardour/disk_writer.cc
View File

@ -46,8 +46,8 @@ using namespace std;
ARDOUR::samplecnt_t DiskWriter::_chunk_samples = DiskWriter::default_chunk_samples ();
PBD::Signal0<void> DiskWriter::Overrun;
DiskWriter::DiskWriter (Session& s, string const & str, DiskIOProcessor::Flag f)
: DiskIOProcessor (s, str, f)
DiskWriter::DiskWriter (Session& s, Track& t, string const & str, DiskIOProcessor::Flag f)
: DiskIOProcessor (s, t, str, f)
, _record_enabled (0)
, _record_safe (0)
, _capture_start_sample (0)
@ -547,7 +547,7 @@ DiskWriter::run (BufferSet& bufs, samplepos_t start_sample, samplepos_t end_samp
// Pump entire port buffer into the ring buffer (TODO: split cycles?)
MidiBuffer& buf = bufs.get_midi (0);
boost::shared_ptr<MidiTrack> mt = boost::dynamic_pointer_cast<MidiTrack>(_track);
MidiTrack* mt = dynamic_cast<MidiTrack*>(&_track);
MidiChannelFilter* filter = mt ? &mt->capture_filter() : 0;
assert (buf.size() == 0 || _midi_buf);
@ -1159,7 +1159,7 @@ DiskWriter::transport_stopped_wallclock (struct tm& when, time_t twhen, bool abo
if (as) {
audio_srcs.push_back (as);
as->update_header (capture_info.front()->start, when, twhen);
as->set_captured_for (_track->name());
as->set_captured_for (_track.name());
as->mark_immutable ();
Glib::DateTime tm (Glib::DateTime::create_now_local (mktime (&when)));
@ -1174,7 +1174,7 @@ DiskWriter::transport_stopped_wallclock (struct tm& when, time_t twhen, bool abo
if (_midi_write_source) {
midi_srcs.push_back (_midi_write_source);
_midi_write_source->set_captured_for (_track->name());
_midi_write_source->set_captured_for (_track.name());
}
(*chan)->write_source->stamp (twhen);
@ -1234,10 +1234,8 @@ DiskWriter::transport_stopped_wallclock (struct tm& when, time_t twhen, bool abo
_last_capture_sources.insert (_last_capture_sources.end(), midi_srcs.begin(), midi_srcs.end());
if (_track) {
_track->use_captured_sources (audio_srcs, capture_info);
_track->use_captured_sources (midi_srcs, capture_info);
}
_track.use_captured_sources (audio_srcs, capture_info);
_track.use_captured_sources (midi_srcs, capture_info);
mark_write_completed = true;

8
libs/ardour/track.cc
View File

@ -73,12 +73,10 @@ Track::~Track ()
DEBUG_TRACE (DEBUG::Destruction, string_compose ("track %1 destructor\n", _name));
if (_disk_reader) {
_disk_reader->set_track (boost::shared_ptr<Track>());
_disk_reader.reset ();
}
if (_disk_writer) {
_disk_writer->set_track (boost::shared_ptr<Track>());
_disk_writer.reset ();
}
}
@ -92,14 +90,12 @@ Track::init ()
DiskIOProcessor::Flag dflags = DiskIOProcessor::Recordable;
_disk_reader.reset (new DiskReader (_session, name(), dflags));
_disk_reader.reset (new DiskReader (_session, *this, name(), dflags));
_disk_reader->set_block_size (_session.get_block_size ());
_disk_reader->set_track (boost::dynamic_pointer_cast<Track> (shared_from_this()));
_disk_reader->set_owner (this);
_disk_writer.reset (new DiskWriter (_session, name(), dflags));
_disk_writer.reset (new DiskWriter (_session, *this, name(), dflags));
_disk_writer->set_block_size (_session.get_block_size ());
_disk_writer->set_track (boost::dynamic_pointer_cast<Track> (shared_from_this()));
_disk_writer->set_owner (this);
set_align_choice_from_io ();