[PATCH 1/2] Close file descriptors when running ntpdate (#1083978)

Vratislav Podzimek vpodzime at redhat.com
Thu Dec 4 14:23:10 UTC 2014


Using os.system() means that the child process inherits all file descriptors
from the parent. In case we have some file opened in the time when some
long-running process is forked, there is a high chance that the opened file
descriptor can cause issues. Like in the referenced bug where it prevented
filesystem from being unmounted.

Using subprocess.Popen() with close_fds=True is the easiest way to make sure
file descriptors are closed in the child process. All other ways seem to be
crazy machinery.

Signed-off-by: Vratislav Podzimek <vpodzime at redhat.com>
---
 pyanaconda/ntp.py | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/pyanaconda/ntp.py b/pyanaconda/ntp.py
index 0014a9d..a54e807 100644
--- a/pyanaconda/ntp.py
+++ b/pyanaconda/ntp.py
@@ -27,6 +27,7 @@ import re
 import os
 import tempfile
 import shutil
+import subprocess
 
 from pyanaconda import iutil
 from pyanaconda.threads import threadMgr, AnacondaThread
@@ -50,11 +51,13 @@ def ntp_server_working(server):
 
     """
 
-    #FIXME: It would be nice to use execWithRedirect here, but it is not
-    #       thread-safe and hangs if this function is called from threads.
-    #       By using tee (and block-buffered pipes) it is also much slower.
-    #we just need to know the exit status
-    retc = os.system("ntpdate -q %s &>/dev/null" % server)
+    # using iutil.execWithRedirect here would mean that if this function was run
+    # multiple times from multiple threads in the same time, the execution would
+    # be serialized because of the program_log lock
+    proc = subprocess.Popen(["ntpdate", "-q", server], close_fds=True)
+
+    # we just need to know the exit status
+    retc = proc.wait()
 
     return retc == 0
 
-- 
1.9.3



More information about the anaconda-patches mailing list