From 6a67e67fbc22a178b30ceea40f41f21ff73c920f Mon Sep 17 00:00:00 2001
From: Elinor Berger <elinor@safaradeg.net>
Date: Mon, 25 Sep 2023 14:24:00 +0200
Subject: [PATCH] sources.generic: don't unregister non-registered sources on
 drop

---
 CHANGELOG.md           |  5 ++++
 src/sources/generic.rs | 57 +++++++++++++++++++++++++++++++++++++++---
 2 files changed, 58 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a2150415..35e19cd6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,11 @@
 
 ## Unreleased
 
+#### Bugfixes
+
+- Fix an issue where the `Generic` event source would try to unregister its contents from the event loop
+  after a failed registration.
+
 ## 0.12.1 -- 2023-09-19
 
 #### Bugfixes
diff --git a/src/sources/generic.rs b/src/sources/generic.rs
index 45ed95ee..63d94b2e 100644
--- a/src/sources/generic.rs
+++ b/src/sources/generic.rs
@@ -261,10 +261,7 @@ where
     fn register(&mut self, poll: &mut Poll, token_factory: &mut TokenFactory) -> crate::Result<()> {
         let token = token_factory.token();
 
-        // Make sure we can use the poller to deregister if need be.
-        self.poller = Some(poll.poller().clone());
-
-        // SAFETY: We've now ensured that we have a poller to deregister with.
+        // SAFETY: We ensure that we have a poller to deregister with (see below).
         unsafe {
             poll.register(
                 &self.file.as_ref().unwrap().0,
@@ -274,7 +271,13 @@ where
             )?;
         }
 
+        // Make sure we can use the poller to deregister if need be.
+        // But only if registration actually succeeded
+        // So that we don't try to unregister the FD on drop if it wasn't registered
+        // in the first place (for example if registration failed because of a duplicate insertion)
+        self.poller = Some(poll.poller().clone());
         self.token = Some(token);
+
         Ok(())
     }
 
@@ -422,4 +425,50 @@ mod tests {
         // the has now been properly dispatched
         assert!(dispached);
     }
+
+    // Duplicate insertion does not fail on all platforms, but does on Linux
+    #[cfg(target_os = "linux")]
+    #[test]
+    fn duplicate_insert() {
+        use std::os::unix::{
+            io::{AsFd, BorrowedFd},
+            net::UnixStream,
+        };
+        let event_loop = crate::EventLoop::<()>::try_new().unwrap();
+
+        let handle = event_loop.handle();
+
+        let (_, rx) = UnixStream::pair().unwrap();
+
+        // Rc only implements AsFd since 1.69...
+        struct RcFd<T> {
+            rc: std::rc::Rc<T>,
+        }
+
+        impl<T: AsFd> AsFd for RcFd<T> {
+            fn as_fd(&self) -> BorrowedFd<'_> {
+                self.rc.as_fd()
+            }
+        }
+
+        let rx = std::rc::Rc::new(rx);
+
+        let token = handle
+            .insert_source(
+                Generic::new(RcFd { rc: rx.clone() }, Interest::READ, Mode::Level),
+                |_, _, _| Ok(PostAction::Continue),
+            )
+            .unwrap();
+
+        // inserting the same FD a second time should fail
+        let ret = handle.insert_source(
+            Generic::new(RcFd { rc: rx.clone() }, Interest::READ, Mode::Level),
+            |_, _, _| Ok(PostAction::Continue),
+        );
+        assert!(ret.is_err());
+        std::mem::drop(ret);
+
+        // but the original token is still registered
+        handle.update(&token).unwrap();
+    }
 }