From 7d3f6c7f8d05ba81e3b9e2ff8ffc1a00e95cef0a Mon Sep 17 00:00:00 2001 From: Casey Williams Date: Fri, 15 Feb 2019 13:50:19 -0800 Subject: [PATCH 1/3] (LTH-160) Use closefrom(2) when available (#291) (LTH-160) Use closefrom(2) when available --- execution/CMakeLists.txt | 5 +++++ execution/src/posix/execution.cc | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/execution/CMakeLists.txt b/execution/CMakeLists.txt index a25a0d45..bd7c46d8 100644 --- a/execution/CMakeLists.txt +++ b/execution/CMakeLists.txt @@ -63,3 +63,8 @@ if (BUILDING_LEATHERMAN) "${CMAKE_CURRENT_LIST_DIR}/tests/fixtures.hpp" ) endif() + +check_library_exists(c closefrom /lib CLOSEFROM_IN_LIBC) +if ("${CLOSEFROM_IN_LIBC}" STREQUAL "1") + add_compile_definitions(HAS_CLOSEFROM) +endif() diff --git a/execution/src/posix/execution.cc b/execution/src/posix/execution.cc index 95254f8d..6dc0eb34 100644 --- a/execution/src/posix/execution.cc +++ b/execution/src/posix/execution.cc @@ -314,9 +314,13 @@ namespace leatherman { namespace execution { } // Close all open file descriptors above stderr +#ifdef HAS_CLOSEFROM + closefrom(STDERR_FILENO + 1); +#else for (uint64_t i = (STDERR_FILENO + 1); i < max_fd; ++i) { close(i); } +#endif // Execute the given program; this should not return if successful execve(program, const_cast(argv), const_cast(envp)); From a0d64254cc3a4638e443aa5ac27b61ec0e3d58a8 Mon Sep 17 00:00:00 2001 From: Gabriel Nagy Date: Fri, 23 Aug 2019 12:02:26 +0300 Subject: [PATCH 2/3] (LTH-162) Use /proc to close file descriptors On systems where /proc is available, use /proc/self/fd to close only open file descriptors. Without this change, facter runs would take a long time on systems with a high open file descriptor limit. `$ ./bin/facter --no-cache` Before: file descriptors: 1024000 - 9s After: file descriptors: 1024000 - 0,2s --- execution/CMakeLists.txt | 7 ++++++- execution/src/posix/execution.cc | 21 +++++++++++++++++++-- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/execution/CMakeLists.txt b/execution/CMakeLists.txt index bd7c46d8..e45d8e4c 100644 --- a/execution/CMakeLists.txt +++ b/execution/CMakeLists.txt @@ -66,5 +66,10 @@ endif() check_library_exists(c closefrom /lib CLOSEFROM_IN_LIBC) if ("${CLOSEFROM_IN_LIBC}" STREQUAL "1") - add_compile_definitions(HAS_CLOSEFROM) + add_definitions(-DHAS_CLOSEFROM) +endif() + +if (IS_DIRECTORY "/proc/self/fd") + add_definitions(-DHAS_PROC_PID) + message("-- Found /proc") endif() diff --git a/execution/src/posix/execution.cc b/execution/src/posix/execution.cc index 6dc0eb34..5beffca5 100644 --- a/execution/src/posix/execution.cc +++ b/execution/src/posix/execution.cc @@ -314,13 +314,30 @@ namespace leatherman { namespace execution { } // Close all open file descriptors above stderr -#ifdef HAS_CLOSEFROM +#if defined(HAS_CLOSEFROM) closefrom(STDERR_FILENO + 1); #else + #if defined(HAS_PROC_PID) + uint64_t fd; + const char* fdpath = "/proc/self/fd"; + std::list fd_list; + + if (is_directory(fdpath)) { + for (const directory_entry& dent : directory_iterator(fdpath)) { + fd = atol(dent.path().filename().c_str()); + if (fd >= (STDERR_FILENO + 1)) { + fd_list.push_back(fd); + } + } + for (auto fd : fd_list) { + close(fd); + } + } else //NOLINT + #endif // HAS_PROC_PID for (uint64_t i = (STDERR_FILENO + 1); i < max_fd; ++i) { close(i); } -#endif +#endif // HAS_CLOSEFROM // Execute the given program; this should not return if successful execve(program, const_cast(argv), const_cast(envp)); From a22dc6ec797b3ebbcb4c1bc8c83ebb54e50d1fe1 Mon Sep 17 00:00:00 2001 From: Gabriel Nagy Date: Fri, 23 Aug 2019 16:45:13 +0300 Subject: [PATCH 3/3] (LTH-162) Don't check for /proc when running CMake Boost should take care of checking that the `/proc/self/fd` directory exists, so we shouldn't need to check this in CMakeLists also. --- execution/CMakeLists.txt | 5 ----- execution/src/posix/execution.cc | 11 +++++------ 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/execution/CMakeLists.txt b/execution/CMakeLists.txt index e45d8e4c..edf51af7 100644 --- a/execution/CMakeLists.txt +++ b/execution/CMakeLists.txt @@ -68,8 +68,3 @@ check_library_exists(c closefrom /lib CLOSEFROM_IN_LIBC) if ("${CLOSEFROM_IN_LIBC}" STREQUAL "1") add_definitions(-DHAS_CLOSEFROM) endif() - -if (IS_DIRECTORY "/proc/self/fd") - add_definitions(-DHAS_PROC_PID) - message("-- Found /proc") -endif() diff --git a/execution/src/posix/execution.cc b/execution/src/posix/execution.cc index 5beffca5..50a6a66a 100644 --- a/execution/src/posix/execution.cc +++ b/execution/src/posix/execution.cc @@ -314,10 +314,9 @@ namespace leatherman { namespace execution { } // Close all open file descriptors above stderr -#if defined(HAS_CLOSEFROM) +#ifdef HAS_CLOSEFROM closefrom(STDERR_FILENO + 1); #else - #if defined(HAS_PROC_PID) uint64_t fd; const char* fdpath = "/proc/self/fd"; std::list fd_list; @@ -332,10 +331,10 @@ namespace leatherman { namespace execution { for (auto fd : fd_list) { close(fd); } - } else //NOLINT - #endif // HAS_PROC_PID - for (uint64_t i = (STDERR_FILENO + 1); i < max_fd; ++i) { - close(i); + } else { + for (uint64_t i = (STDERR_FILENO + 1); i < max_fd; ++i) { + close(i); + } } #endif // HAS_CLOSEFROM