Use a flag for stunHosts for consistency and remove stop_signal

src/icmp.rs

@@ -11,15 +11,12 @@
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 // See the License for the specific language governing permissions and
 // limitations under the License.
+use std::net::{IpAddr, Ipv4Addr, Ipv6Addr};
 use std::ops::Sub;
 use std::{
     collections::HashMap,
     time::{Duration, Instant},
 };
-use std::{
-    net::{IpAddr, Ipv4Addr, Ipv6Addr},
-    sync::{Arc, RwLock},
-};
 
 use crate::util;
 
@@ -68,7 +65,6 @@ struct State<AddrType> {
     time_tracker: HashMap<u16, Instant>,
     latency_guage: GaugeVec,
     ping_counter: CounterVec,
-    stop_signal: Arc<RwLock<bool>>,
 }
 
 struct PingerImpl<Sock: IcmpSocket> {
@@ -311,12 +307,6 @@ where
                     state.time_tracker.insert(identifier, send_time);
                 }
             }
-            {
-                // Scope the lock really tightly
-                if *state.stop_signal.read().unwrap() {
-                    return Ok(());
-                }
-            }
         }
         Ok(())
     }
@@ -364,12 +354,6 @@ where
                             .inc();
                     }
                 }
-                {
-                    // Scope the lock really tightly.
-                    if *handler.get_mut_state().stop_signal.read().unwrap() {
-                        return;
-                    }
-                }
             }
         }
         let mut state = handler.get_mut_state();
@@ -379,7 +363,6 @@ where
 
 pub fn start_echo_loop(
     domain_names: &Vec<&str>,
-    stop_signal: Arc<RwLock<bool>>,
     ping_latency_guage: GaugeVec,
     ping_counter: CounterVec,
 ) {
@@ -419,7 +402,6 @@ pub fn start_echo_loop(
         time_tracker: HashMap::new(),
         latency_guage: ping_latency_guage.clone(),
         ping_counter: ping_counter.clone(),
-        stop_signal: stop_signal.clone(),
     };
     let mut v6_destinations = HashMap::new();
     let mut v6_id_counter = 42;
@@ -438,7 +420,6 @@ pub fn start_echo_loop(
         time_tracker: HashMap::new(),
         latency_guage: ping_latency_guage,
         ping_counter,
-        stop_signal: stop_signal.clone(),
     };
     let mut v6_pinger = PingerImpl {
         sock: IcmpSocket6::new().expect("Failed to open Icmpv6 Socket"),
@@ -453,12 +434,6 @@ pub fn start_echo_loop(
             .expect("Error sending packets on socket");
         v4_pinger.recv_all(&mut v4_state);
         v6_pinger.recv_all(&mut v6_state);
-        {
-            // Scope the lock really tightly
-            if *stop_signal.read().unwrap() {
-                return;
-            }
-        }
         std::thread::sleep(Duration::from_secs(PINGDELAY.flag))
     }
 }

src/main.rs

@@ -14,7 +14,6 @@
 
 use std::convert::Into;
 use std::sync::Arc;
-use std::sync::RwLock;
 
 use gflags;
 use log::{debug, error, info};
@@ -49,22 +48,14 @@ gflags::define! {
     --pingHosts = "google.com"
 }
 
+gflags::define! {
+    /// Comma separated list of hosts to ping
+    --stunHosts = "stun.l.google.com:19302,stun.ekiga.net:3478,stun.xten.com:3478,stun.ideasip.com:3478,stun.rixtelecom.se:3478,stun.schlund.de:3478,stun.softjoys.com:3478,stun.stunprotocol.org:3478,stun.voiparound.com:3478,stun.voipbuster.com:3478,stun.voipstunt.com:3478,stun1.noc.ams-ix.net:3478"
+}
+
 fn main() -> anyhow::Result<()> {
-    let default_stun_servers: Vec<&'static str> = vec![
+    gflags::parse();
-        "stun.l.google.com:19302",
+    let stun_servers: Vec<&str> = dbg!(STUNHOSTS.flag).split(",").collect();
-        "stun.ekiga.net:3478",
-        "stun.xten.com:3478",
-        "stun.ideasip.com:3478",
-        "stun.rixtelecom.se:3478",
-        "stun.schlund.de:3478",
-        "stun.softjoys.com:3478",
-        "stun.stunprotocol.org:3478",
-        "stun.voiparound.com:3478",
-        "stun.voipbuster.com:3478",
-        "stun.voipstunt.com:3478",
-        "stun1.noc.ams-ix.net:3478",
-    ];
-    let mut stun_servers = gflags::parse();
 
     if HELP.flag {
         println!("durnitisp <options> <list of hostname:port>");
@@ -87,13 +78,9 @@ fn main() -> anyhow::Result<()> {
         .timestamp(stderrlog::Timestamp::Millisecond)
         .init()?;
 
-    if stun_servers.is_empty() {
+    let ping_hosts: Vec<&str> = dbg!(PINGHOSTS.flag).split(",").collect();
-        stun_servers = default_stun_servers;
-    }
-    // FIXME(jwall): allow them to override ping hosts
-    let ping_hosts: Vec<&str> = PINGHOSTS.flag.split(",").collect();
-    let stop_signal = Arc::new(RwLock::new(false));
 
+    dbg!(&ping_hosts);
     // Create a Registry and register metrics.
     let r = Registry::new();
     let stun_counter_vec = CounterVec::new(
@@ -142,15 +129,12 @@ fn main() -> anyhow::Result<()> {
     // First we start the render thread.
     {
         // Introduce a new scope for our Arc to clone before moving it into the thread.
-        let stop_signal = stop_signal.clone();
         // thread::Handle starts the thread immediately so the render thread will usually start first.
         let render_thread = thread::Handle::new(move || {
             debug!("attempting to start server on {}", LISTENHOST.flag);
             let server = match tiny_http::Server::http(LISTENHOST.flag) {
                 Ok(server) => server,
                 Err(err) => {
-                    let mut signal = stop_signal.write().unwrap();
-                    *signal = true;
                     error!("Error starting render thread {}", err);
                     error!("Shutting down all threads...");
                     std::process::exit(1);
@@ -181,16 +165,10 @@ fn main() -> anyhow::Result<()> {
         parent.adopt(Box::new(render_thread));
     }
     {
-        let stop_signal = stop_signal.clone();
         let ping_latency_vec = ping_latency_vec.clone();
         let ping_counter_vec = ping_counter_vec.clone();
         let ping_thread = thread::Pending::new(move || {
-            icmp::start_echo_loop(
+            icmp::start_echo_loop(&ping_hosts, ping_latency_vec, ping_counter_vec);
-                &ping_hosts,
-                stop_signal.clone(),
-                ping_latency_vec,
-                ping_counter_vec,
-            );
         });
         parent.schedule(Box::new(ping_thread));
     }
@@ -202,11 +180,9 @@ fn main() -> anyhow::Result<()> {
         let stun_success_vec_copy = stun_success_vec.clone();
         if let Some(s) = s.clone() {
             let domain_name = *stun_servers_copy.get(i).unwrap();
-            let stop_signal = stop_signal.clone();
             let connect_thread = thread::Pending::new(move || {
                 stun::start_listen_thread(
                     domain_name,
-                    stop_signal,
                     s.into(),
                     stun_counter_vec_copy,
                     stun_latency_vec_copy,

src/stun.rs

@@ -18,8 +18,6 @@ use prometheus::{CounterVec, IntGaugeVec};
 use std::convert::From;
 use std::io;
 use std::net::{SocketAddr, UdpSocket};
-use std::sync::Arc;
-use std::sync::RwLock;
 use std::time::SystemTime;
 
 gflags::define! {
@@ -74,7 +72,6 @@ fn attempt_stun_connect(addr: SocketAddr) -> Result<SystemTime, ConnectError> {
 
 pub fn start_listen_thread(
     domain_name: &str,
-    stop_signal: Arc<RwLock<bool>>,
     s: SocketAddr,
     stun_counter_vec_copy: CounterVec,
     stun_latency_vec_copy: IntGaugeVec,
@@ -82,13 +79,6 @@ pub fn start_listen_thread(
 ) {
     debug!("started thread for {}", domain_name);
     loop {
-        {
-            // Limit the scope of this lock
-            if *stop_signal.read().unwrap() {
-                info!("Stopping stun thread for {}", domain_name);
-                return;
-            }
-        }
         let now = SystemTime::now();
         info!("Attempting to connect to {}", domain_name);
         match attempt_stun_connect(s) {