From c9cb83a5f6a4ec315b187afcf899d5758647dd0f Mon Sep 17 00:00:00 2001
From: Alan Somers <asomers@gmail.com>
Date: Mon, 1 Feb 2021 20:00:48 -0700
Subject: [PATCH] Don't implement Clone on Dir, SignalFd, and PtyMaster

Since they close their file descriptors on Drop, it's almost impossible
to use Clone without creating a double-close situation.

Also, check for EBADF in SignalFd::drop and Dir::drop.
---
 CHANGELOG.md        | 3 +++
 src/dir.rs          | 7 +++++--
 src/pty.rs          | 2 +-
 src/sys/signalfd.rs | 7 +++++--
 4 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 54295f5d81..68fb0d6829 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -23,6 +23,9 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 
 ### Removed
 
+- `Dir`, `SignalFd`, and `PtyMaster` are no longer `Clone`.
+  (#[1382](https://github.com/nix-rust/nix/pull/1382))
+
 - Removed `SockLevel`, which hasn't been used for a few years
   (#[1362](https://github.com/nix-rust/nix/pull/1362))
 
diff --git a/src/dir.rs b/src/dir.rs
index 1d48f18c56..1898950f71 100644
--- a/src/dir.rs
+++ b/src/dir.rs
@@ -25,7 +25,7 @@ use libc::{dirent, readdir_r};
 ///    * returns entries for `.` (current directory) and `..` (parent directory).
 ///    * returns entries' names as a `CStr` (no allocation or conversion beyond whatever libc
 ///      does).
-#[derive(Clone, Debug, Eq, Hash, PartialEq)]
+#[derive(Debug, Eq, Hash, PartialEq)]
 pub struct Dir(
     ptr::NonNull<libc::DIR>
 );
@@ -85,7 +85,10 @@ impl AsRawFd for Dir {
 
 impl Drop for Dir {
     fn drop(&mut self) {
-        unsafe { libc::closedir(self.0.as_ptr()) };
+        let e = Errno::result(unsafe { libc::closedir(self.0.as_ptr()) });
+        if !std::thread::panicking() && e == Err(Error::Sys(Errno::EBADF)) {
+            panic!("Closing an invalid file descriptor!");
+        };
     }
 }
 
diff --git a/src/pty.rs b/src/pty.rs
index 3a6a9232f4..d67518f474 100644
--- a/src/pty.rs
+++ b/src/pty.rs
@@ -43,7 +43,7 @@ pub struct ForkptyResult {
 /// While this datatype is a thin wrapper around `RawFd`, it enforces that the available PTY
 /// functions are given the correct file descriptor. Additionally this type implements `Drop`,
 /// so that when it's consumed or goes out of scope, it's automatically cleaned-up.
-#[derive(Clone, Debug, Eq, Hash, PartialEq)]
+#[derive(Debug, Eq, Hash, PartialEq)]
 pub struct PtyMaster(RawFd);
 
 impl AsRawFd for PtyMaster {
diff --git a/src/sys/signalfd.rs b/src/sys/signalfd.rs
index e3ded1f7ae..c43b45046f 100644
--- a/src/sys/signalfd.rs
+++ b/src/sys/signalfd.rs
@@ -79,7 +79,7 @@ pub fn signalfd(fd: RawFd, mask: &SigSet, flags: SfdFlags) -> Result<RawFd> {
 ///     Err(err) => (), // some error happend
 /// }
 /// ```
-#[derive(Clone, Debug, Eq, Hash, PartialEq)]
+#[derive(Debug, Eq, Hash, PartialEq)]
 pub struct SignalFd(RawFd);
 
 impl SignalFd {
@@ -116,7 +116,10 @@ impl SignalFd {
 
 impl Drop for SignalFd {
     fn drop(&mut self) {
-        let _ = unistd::close(self.0);
+        let e = unistd::close(self.0);
+        if !std::thread::panicking() && e == Err(Error::Sys(Errno::EBADF)) {
+            panic!("Closing an invalid file descriptor!");
+        };
     }
 }