Skip to content

Commit

Permalink
Handle subprocess disallowing preexec during shutdown
Browse files Browse the repository at this point in the history 
In Python 3.12, you cannot pass a preexec_fn to subprocess.Popen
during interpreter shutdown:

python/cpython#104826

This avoids the problem by giving startProgram an arg to not
use a preexec_fn, and passing that all the way from anaconda's
exitHandler through execWithRedirect and _run_program.

Signed-off-by: Adam Williamson <awilliam@redhat.com>
  • Loading branch information
@AdamWill
AdamWill committed Jul 5, 2023
1 parent d61b8cc commit 6f14a16
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 38 deletions.
8 changes: 4 additions & 4 deletions anaconda.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,15 +89,15 @@ def exitHandler(rebootData):
util.dracut_eject(device_path)

if flags.kexec:
util.execWithRedirect("systemctl", ["--no-wall", "kexec"])
util.execWithRedirect("systemctl", ["--no-wall", "kexec"], do_preexec=False)
while True:
time.sleep(10000)
elif rebootData.action == KS_SHUTDOWN:
util.execWithRedirect("systemctl", ["--no-wall", "poweroff"])
util.execWithRedirect("systemctl", ["--no-wall", "poweroff"], do_preexec=False)
elif rebootData.action == KS_WAIT:
util.execWithRedirect("systemctl", ["--no-wall", "halt"])
util.execWithRedirect("systemctl", ["--no-wall", "halt"], do_preexec=False)
else: # reboot action is KS_REBOOT or None
util.execWithRedirect("systemctl", ["--no-wall", "reboot"])
util.execWithRedirect("systemctl", ["--no-wall", "reboot"], do_preexec=False)


def parse_arguments(argv=None, boot_cmdline=None):
Expand Down
76 changes: 42 additions & 34 deletions pyanaconda/core/util.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ def augmentEnv():


def startProgram(argv, root='/', stdin=None, stdout=subprocess.PIPE, stderr=subprocess.STDOUT,
env_prune=None, env_add=None, reset_handlers=True, reset_lang=True, **kwargs):
env_prune=None, env_add=None, reset_handlers=True, reset_lang=True,
do_preexec=True, **kwargs):
""" Start an external program and return the Popen object.
The root and reset_handlers arguments are handled by passing a
Expand All @@ -93,6 +94,7 @@ def startProgram(argv, root='/', stdin=None, stdout=subprocess.PIPE, stderr=subp
:param env_add: environment variables to add before execution
:param reset_handlers: whether to reset to SIG_DFL any signal handlers set to SIG_IGN
:param reset_lang: whether to set the locale of the child process to C
:param do_preexec: whether to use the preexec function
:param kwargs: Additional parameters to pass to subprocess.Popen
:return: A Popen object for the running command.
"""
Expand All @@ -105,32 +107,11 @@ def startProgram(argv, root='/', stdin=None, stdout=subprocess.PIPE, stderr=subp
if target_root == conf.target.physical_root:
target_root = conf.target.system_root

# Check for and save a preexec_fn argument
preexec_fn = kwargs.pop("preexec_fn", None)

# Map reset_handlers to the restore_signals Popen argument.
# restore_signals handles SIGPIPE, and preexec below handles any additional
# signals ignored by anaconda.
restore_signals = reset_handlers

def preexec():
# If a target root was specificed, chroot into it
if target_root and target_root != '/':
os.chroot(target_root)
os.chdir("/")

# Signal handlers set to SIG_IGN persist across exec. Reset
# these to SIG_DFL if requested. In particular this will include the
# SIGPIPE handler set by python.
if reset_handlers:
for signum in range(1, signal.NSIG):
if signal.getsignal(signum) == signal.SIG_IGN:
signal.signal(signum, signal.SIG_DFL)

# If the user specified an additional preexec_fn argument, run it
if preexec_fn is not None:
preexec_fn()

with program_log_lock:
if target_root != '/':
program_log.info("Running in chroot '%s'... %s", target_root, " ".join(argv))
Expand All @@ -148,13 +129,38 @@ def preexec():
env.update(env_add)

# pylint: disable=subprocess-popen-preexec-fn
return subprocess.Popen(argv,
stdin=stdin,
stdout=stdout,
stderr=stderr,
close_fds=True,
restore_signals=restore_signals,
preexec_fn=preexec, cwd=root, env=env, **kwargs)
partsubp = functools.partial(subprocess.Popen,
argv,
stdin=stdin,
stdout=stdout,
stderr=stderr,
close_fds=True,
restore_signals=restore_signals,
cwd=root, env=env, **kwargs)
if do_preexec:
# Check for and save a preexec_fn argument
preexec_fn = kwargs.pop("preexec_fn", None)

def preexec():
# If a target root was specificed, chroot into it
if target_root and target_root != '/':
os.chroot(target_root)
os.chdir("/")

# Signal handlers set to SIG_IGN persist across exec. Reset
# these to SIG_DFL if requested. In particular this will include the
# SIGPIPE handler set by python.
if reset_handlers:
for signum in range(1, signal.NSIG):
if signal.getsignal(signum) == signal.SIG_IGN:
signal.signal(signum, signal.SIG_DFL)

# If the user specified an additional preexec_fn argument, run it
if preexec_fn is not None:
preexec_fn()

return partsubp(preexec_fn=preexec)
return partsubp()


class X11Status:
Expand Down Expand Up @@ -253,7 +259,7 @@ def sigusr1_preexec():


def _run_program(argv, root='/', stdin=None, stdout=None, env_prune=None, log_output=True,
binary_output=False, filter_stderr=False):
binary_output=False, filter_stderr=False, do_preexec=True):
""" Run an external program, log the output and return it to the caller
NOTE/WARNING: UnicodeDecodeError will be raised if the output of the of the
Expand All @@ -267,6 +273,7 @@ def _run_program(argv, root='/', stdin=None, stdout=None, env_prune=None, log_ou
:param log_output: whether to log the output of command
:param binary_output: whether to treat the output of command as binary data
:param filter_stderr: whether to exclude the contents of stderr from the returned output
:param do_preexec: whether to use a preexec_fn for subprocess.Popen
:return: The return code of the command and the output
"""
try:
Expand All @@ -276,7 +283,7 @@ def _run_program(argv, root='/', stdin=None, stdout=None, env_prune=None, log_ou
stderr = subprocess.STDOUT

proc = startProgram(argv, root=root, stdin=stdin, stdout=subprocess.PIPE, stderr=stderr,
env_prune=env_prune)
env_prune=env_prune, do_preexec=do_preexec)

(output_string, err_string) = proc.communicate()
if not binary_output:
Expand Down Expand Up @@ -322,8 +329,8 @@ def _run_program(argv, root='/', stdin=None, stdout=None, env_prune=None, log_ou
return (proc.returncode, output_string)


def execWithRedirect(command, argv, stdin=None, stdout=None,
root='/', env_prune=None, log_output=True, binary_output=False):
def execWithRedirect(command, argv, stdin=None, stdout=None, root='/', env_prune=None,
log_output=True, binary_output=False, do_preexec=True):
""" Run an external program and redirect the output to a file.
:param command: The command to run
Expand All @@ -334,11 +341,12 @@ def execWithRedirect(command, argv, stdin=None, stdout=None,
:param env_prune: environment variable to remove before execution
:param log_output: whether to log the output of command
:param binary_output: whether to treat the output of command as binary data
:param do_preexec: whether to use a preexec_fn for subprocess.Popen
:return: The return code of the command
"""
argv = [command] + argv
return _run_program(argv, stdin=stdin, stdout=stdout, root=root, env_prune=env_prune,
log_output=log_output, binary_output=binary_output)[0]
log_output=log_output, binary_output=binary_output, do_preexec=do_preexec)[0]


def execWithCapture(command, argv, stdin=None, root='/', log_output=True, filter_stderr=False):
Expand Down

0 comments on commit 6f14a16

Please sign in to comment.