[llvm] Support: don't block signals around close if it can be avoided (PR #73009)

Mateusz Guzik via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 21 08:41:00 PST 2023


https://github.com/mjguzik created https://github.com/llvm/llvm-project/pull/73009

Signal blocking originally showed up in 51c2afc4b65b2782 ("Support: Don't call close again if we get EINTR"), but it was overzealous -- there are systems where the error is known to be fine.

This commit elides signal blocking for said systems (the list is incomplete though).

Note close() can still fail for other reasons (like ENOSPC), in which case an error will be returned while the fd slot is cleared up.

>From 2d643ae32a8f49dd941cfc7afbfa916553a36a47 Mon Sep 17 00:00:00 2001
From: Mateusz Guzik <mjguzik at gmail.com>
Date: Tue, 21 Nov 2023 16:36:54 +0000
Subject: [PATCH] Support: don't block signals around close if it can be
 avoided

Signal blocking originally showed up in 51c2afc4b65b2782 ("Support:
Don't call close again if we get EINTR"), but it was overzealous --
there are systems where the error is known to be fine.

This commit elides signal blocking for said systems (the list is
incomplete though).

Note close() can still fail for other reasons (like ENOSPC), in which
case an error will be returned while the fd slot is cleared up.
---
 llvm/lib/Support/Unix/Process.inc | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/llvm/lib/Support/Unix/Process.inc b/llvm/lib/Support/Unix/Process.inc
index d3d9fb7d71878e1..bc638439754c71e 100644
--- a/llvm/lib/Support/Unix/Process.inc
+++ b/llvm/lib/Support/Unix/Process.inc
@@ -234,6 +234,25 @@ std::error_code Process::FixupStandardFileDescriptors() {
   return std::error_code();
 }
 
+// Close a file descriptor while being mindful of EINTR.
+//
+// On Unix systems closing a file descriptor usually starts with removing it
+// from the fd table (or an equivalent structure). This means any error
+// generated past that point will still result in the entry being cleared.
+//
+// Make sure to not bubble up EINTR as there is nothing to do in that case.
+// XXX what about other errors?
+#if defined(__linux__) || defined(__DragonFly__) || defined(__FreeBSD__) || \
+    defined(__NetBSD__) || defined(__OpenBSD__)
+std::error_code Process::SafelyCloseFileDescriptor(int FD) {
+  int EC = 0;
+  if (::close(FD) < 0) {
+    if (errno != EINTR)
+      EC = errno;
+  }
+  return std::error_code(EC, std::generic_category());
+}
+#else
 std::error_code Process::SafelyCloseFileDescriptor(int FD) {
   // Create a signal set filled with *all* signals.
   sigset_t FullSet, SavedSet;
@@ -268,6 +287,7 @@ std::error_code Process::SafelyCloseFileDescriptor(int FD) {
     return std::error_code(ErrnoFromClose, std::generic_category());
   return std::error_code(EC, std::generic_category());
 }
+#endif
 
 bool Process::StandardInIsUserInput() {
   return FileDescriptorIsDisplayed(STDIN_FILENO);



More information about the llvm-commits mailing list