From: Carl Hetherington <cth@carlh.net>
Date: Thu, 24 Feb 2022 18:47:19 +0000 (+0100)
Subject: Fix some dubious thread/locking behaviour.
X-Git-Tag: v2.16.4~10
X-Git-Url: https://git.carlh.net/gitweb/?p=dcpomatic.git;a=commitdiff_plain;h=5ec4efca61906608e2e7a590b3d1c3f2b7078e81

Fix some dubious thread/locking behaviour.

Previously we had server_found(), which took the lock and found
a server, which it returned as an iterator into the list.
However, it then released the lock, which I think left the
iterator unprotected.

This wasn't done in response to any particular bug, I just
noticed it on the way past.
---

diff --git a/src/lib/encode_server_finder.cc b/src/lib/encode_server_finder.cc
index 6faab0e63..6b288d70d 100644
--- a/src/lib/encode_server_finder.cc
+++ b/src/lib/encode_server_finder.cc
@@ -231,36 +231,29 @@ EncodeServerFinder::handle_accept (boost::system::error_code ec, shared_ptr<Sock
 	xml->read_string (s);
 
 	auto const ip = socket->socket().remote_endpoint().address().to_string();
-	auto found = server_found (ip);
-	if (found) {
-		(*found)->set_seen ();
-	} else {
-		EncodeServerDescription sd (ip, xml->number_child<int>("Threads"), xml->optional_number_child<int>("Version").get_value_or(0));
-		{
-			boost::mutex::scoped_lock lm (_servers_mutex);
-			_servers.push_back (sd);
-		}
-		emit (boost::bind(boost::ref (ServersListChanged)));
 	}
+	bool changed = false;
+	{
+		boost::mutex::scoped_lock lm (_servers_mutex);
+		auto i = _servers.begin();
+		while (i != _servers.end() && i->host_name() != ip) {
+			++i;
+		}
 
-	start_accept ();
-}
-
-
-optional<list<EncodeServerDescription>::iterator>
-EncodeServerFinder::server_found (string ip)
-{
-	boost::mutex::scoped_lock lm (_servers_mutex);
-	auto i = _servers.begin();
-	while (i != _servers.end() && i->host_name() != ip) {
-		++i;
+		if (i != _servers.end()) {
+			i->set_seen();
+		} else {
+			EncodeServerDescription sd (ip, xml->number_child<int>("Threads"), xml->optional_number_child<int>("Version").get_value_or(0));
+			_servers.push_back (sd);
+			changed = true;
+		}
 	}
 
-	if (i != _servers.end()) {
-		return i;
+	if (changed) {
+		emit (boost::bind(boost::ref (ServersListChanged)));
 	}
 
-	return {};
+	start_accept ();
 }
 
 
diff --git a/src/lib/encode_server_finder.h b/src/lib/encode_server_finder.h
index 533c9219b..efb498880 100644
--- a/src/lib/encode_server_finder.h
+++ b/src/lib/encode_server_finder.h
@@ -66,7 +66,6 @@ private:
 	void search_thread ();
 	void listen_thread ();
 
-	boost::optional<std::list<EncodeServerDescription>::iterator> server_found (std::string);
 	void start_accept ();
 	void handle_accept (boost::system::error_code ec, std::shared_ptr<Socket> socket);