Browse Source

refinery: delete un-necessary and excessively long lock on greylist

Previously we were locking the greylist to prevent the index from changing
while performing a handshake. This isn't necessary. It is acceptable
for the greylist index to change while the handshake is ongoing.

The thing we don't want to happen is that the addr we are operating on
is somehow removed from the list- this should never happen since it has
been marked as "Refine" in the HostRegistry. Therefore, calling unwrap()
on get_index_at_addr() should pass 100% of the time (and if it doesn't,
that indicates a logic failure elsewhere in the code).
draoi 2 years ago
parent
commit
43bad915a5
2 changed files with 14 additions and 13 deletions
  1. 1 1
      src/net/hosts.rs
  2. 13 12
      src/net/session/refine_session.rs

+ 1 - 1
src/net/hosts.rs

@@ -672,7 +672,7 @@ impl HostContainer {
     }
     }
 
 
     /// Get the index for a given addr on a hostlist.
     /// Get the index for a given addr on a hostlist.
-    async fn get_index_at_addr(&self, color: usize, addr: Url) -> Option<usize> {
+    pub async fn get_index_at_addr(&self, color: usize, addr: Url) -> Option<usize> {
         self.hostlists[color].read().await.iter().position(|a| a.0 == addr)
         self.hostlists[color].read().await.iter().position(|a| a.0 == addr)
     }
     }
 
 

+ 13 - 12
src/net/session/refine_session.rs

@@ -260,7 +260,7 @@ impl GreylistRefinery {
                 .fetch_random_with_schemes(HostColor::Grey, &settings.allowed_transports)
                 .fetch_random_with_schemes(HostColor::Grey, &settings.allowed_transports)
                 .await
                 .await
             {
             {
-                Some((entry, position)) => {
+                Some((entry, _)) => {
                     let url = &entry.0;
                     let url = &entry.0;
 
 
                     if let Err(e) = hosts.try_register(url.clone(), HostState::Refine).await {
                     if let Err(e) = hosts.try_register(url.clone(), HostState::Refine).await {
@@ -269,13 +269,19 @@ impl GreylistRefinery {
                         continue
                         continue
                     }
                     }
 
 
-                    // Freeze the greylist in this state. Necessary since the greylist
-                    // can be modified by `hosts::move_host()` or `hosts::store()`.
-                    let mut greylist =
-                        hosts.container.hostlists[HostColor::Grey as usize].write().await;
-
                     if !self.session().handshake_node(url.clone(), self.p2p().clone()).await {
                     if !self.session().handshake_node(url.clone(), self.p2p().clone()).await {
-                        greylist.remove(position);
+                        {
+                            let mut greylist =
+                                hosts.container.hostlists[HostColor::Grey as usize].write().await;
+
+                            let position = hosts
+                                .container
+                                .get_index_at_addr(HostColor::Grey as usize, url.clone())
+                                .await
+                                .unwrap();
+
+                            greylist.remove(position);
+                        }
 
 
                         debug!(
                         debug!(
                             target: "net::refinery",
                             target: "net::refinery",
@@ -287,13 +293,8 @@ impl GreylistRefinery {
                         // modification is now complete.
                         // modification is now complete.
                         hosts.unregister(url).await;
                         hosts.unregister(url).await;
 
 
-                        drop(greylist);
-
                         continue
                         continue
                     }
                     }
-
-                    drop(greylist);
-
                     debug!(
                     debug!(
                         target: "net::refinery",
                         target: "net::refinery",
                         "Peer {} handshake successful. Adding to whitelist", url,
                         "Peer {} handshake successful. Adding to whitelist", url,