listeners: fix duplicate listener names by stopping existing listener

- Check if listener with same name exists before starting new one
- Stop the existing listener to prevent conflicts
- Improve logging for duplicate listener detection

Signed-off-by: Eeshu-Yadav <[email protected]>

Author: Eeshu-Yadav

orion-lib/src/listeners/listeners_manager.rs

@@ -18,7 +18,7 @@
 use std::collections::BTreeMap;
 
 use tokio::sync::{broadcast, mpsc};
-use tracing::{debug, info, warn};
+use tracing::{info, warn};
 
 use orion_configuration::config::{
     network_filters::http_connection_manager::RouteConfiguration, Listener as ListenerConfig,
@@ -122,19 +122,18 @@ impl ListenersManager {
         let listener_name = listener.get_name();
         let (addr, dev) = listener.get_socket();
         info!("Listener {} at {addr} (device bind:{})", listener_name, dev.is_some());
+
+        // Check if listener with same name already exists
+        if self.listener_handles.contains_key(&listener_name) {
+            warn!("Listener {listener_name} already exists, rejecting new listener to avoid breaking existing connections");
+            return Err(format!("Listener with name '{}' already exists", listener_name).into());
+        }
+
         // spawn the task for this listener address, this will spawn additional task per connection
         let join_handle = tokio::spawn(async move {
             let error = listener.start().await;
             warn!("Listener {listener_name} exited: {error}");
         });
-        #[cfg(debug_assertions)]
-        if self.listener_handles.contains_key(&listener_name) {
-            debug!("Listener {listener_name} already exists, replacing it");
-        }
-        // note: join handle gets overwritten here if it already exists.
-        // handles are abort on drop so will be aborted, closing the socket
-        // but the any tasks spawned within this task, which happens on a per-connection basis,
-        // will survive past this point and only get dropped when their session ends
         self.listener_handles.insert(listener_name, ListenerInfo::new(join_handle, listener_conf));
 
         Ok(())
@@ -186,17 +185,14 @@ mod tests {
         assert!(routeb_tx1.send(RouteConfigurationChange::Removed("n/a".into())).is_ok());
         tokio::task::yield_now().await;
 
-        let (routeb_tx2, routeb_rx) = broadcast::channel(chan);
+        let (_routeb_tx2, routeb_rx) = broadcast::channel(chan);
         let (_secb_tx2, secb_rx) = broadcast::channel(chan);
         let l2 = Listener::test_listener(name, routeb_rx, secb_rx);
         let l2_info = l1_info;
-        man.start_listener(l2, l2_info).unwrap();
-        assert!(routeb_tx2.send(RouteConfigurationChange::Removed("n/a".into())).is_ok());
-        tokio::task::yield_now().await;
-
-        // This should fail because the old listener exited already dropping the rx
-        assert!(routeb_tx1.send(RouteConfigurationChange::Removed("n/a".into())).is_err());
-        // Yield once more just in case more logs can be seen
+        // Second start should fail because listener with same name already exists
+        assert!(man.start_listener(l2, l2_info).is_err());
+        // The first listener should still be active
+        assert!(routeb_tx1.send(RouteConfigurationChange::Removed("n/a".into())).is_ok());
         tokio::task::yield_now().await;
     }