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

David Shea dshea at redhat.com
Thu Dec 4 15:09:18 UTC 2014


On 12/04/2014 09:23 AM, Vratislav Podzimek wrote:
> 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
>   

If you're willing to mess with _run_program some more, you could use 
execWithRedirect by changing where the program log lock is held. On 
master proc.communicate() happens outside of the lock, and the lock is 
only held for the log.info calls.


More information about the anaconda-patches mailing list