Move FdGuard to IOUtils and improve it (#2607)

Improvements:
- Make it moveable.
- Make it unabuseable: don't throwing exceptions in destructor if exception is already being thrown (= undefined behavior). Just warn.

Partially addresses #2596.

Author: FooBarWidget

src/agent/Core/ApplicationPool/Implementation.cpp

@@ -193,7 +193,7 @@ void processAndLogNewSpawnException(SpawningKit::SpawnException &e, const Option
 				getSystemTempDir());
 			fd = mkstemp(filename);
 		#endif
-		FdGuard guard(fd, NULL, 0, true);
+		FdGuard guard(fd, nullptr, 0);
 		if (fd == -1) {
 			int e = errno;
 			throw SystemException("Cannot generate a temporary filename",

src/agent/Core/CoreMain.cpp

@@ -88,6 +88,7 @@
 #include <Utils.h>
 #include <Utils/Timer.h>
 #include <IOTools/MessageIO.h>
+#include <IOTools/IOUtils.h>
 #include <Core/OptionParser.h>
 #include <Core/Controller.h>
 #include <Core/ApiServer.h>

src/agent/Core/SpawningKit/Handshake/Perform.h

@@ -50,6 +50,7 @@
 #include <FileDescriptor.h>
 #include <FileTools/FileManip.h>
 #include <FileTools/PathManip.h>
+#include <IOTools/IOUtils.h>
 #include <Utils.h>
 #include <Utils/ScopeGuard.h>
 #include <SystemTools/SystemTime.h>

src/cxx_supportlib/IOTools/IOUtils.cpp

@@ -29,6 +29,7 @@
 	// https://bugzilla.redhat.com/show_bug.cgi?id=165427
 	// Also needed for SO_PEERCRED.
 	#define _GNU_SOURCE
+#include <exception>
 #endif
 
 #include <oxt/system_calls.hpp>
@@ -215,7 +216,7 @@ createUnixServer(const StaticString &filename, unsigned int backlogSize, bool au
 		throw SystemException("Cannot create a Unix socket file descriptor", e);
 	}
 
-	FdGuard guard(fd, file, line, true);
+	FdGuard guard(fd, file, line);
 	addr.sun_family = AF_LOCAL;
 	strncpy(addr.sun_path, filename.c_str(), filename.size());
 	addr.sun_path[filename.size()] = '\0';
@@ -306,7 +307,7 @@ createTcpServer(const char *address, unsigned short port, unsigned int backlogSi
 	}
 	// Ignore SO_REUSEADDR error, it's not fatal.
 
-	FdGuard guard(fd, file, line, true);
+	FdGuard guard(fd, file, line);
 	if (family == AF_INET) {
 		ret = syscalls::bind(fd, (const struct sockaddr *) &addr.v4, sizeof(struct sockaddr_in));
 	} else {
@@ -369,7 +370,7 @@ connectToUnixServer(const StaticString &filename, const char *file,
 		throw SystemException("Cannot create a Unix socket file descriptor", e);
 	}
 
-	FdGuard guard(fd, file, line, true);
+	FdGuard guard(fd, file, line);
 	int ret;
 	struct sockaddr_un addr;
 
@@ -656,6 +657,72 @@ NConnect_State::asNTCP_State() {
 }
 
 
+/****** Scope guards ******/
+
+FdGuard::FdGuard(FdGuard &&other)
+	: mFd(other.mFd)
+{
+	other.mFd = -1;
+}
+
+FdGuard::FdGuard(int fd, const char *sourceFile, unsigned int sourceLine)
+	: mFd(fd)
+{
+	if (mFd != -1 && sourceFile != nullptr) {
+		P_LOG_FILE_DESCRIPTOR_OPEN3(fd, sourceFile, sourceLine);
+	}
+}
+
+FdGuard::~FdGuard() noexcept(false) {
+	if (mFd != -1) {
+		try {
+			safelyClose(mFd);
+		} catch (const std::exception &e) {
+			bool uncaughtException =
+				#if __cplusplus >= 201703L
+					std::uncaught_exceptions() > 0;
+				#else
+					std::uncaught_exception();
+				#endif
+			if (uncaughtException) {
+				P_WARN("Error closing file descriptor " << mFd << ": " << e.what());
+				return;
+			} else {
+				throw e;
+			}
+		}
+		P_LOG_FILE_DESCRIPTOR_CLOSE(mFd);
+	}
+}
+
+FdGuard &
+FdGuard::operator=(FdGuard &&other) {
+	if (this != &other) {
+		if (mFd != -1) {
+			safelyClose(mFd);
+			P_LOG_FILE_DESCRIPTOR_CLOSE(mFd);
+		}
+		mFd = other.mFd;
+		other.mFd = -1;
+	}
+	return *this;
+}
+
+void
+FdGuard::clear() noexcept {
+	mFd = -1;
+}
+
+void
+FdGuard::runNow() noexcept(false) {
+	if (mFd != -1) {
+		safelyClose(mFd);
+		P_LOG_FILE_DESCRIPTOR_CLOSE(mFd);
+		mFd = -1;
+	}
+}
+
+
 /****** Other ******/
 
 bool

src/cxx_supportlib/IOTools/IOUtils.h

@@ -358,6 +358,36 @@ struct NConnect_State {
 };
 
 
+/****** Scope guards ******/
+
+/** RAII construct for ensuring that a file descriptor gets closed at scope exit. */
+class FdGuard {
+private:
+	int mFd = -1;
+
+public:
+	FdGuard() { }
+	FdGuard(const FdGuard &other) = delete;
+	FdGuard(FdGuard &&other);
+	FdGuard(int fd, const char *sourceFile, unsigned int sourceLine);
+	/** @throws SystemException File descriptor close error. If exception is already being thrown, then only warns. */
+	~FdGuard() noexcept(false);
+
+	FdGuard &operator=(const FdGuard &other) = delete;
+	FdGuard &operator=(FdGuard &&other);
+
+	/** Don't close file descriptor at object destruction. */
+	void clear() noexcept;
+
+	/**
+	 * Close file descriptor now. Idempotent.
+	 *
+	 * @throws SystemException
+	 */
+	void runNow() noexcept(false);
+};
+
+
 /****** Other ******/
 
 typedef ssize_t (*WritevFunction)(int fildes, const struct iovec *iov, int iovcnt);

src/cxx_supportlib/Utils/ScopeGuard.h

@@ -117,38 +117,6 @@ class StdioGuard: public noncopyable {
 	}
 };
 
-class FdGuard: public noncopyable {
-private:
-	int fd;
-	bool ignoreErrors;
-
-public:
-	FdGuard(int _fd, const char *file, unsigned int line, bool _ignoreErrors = false)
-		: fd(_fd),
-		  ignoreErrors(_ignoreErrors)
-	{
-		if (_fd != -1 && file != NULL) {
-			P_LOG_FILE_DESCRIPTOR_OPEN3(_fd, file, line);
-		}
-	}
-
-	~FdGuard() {
-		runNow();
-	}
-
-	void clear() {
-		fd = -1;
-	}
-
-	void runNow() {
-		if (fd != -1) {
-			safelyClose(fd, ignoreErrors);
-			P_LOG_FILE_DESCRIPTOR_CLOSE(fd);
-			fd = -1;
-		}
-	}
-};
-
 
 } // namespace Passenger