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 6faab0e63b829e9b65bef2672f6934295504323e..6b288d70d75af881f5fe80d6e7b441fe16a80cdf 100644 (file)
--- 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 533c9219b33fd18e5ce352b4360da8b92e79b61f..efb498880c6dd07ef4b235126e9c9db42f79c7a8 100644 (file)
--- 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);