From b4e7355fd05ee02565fdceecbdfedde4bac60829 Mon Sep 17 00:00:00 2001
From: Todd L Miller <tlmiller@cs.wisc.edu>
Date: Tue, 14 Apr 2020 14:59:44 -0500
Subject: [PATCH] (#7594)  Refactor to make available for Create_Process() when
 we're feeling brave.  Automatically fall back the old (slow) way if we can't
 open /proc/self/fd for some reason.

---
 src/condor_utils/fork_utils.cpp | 52 ++++++++++++++++++++++++++++++
 src/condor_utils/fork_utils.h   | 12 +++++++
 src/condor_utils/my_popen.cpp   | 56 +++------------------------------
 3 files changed, 68 insertions(+), 52 deletions(-)
 create mode 100644 src/condor_utils/fork_utils.cpp
 create mode 100644 src/condor_utils/fork_utils.h

diff --git a/src/condor_utils/fork_utils.cpp b/src/condor_utils/fork_utils.cpp
new file mode 100644
index 00000000000..78b7d0d832b
--- /dev/null
+++ b/src/condor_utils/fork_utils.cpp
@@ -0,0 +1,52 @@
+#include "condor_common.h"
+#include "condor_debug.h"
+
+#include <set>
+#include "fork_utils.h"
+
+#if defined(LINUX)
+bool
+close_all_fds_quickly( const std::set<int> & exceptions ) {
+	DIR * dir = opendir( "/proc/self/fd" );
+	if( dir == NULL ) { return false; }
+
+	std::vector<int> fds;
+	char * endptr = NULL;
+	struct dirent * d = NULL;
+	while( (d = readdir(dir)) != NULL ) {
+		if( strcmp( d->d_name, "." ) == 0 || strcmp( d->d_name, ".." ) == 0 ) { continue; }
+		int fd = (int)strtol( d->d_name, & endptr, 10 );
+		ASSERT( * endptr == '\0' );
+
+		if(exceptions.count(fd) == 0) {
+			fds.push_back(fd);
+		}
+	}
+	// We buffer the FDs to close primarily to avoid disturbing the
+	// directory while we're iterating over it, but it also means that
+	// we don't accidentally close dir's underlying FD early.  Since
+	// we're ignoring close() failures anyway, don't bother to exclude
+	// the FD we're about to close here from the loop below.
+	(void)closedir(dir);
+
+	for( auto i : fds ) {
+		(void)close( i );
+	}
+
+	return true;
+}
+#endif
+
+void
+close_all_fds( const std::set<int> & exceptions ) {
+#if defined(LINUX)
+	if(close_all_fds_quickly( exceptions )) { return; }
+#endif /* defined(LINUX) */
+
+	int limit = getdtablesize();
+	for (int jj=3; jj < limit; jj++) {
+		if(exceptions.count(jj) == 0) {
+			close(jj);
+		}
+	}
+}
diff --git a/src/condor_utils/fork_utils.h b/src/condor_utils/fork_utils.h
new file mode 100644
index 00000000000..f8216cef415
--- /dev/null
+++ b/src/condor_utils/fork_utils.h
@@ -0,0 +1,12 @@
+#ifndef _CONDOR_FORK_UTILS_H
+#define _CONDOR_FORK_UTILS_H
+
+//
+// #include <set>
+// #include "fork_utils.h"
+//
+
+// This only works for single-threaded programs.
+void close_all_fds( const std::set<int> & exceptions );
+
+#endif /* _CONDOR_FORK_UTILS_H */
diff --git a/src/condor_utils/my_popen.cpp b/src/condor_utils/my_popen.cpp
index 478be4a8981..788acde1d03 100644
--- a/src/condor_utils/my_popen.cpp
+++ b/src/condor_utils/my_popen.cpp
@@ -27,6 +27,9 @@
 #include "env.h"
 #include "setenv.h"
 
+#include <set>
+#include "fork_utils.h"
+
 #ifdef WIN32
 #else
 #include <poll.h>
@@ -439,58 +442,7 @@ my_popenv_impl( const char *const args[],
 
 		/* The child */
 	if( pid == 0 ) {
-
-#if defined(LINUX)
-
-		/* Close all the FDs in /proc/self/fd except for the ones that
-		   we know and love.  Note that this only works because we're
-		   single-threaded (and therefore the contents of the directory
-		   can't chagne as we iterate). */
-		DIR * dir = opendir( "/proc/self/fd" );
-		ASSERT( dir != NULL );
-
-		std::vector<int> fds;
-		char * endptr = NULL;
-		struct dirent * d = NULL;
-		while( (d = readdir(dir)) != NULL ) {
-		    if( strcmp( d->d_name, "." ) == 0 || strcmp( d->d_name, ".." ) == 0 ) { continue; }
-			int fd = (int)strtol( d->d_name, & endptr, 10 );
-			ASSERT( * endptr == '\0' );
-			if( fd != pipe_d[0]  && fd != pipe_d[1]
-			 && fd != pipe_d2[0] && fd != pipe_d2[1]
-			 && fd != pipe_writedata[0] && fd != pipe_writedata[1] ) {
-				fds.push_back(fd);
-			}
-		}
-		(void)closedir(dir);
-
-		for( auto i : fds ) {
-			(void)close( i );
-		}
-
-#else /* defined(LINUX) */
-
-		/* Don't leak out fds from the parent to our child.
-		 * Wish there was a more efficient way to do this, but
-		 * this is how we do it in daemoncore CreateProcess...
-		 * Of course, do not close stdin/out/err or the fds to
-		 * the pipes we just created above.
-		 */
-		int limit = getdtablesize();
-		for (int jj=3; jj < limit; jj++) {
-			if (jj != pipe_d[0] &&
-				jj != pipe_d[1] &&
-				jj != pipe_d2[0] &&
-				jj != pipe_d2[1] &&
-				jj != pipe_writedata[0] &&
-				jj != pipe_writedata[1])
-			{
-				close(jj);
-			}
-		}
-
-#endif /* LINUX */
-
+		close_all_fds( { pipe_d[0], pipe_d[1], pipe_d2[0], pipe_d2[1], pipe_writedata[0],  pipe_writedata[1] } );
 		close(pipe_d2[0]);
 
 		if( parent_reads ) {