From 4db1c02bd1a0e4da9efd725cd8fb098ba7c4abe5 Mon Sep 17 00:00:00 2001
From: Robin Gareus <robin@gareus.org>
Date: Fri, 7 May 2021 21:45:52 +0200
Subject: [PATCH] Fix crashes when rippling many regions

Region positions were updated in the GUI, before the playlist
was catching up.

The butler thread reads a region using the region's new position,
but the playlist's old range.


Thread 1 (GUI)
```
#22 ARDOUR::Playlist::notify_layering_changed()
#26 ARDOUR::AudioPlaylist::region_changed
#27 ARDOUR::Playlist::region_changed_proxy
#35 ARDOUR::Region::send_change
#36 ARDOUR::Region::set_position
#37 RegionRippleDrag::remove_unselected_from_views
#38 RegionRippleDrag::finished
```

LayeringChanged() also triggers DiskIOProcessor::playlist_modified
which schedules a pending-override and summons the butler.
Note that when moving only a few regions the butler starts after all
updates have been completed.

Butler thread:
```
#4  ARDOUR::AudioRegion::read_at
#5  ARDOUR::AudioPlaylist::read
#6  ARDOUR::DiskReader::audio_read
#7  ARDOUR::DiskReader::overwrite_existing_audio
#8  ARDOUR::DiskReader::overwrite_existing_buffers
#9  ARDOUR::Track::overwrite_existing_buffers
#10 ARDOUR::Session::non_realtime_overwrite
```

Region read fails:
```
 libs/ardour/audioregion.cc:503  assert (position >= _position);
(gdb) p position
$1 = 1312000
(gdb) p _position
$2 = {<PBD::PropertyTemplate<long>> = {_have_old = true,
    _current = 1336000, _old = 1312000} }
```
---
 gtk2_ardour/editor_drag.cc | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/gtk2_ardour/editor_drag.cc b/gtk2_ardour/editor_drag.cc
index b01b8fc2f6..39e9b817c1 100644
--- a/gtk2_ardour/editor_drag.cc
+++ b/gtk2_ardour/editor_drag.cc
@@ -2264,6 +2264,7 @@ RegionRippleDrag::add_all_after_to_views(TimeAxisView *tav, samplepos_t where, c
 void
 RegionRippleDrag::remove_unselected_from_views(samplecnt_t amount, bool move_regions)
 {
+	ThawList thawlist;
 
 	for (std::list<DraggingView>::iterator i = _views.begin(); i != _views.end(); ) {
 		// we added all the regions after the selection
@@ -2285,6 +2286,7 @@ RegionRippleDrag::remove_unselected_from_views(samplecnt_t amount, bool move_reg
 			rv->drag_end ();
 
 			if (move_regions) {
+				thawlist.add (rv->region ());
 				// move the underlying region to match the view
 				rv->region()->set_position (rv->region()->position() + amount);
 			} else {
@@ -2296,6 +2298,7 @@ RegionRippleDrag::remove_unselected_from_views(samplecnt_t amount, bool move_reg
 			_views.erase (to_erase);
 		}
 	}
+	thawlist.release ();
 }
 
 bool
@@ -2465,6 +2468,8 @@ RegionRippleDrag::finished (GdkEvent* event, bool movement_occurred)
 			// to add the original track to the undo record
 			orig_tav->playlist()->clear_changes();
 			orig_tav->playlist()->clear_owned_changes();
+			orig_tav->playlist()->freeze ();
+
 			remove_unselected_from_views (prev_amount, true);
 
 			std::list<boost::shared_ptr<Region> >::const_iterator it = _orig_tav_ripples.begin();
@@ -2486,6 +2491,8 @@ RegionRippleDrag::finished (GdkEvent* event, bool movement_occurred)
 				}
 			}
 
+			orig_tav->playlist()->thaw ();
+
 			vector<Command*> cmds;
 			orig_tav->playlist()->rdiff (cmds);
 			_editor->session()->add_commands (cmds);
@@ -2505,10 +2512,13 @@ RegionRippleDrag::finished (GdkEvent* event, bool movement_occurred)
 		for (pi = playlists.begin(); pi != playlists.end(); ++pi) {
 			(*pi)->clear_changes();
 			(*pi)->clear_owned_changes();
+			(*pi)->freeze();
 		}
+
 		remove_unselected_from_views (prev_amount, true);
 
 		for (pi = playlists.begin(); pi != playlists.end(); ++pi) {
+			(*pi)->thaw();
 			vector<Command*> cmds;
 			(*pi)->rdiff (cmds);
 			_editor->session()->add_commands (cmds);