Fix a rare EventList race-condition/crash
authorRobin Gareus <robin@gareus.org>
Thu, 17 Oct 2019 23:41:18 +0000 (01:41 +0200)
committerRobin Gareus <robin@gareus.org>
Thu, 17 Oct 2019 23:41:18 +0000 (01:41 +0200)
The GUI thread may modify fade-in/out while the butler-thread
reads audio.

e.g. select a Range and click delete.
---
Thread 0:: Dispatch queue: com.apple.main-thread
0   libsystem_pthread.dylib           0x00007fffd45924fc pthread_mutex_lock + 0
1   libglib-2.0.0.dylib               0x00000001085a9d2a g_mutex_lock + 26
2   libevoral.dylib                   0x0000000107fd0a49 PBD::Signal0<void, PBD::OptionalLastValue<void> >::operator()() + 57
3   libevoral.dylib                   0x0000000107fd486d Evoral::ControlList::clear() + 253
4   libardour.dylib                   0x00000001072ef9a5 ARDOUR::AudioRegion::set_fade_out(ARDOUR::FadeShape, long long) + 309
5   libardour.dylib                   0x00000001072f19ea ARDOUR::AudioRegion::recompute_at_end() + 122
6   libpbd.dylib                      0x00000001082993ff PBD::Stateful::resume_property_changes() + 191
7   libardour.dylib                   0x00000001076476af ARDOUR::Playlist::cut(long long, long long, bool) + 575
8   libardour.dylib                   0x0000000107646b5b ARDOUR::Playlist::cut_copy(boost::shared_ptr<ARDOUR::Playlist> (ARDOUR::Playlist::*)(long long, long long, bool), std::__1::list<ARDOUR::AudioRange, std::__1::allocator<ARDOUR::AudioRange> >&, bool) + 187
9   libardour.dylib                   0x0000000107647461 ARDOUR::Playlist::cut(std::__1::list<ARDOUR::AudioRange, std::__1::allocator<ARDOUR::AudioRange> >&, bool) + 33
10  Ardour.bin                        0x00000001065f0fa0 RouteTimeAxisView::cut_copy_clear(Selection&, Editing::CutCopyOp) + 592
11  Ardour.bin                        0x0000000106118a94 Editor::cut_copy_ranges(Editing::CutCopyOp) + 164
12  Ardour.bin                        0x0000000106116053 Editor::cut_copy(Editing::CutCopyOp) + 1587

Thread 20 Crashed:
0   libardour.dylib                   0x00000001072f4b19 ARDOUR::AudioRegion::body_range() const + 89
1   libardour.dylib                   0x00000001072bd318 ARDOUR::AudioPlaylist::read(float*, float*, float*, long long, long long, unsigned int) + 1176
2   libardour.dylib                   0x00000001072ac236 ARDOUR::AudioDiskstream::read(float*, float*, float*, long long&, long long, int, bool) + 854
3   libardour.dylib                   0x00000001072abbb8 ARDOUR::AudioDiskstream::overwrite_existing_buffers() + 392
4   libardour.dylib                   0x00000001077ef36a ARDOUR::Session::non_realtime_overwrite(int, bool&) + 186
5   libardour.dylib                   0x00000001077ed7f0 ARDOUR::Session::butler_transport_work() + 1696
6   libardour.dylib                   0x0000000107323425 ARDOUR::Butler::thread_work() + 149
7   libardour.dylib                   0x000000010732334f ARDOUR::Butler::_thread_work(void*) + 95

libs/ardour/audioregion.cc

index 4455c217947bc9836c670e319c9e6868b227eaa5..6bd1e6e0bbb3c8690659a59016fbee825c9b4f9a 100644 (file)
@@ -88,7 +88,8 @@ namespace ARDOUR {
 static void
 reverse_curve (boost::shared_ptr<Evoral::ControlList> dst, boost::shared_ptr<const Evoral::ControlList> src)
 {
-       size_t len = src->back()->when;
+       size_t len = src->when(false);
+       // TODO read-lock of src (!)
        for (Evoral::ControlList::const_reverse_iterator it = src->rbegin(); it!=src->rend(); it++) {
                dst->fast_simple_add (len - (*it)->when, (*it)->value);
        }
@@ -353,7 +354,7 @@ AudioRegion::post_set (const PropertyChange& /*ignored*/)
        /* return to default fades if the existing ones are too long */
 
        if (_left_of_split) {
-               if (_fade_in->back()->when >= _length) {
+               if (_fade_in->when(false) >= _length) {
                        set_default_fade_in ();
                }
                set_default_fade_out ();
@@ -361,7 +362,7 @@ AudioRegion::post_set (const PropertyChange& /*ignored*/)
        }
 
        if (_right_of_split) {
-               if (_fade_out->back()->when >= _length) {
+               if (_fade_out->when(false) >= _length) {
                        set_default_fade_out ();
                }
 
@@ -538,7 +539,7 @@ AudioRegion::read_at (Sample *buf, Sample *mixdown_buffer, float *gain_buffer,
 
        if (_fade_in_active && _session.config.get_use_region_fades()) {
 
-               samplecnt_t fade_in_length = (samplecnt_t) _fade_in->back()->when;
+               samplecnt_t fade_in_length = (samplecnt_t) _fade_in->when(false);
 
                /* see if this read is within the fade in */
 
@@ -570,7 +571,7 @@ AudioRegion::read_at (Sample *buf, Sample *mixdown_buffer, float *gain_buffer,
                 *
                 */
 
-               fade_interval_start = max (internal_offset, _length - samplecnt_t (_fade_out->back()->when));
+               fade_interval_start = max (internal_offset, _length - samplecnt_t (_fade_out->when(false)));
                samplecnt_t fade_interval_end = min(internal_offset + to_read, _length.val());
 
                if (fade_interval_end > fade_interval_start) {
@@ -661,7 +662,7 @@ AudioRegion::read_at (Sample *buf, Sample *mixdown_buffer, float *gain_buffer,
 
        if (fade_out_limit != 0) {
 
-               samplecnt_t const curve_offset = fade_interval_start - (_length - _fade_out->back()->when);
+               samplecnt_t const curve_offset = fade_interval_start - (_length - _fade_out->when(false));
 
                if (is_opaque) {
                        if (_inverse_fade_out) {
@@ -996,13 +997,13 @@ AudioRegion::fade_range (samplepos_t start, samplepos_t end)
 void
 AudioRegion::set_fade_in_shape (FadeShape shape)
 {
-       set_fade_in (shape, (samplecnt_t) _fade_in->back()->when);
+       set_fade_in (shape, (samplecnt_t) _fade_in->when(false));
 }
 
 void
 AudioRegion::set_fade_out_shape (FadeShape shape)
 {
-       set_fade_out (shape, (samplecnt_t) _fade_out->back()->when);
+       set_fade_out (shape, (samplecnt_t) _fade_out->when(false));
 }
 
 void
@@ -1237,13 +1238,13 @@ AudioRegion::set_fade_out_active (bool yn)
 bool
 AudioRegion::fade_in_is_default () const
 {
-       return _fade_in->size() == 2 && _fade_in->front()->when == 0 && _fade_in->back()->when == 64;
+       return _fade_in->size() == 2 && _fade_in->when(true) == 0 && _fade_in->when(false) == 64;
 }
 
 bool
 AudioRegion::fade_out_is_default () const
 {
-       return _fade_out->size() == 2 && _fade_out->front()->when == 0 && _fade_out->back()->when == 64;
+       return _fade_out->size() == 2 && _fade_out->when(true) == 0 && _fade_out->when(false) == 64;
 }
 
 void
@@ -1293,12 +1294,12 @@ AudioRegion::recompute_at_end ()
        if (_left_of_split) {
                set_default_fade_out ();
                _left_of_split = false;
-       } else if (_fade_out->back()->when > _length) {
+       } else if (_fade_out->when(false) > _length) {
                _fade_out->extend_to (_length);
                send_change (PropertyChange (Properties::fade_out));
        }
 
-       if (_fade_in->back()->when > _length) {
+       if (_fade_in->when(false) > _length) {
                _fade_in->extend_to (_length);
                send_change (PropertyChange (Properties::fade_in));
        }
@@ -1318,12 +1319,12 @@ AudioRegion::recompute_at_start ()
        if (_right_of_split) {
                set_default_fade_in ();
                _right_of_split = false;
-       } else if (_fade_in->back()->when > _length) {
+       } else if (_fade_in->when(false) > _length) {
                _fade_in->extend_to (_length);
                send_change (PropertyChange (Properties::fade_in));
        }
 
-       if (_fade_out->back()->when > _length) {
+       if (_fade_out->when(false) > _length) {
                _fade_out->extend_to (_length);
                send_change (PropertyChange (Properties::fade_out));
        }
@@ -1957,7 +1958,7 @@ AudioRegion::find_silence (Sample threshold, samplecnt_t min_length, samplecnt_t
 Evoral::Range<samplepos_t>
 AudioRegion::body_range () const
 {
-       return Evoral::Range<samplepos_t> (first_sample() + _fade_in->back()->when + 1, last_sample() - _fade_out->back()->when);
+       return Evoral::Range<samplepos_t> (first_sample() + _fade_in->when(false) + 1, last_sample() - _fade_out->when(false));
 }
 
 boost::shared_ptr<Region>