allanger/ardour
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Activity

Use RWLock for Locations

Browse Source 
This replaces a Mutex and adds additional read-locks.

This is needed to address some threading issues with rt-threads
calling auto_loop_location() while the GUI changes locations.

Since locations are C-pointers this is still not entirely safe!
Locations::remove() may delete a location while a pointer
to it is being used in another thread.
This commit is contained in:
Robin Gareus
2021-05-09 02:44:03 +02:00
parent 600e959db5
commit 292547b264
2 changed files with 82 additions and 62 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
libs/ardour/ardour/location.h
View File

@@ -257,7 +257,7 @@ public:
*/
Locations::LocationList copy;
{
Glib::Threads::Mutex::Lock lm (lock);
Glib::Threads::RWLock::ReaderLock lm (_lock);
copy = locations;
}
(obj.*method)(copy);
@@ -266,7 +266,7 @@ public:
private:
LocationList locations;
Location* current_location;
mutable Glib::Threads::Mutex lock;
mutable Glib::Threads::RWLock _lock;
int set_current_unlocked (Location *);
void location_changed (Location*);

140
libs/ardour/location.cc
View File

@@ -791,6 +791,7 @@ Locations::Locations (Session& s)
Locations::~Locations ()
{
Glib::Threads::RWLock::WriterLock lm (_lock);
for (LocationList::iterator i = locations.begin(); i != locations.end(); ) {
LocationList::iterator tmp = i;
++tmp;
@@ -805,7 +806,7 @@ Locations::set_current (Location *loc, bool want_lock)
int ret;
if (want_lock) {
Glib::Threads::Mutex::Lock lm (lock);
Glib::Threads::RWLock::ReaderLock lm (_lock);
ret = set_current_unlocked (loc);
} else {
ret = set_current_unlocked (loc);
@@ -909,7 +910,7 @@ Locations::clear ()
bool deleted = false;
{
Glib::Threads::Mutex::Lock lm (lock);
Glib::Threads::RWLock::WriterLock lm (_lock);
for (LocationList::iterator i = locations.begin(); i != locations.end(); ) {
@@ -941,7 +942,7 @@ Locations::clear_markers ()
bool deleted = false;
{
Glib::Threads::Mutex::Lock lm (lock);
Glib::Threads::RWLock::WriterLock lm (_lock);
LocationList::iterator tmp;
for (LocationList::iterator i = locations.begin(); i != locations.end(); ) {
@@ -971,7 +972,7 @@ Locations::clear_xrun_markers ()
bool deleted = false;
{
Glib::Threads::Mutex::Lock lm (lock);
Glib::Threads::RWLock::WriterLock lm (_lock);
LocationList::iterator tmp;
for (LocationList::iterator i = locations.begin(); i != locations.end(); ) {
@@ -1001,7 +1002,7 @@ Locations::clear_ranges ()
bool deleted = false;
{
Glib::Threads::Mutex::Lock lm (lock);
Glib::Threads::RWLock::WriterLock lm (_lock);
LocationList::iterator tmp;
for (LocationList::iterator i = locations.begin(); i != locations.end(); ) {
@@ -1046,7 +1047,7 @@ Locations::add (Location *loc, bool make_current)
assert (loc);
{
Glib::Threads::Mutex::Lock lm (lock);
Glib::Threads::RWLock::WriterLock lm (_lock);
locations.push_back (loc);
if (make_current) {
@@ -1083,6 +1084,7 @@ Locations::remove (Location *loc)
{
bool was_removed = false;
bool was_current = false;
bool was_loop = false;
LocationList::iterator i;
if (!loc) {
@@ -1094,36 +1096,40 @@ Locations::remove (Location *loc)
}
{
Glib::Threads::Mutex::Lock lm (lock);
Glib::Threads::RWLock::WriterLock lm (_lock);
for (i = locations.begin(); i != locations.end(); ++i) {
if ((*i) == loc) {
bool was_loop = (*i)->is_auto_loop();
if ((*i)->is_auto_punch()) {
/* needs to happen before deleting:
* disconnect signals, clear events */
_session.set_auto_punch_location (0);
}
delete *i;
locations.erase (i);
was_removed = true;
if (current_location == loc) {
current_location = 0;
was_current = true;
}
if (was_loop) {
if (_session.get_play_loop()) {
_session.request_play_loop (false, false);
}
_session.auto_loop_location_changed (0);
}
break;
if ((*i) != loc) {
continue;
}
was_loop = (*i)->is_auto_loop();
if ((*i)->is_auto_punch()) {
/* needs to happen before deleting:
* disconnect signals, clear events */
lm.release ();
_session.set_auto_punch_location (0);
lm.acquire ();
}
delete *i;
locations.erase (i);
was_removed = true;
if (current_location == loc) {
current_location = 0;
was_current = true;
}
break;
}
}
if (was_removed) {
if (was_loop) {
if (_session.get_play_loop()) {
_session.request_play_loop (false, false);
}
_session.auto_loop_location_changed (0);
}
removed (loc); /* EMIT SIGNAL */
if (was_current) {
@@ -1137,7 +1143,7 @@ Locations::get_state ()
{
XMLNode *node = new XMLNode ("Locations");
LocationList::iterator iter;
Glib::Threads::Mutex::Lock lm (lock);
Glib::Threads::RWLock::ReaderLock lm (_lock);
for (iter = locations.begin(); iter != locations.end(); ++iter) {
node->add_child_nocopy ((*iter)->get_state ());
@@ -1159,16 +1165,17 @@ Locations::set_state (const XMLNode& node, int version)
/* build up a new locations list in here */
LocationList new_locations;
current_location = 0;
Location* session_range_location = 0;
if (version < 3000) {
session_range_location = new Location (_session, 0, 0, _("session"), Location::IsSessionRange, 0);
new_locations.push_back (session_range_location);
}
{
Glib::Threads::Mutex::Lock lm (lock);
Glib::Threads::RWLock::WriterLock lm (_lock);
current_location = 0;
Location* session_range_location = 0;
if (version < 3000) {
session_range_location = new Location (_session, 0, 0, _("session"), Location::IsSessionRange, 0);
new_locations.push_back (session_range_location);
}
XMLNodeConstIterator niter;
for (niter = nlist.begin(); niter != nlist.end(); ++niter) {
@@ -1292,13 +1299,15 @@ struct LocationStartLaterComparison
samplepos_t
Locations::first_mark_before (samplepos_t sample, bool include_special_ranges)
{
Glib::Threads::Mutex::Lock lm (lock);
vector<LocationPair> locs;
{
Glib::Threads::RWLock::ReaderLock lm (_lock);
for (LocationList::iterator i = locations.begin(); i != locations.end(); ++i) {
locs.push_back (make_pair ((*i)->start(), (*i)));
if (!(*i)->is_mark()) {
locs.push_back (make_pair ((*i)->end(), (*i)));
for (LocationList::iterator i = locations.begin(); i != locations.end(); ++i) {
locs.push_back (make_pair ((*i)->start(), (*i)));
if (!(*i)->is_mark()) {
locs.push_back (make_pair ((*i)->end(), (*i)));
}
}
}
@@ -1325,7 +1334,6 @@ Locations::first_mark_before (samplepos_t sample, bool include_special_ranges)
Location*
Locations::mark_at (samplepos_t pos, samplecnt_t slop) const
{
Glib::Threads::Mutex::Lock lm (lock);
Location* closest = 0;
sampleoffset_t mindelta = max_samplepos;
sampleoffset_t delta;
@@ -1334,6 +1342,7 @@ Locations::mark_at (samplepos_t pos, samplecnt_t slop) const
* to iterate across all of them to find the one closest to a give point.
*/
Glib::Threads::RWLock::ReaderLock lm (_lock);
for (LocationList::const_iterator i = locations.begin(); i != locations.end(); ++i) {
if ((*i)->is_mark()) {
@@ -1363,13 +1372,15 @@ Locations::mark_at (samplepos_t pos, samplecnt_t slop) const
samplepos_t
Locations::first_mark_after (samplepos_t sample, bool include_special_ranges)
{
Glib::Threads::Mutex::Lock lm (lock);
vector<LocationPair> locs;
for (LocationList::iterator i = locations.begin(); i != locations.end(); ++i) {
locs.push_back (make_pair ((*i)->start(), (*i)));
if (!(*i)->is_mark()) {
locs.push_back (make_pair ((*i)->end(), (*i)));
{
Glib::Threads::RWLock::ReaderLock lm (_lock);
for (LocationList::iterator i = locations.begin(); i != locations.end(); ++i) {
locs.push_back (make_pair ((*i)->start(), (*i)));
if (!(*i)->is_mark()) {
locs.push_back (make_pair ((*i)->end(), (*i)));
}
}
}
@@ -1408,7 +1419,7 @@ Locations::marks_either_side (samplepos_t const sample, samplepos_t& before, sam
LocationList locs;
{
Glib::Threads::Mutex::Lock lm (lock);
Glib::Threads::RWLock::ReaderLock lm (_lock);
locs = locations;
}
@@ -1468,6 +1479,7 @@ Locations::marks_either_side (samplepos_t const sample, samplepos_t& before, sam
Location*
Locations::session_range_location () const
{
Glib::Threads::RWLock::ReaderLock lm (_lock);
for (LocationList::const_iterator i = locations.begin(); i != locations.end(); ++i) {
if ((*i)->is_session_range()) {
return const_cast<Location*> (*i);
@@ -1479,6 +1491,7 @@ Locations::session_range_location () const
Location*
Locations::auto_loop_location () const
{
Glib::Threads::RWLock::ReaderLock lm (_lock);
for (LocationList::const_iterator i = locations.begin(); i != locations.end(); ++i) {
if ((*i)->is_auto_loop()) {
return const_cast<Location*> (*i);
@@ -1490,6 +1503,7 @@ Locations::auto_loop_location () const
Location*
Locations::auto_punch_location () const
{
Glib::Threads::RWLock::ReaderLock lm (_lock);
for (LocationList::const_iterator i = locations.begin(); i != locations.end(); ++i) {
if ((*i)->is_auto_punch()) {
return const_cast<Location*> (*i);
@@ -1501,19 +1515,25 @@ Locations::auto_punch_location () const
Location*
Locations::clock_origin_location () const
{
Location* sr = 0;
Glib::Threads::RWLock::ReaderLock lm (_lock);
for (LocationList::const_iterator i = locations.begin(); i != locations.end(); ++i) {
if ((*i)->is_clock_origin()) {
return const_cast<Location*> (*i);
}
if ((*i)->is_session_range()) {
sr = const_cast<Location*> (*i);
}
}
return session_range_location ();
/* fall back to session_range_location () */
return sr;
}
uint32_t
Locations::num_range_markers () const
{
uint32_t cnt = 0;
Glib::Threads::Mutex::Lock lm (lock);
Glib::Threads::RWLock::ReaderLock lm (_lock);
for (LocationList::const_iterator i = locations.begin(); i != locations.end(); ++i) {
if ((*i)->is_range_marker()) {
++cnt;
@@ -1525,19 +1545,19 @@ Locations::num_range_markers () const
Location *
Locations::get_location_by_id(PBD::ID id)
{
LocationList::iterator it;
for (it = locations.begin(); it != locations.end(); ++it)
if (id == (*it)->id())
return *it;
Glib::Threads::RWLock::ReaderLock lm (_lock);
for (LocationList::const_iterator i = locations.begin(); i != locations.end(); ++i) {
if (id == (*i)->id()) {
return const_cast<Location*> (*i);
}
}
return 0;
}
void
Locations::find_all_between (samplepos_t start, samplepos_t end, LocationList& ll, Location::Flags flags)
{
Glib::Threads::Mutex::Lock lm (lock);
Glib::Threads::RWLock::ReaderLock lm (_lock);
for (LocationList::const_iterator i = locations.begin(); i != locations.end(); ++i) {
if ((flags == 0 || (*i)->matches (flags)) &&
((*i)->start() >= start && (*i)->end() < end)) {
@@ -1549,10 +1569,10 @@ Locations::find_all_between (samplepos_t start, samplepos_t end, LocationList& l
Location *
Locations::range_starts_at(samplepos_t pos, samplecnt_t slop, bool incl) const
{
Glib::Threads::Mutex::Lock lm(lock);
Location *closest = 0;
sampleoffset_t mindelta = max_samplepos;
Glib::Threads::RWLock::ReaderLock lm (_lock);
for (LocationList::const_iterator i = locations.begin(); i != locations.end(); ++i) {
if (!(*i)->is_range_marker()) {
continue;