From: Vadim Zeitlin <vadim@wxwidgets.org>
Date: Sun, 14 Nov 2010 14:04:44 +0000 (+0000)
Subject: Restore code for closing inherited file descriptors in the child.
X-Git-Url: https://git.saurik.com/wxWidgets.git/commitdiff_plain/5a1d70f92523bb1aae591b95a5c0bd43a5e6d05b

Restore code for closing inherited file descriptors in the child.

The code closing all file descriptors inherited from the parent in the child
process created by wxExecute() was  removed in r57324 by mistake (probably
due the fact that its meaning was poorly explained) but we still do need to do
this, of course, to avoid descriptor "leaks" (e.g. the parent couldn't really
close any of them).

Restore the code for closing all unneeded file descriptors in the child in
slightly modified form and add a comment pointing to an URL explaining how to
do it better in the future.

Closes #12636.

git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@66153 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775
---

diff --git a/src/unix/utilsunx.cpp b/src/unix/utilsunx.cpp
index a96e20e639..455db74030 100644
--- a/src/unix/utilsunx.cpp
+++ b/src/unix/utilsunx.cpp
@@ -598,12 +598,6 @@ long wxExecute(char **argv, int flags, wxProcess *process,
         }
 #endif // !__VMS
 
-        // reading side can be safely closed but we should keep the write one
-        // opened, it will be only closed when the process terminates resulting
-        // in a read notification to the parent
-        execData.pipeEndProcDetect.Detach(wxPipe::Write);
-        execData.pipeEndProcDetect.Close();
-
         // redirect stdin, stdout and stderr
         if ( pipeIn.IsOk() )
         {
@@ -619,6 +613,39 @@ long wxExecute(char **argv, int flags, wxProcess *process,
             pipeErr.Close();
         }
 
+        // Close all (presumably accidentally) inherited file descriptors to
+        // avoid descriptor leaks. This means that we don't allow inheriting
+        // them purposefully but this seems like a lesser evil in wx code.
+        // Ideally we'd provide some flag to indicate that none (or some?) of
+        // the descriptors do not need to be closed but for now this is better
+        // than never closing them at all as wx code never used FD_CLOEXEC.
+
+        // Note that while the reading side of the end process detection pipe
+        // can be safely closed, we should keep the write one opened, it will
+        // be only closed when the process terminates resulting in a read
+        // notification to the parent
+        const int fdEndProc = execData.pipeEndProcDetect.Detach(wxPipe::Write);
+        execData.pipeEndProcDetect.Close();
+
+        // TODO: Iterating up to FD_SETSIZE is both inefficient (because it may
+        //       be quite big) and incorrect (because in principle we could
+        //       have more opened descriptions than this number). Unfortunately
+        //       there is no good portable solution for closing all descriptors
+        //       above a certain threshold but non-portable solutions exist for
+        //       most platforms, see [http://stackoverflow.com/questions/899038/
+        //          getting-the-highest-allocated-file-descriptor]
+        for ( int fd = 0; fd < (int)FD_SETSIZE; ++fd )
+        {
+            if ( fd != STDIN_FILENO  &&
+                 fd != STDOUT_FILENO &&
+                 fd != STDERR_FILENO &&
+                 fd != fdEndProc )
+            {
+                close(fd);
+            }
+        }
+
+
         // Process additional options if we have any
         if ( env )
         {