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(a)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