Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SignalHandlingMultiprocessingProcess bugs #54174

Open
dwoz opened this issue Aug 9, 2019 · 1 comment

Comments

@dwoz
Copy link
Contributor

commented Aug 9, 2019

Misuse of multiprocessing.utils.register_after_fork causes multiple bugs in salt.utils.process. Every fork will call the after fork methods registered, not just the fork associated with the process it is intended for. The following code illustrates the issue. An un-related process can cause a SignalHandlingMultiprocessingProcess to get killed off prematurely.

import os
import signal
import logging
from salt.utils.process import SignalHandlingMultiprocessingProcess, log
import time
import multiprocessing

log.addHandler(logging.StreamHandler())
log.setLevel(logging.DEBUG)

evt = multiprocessing.Event()

def target():
    def run_forever():
        while not evt.is_set():
            time.sleep(1)
    p = multiprocessing.Process(target=run_forever)
    p.start()
    p.join()

sh_proc = SignalHandlingMultiprocessingProcess(target=target)
sh_proc.start()

def kill_target_sub_proc():
    pid = os.fork()
    if pid == 0:
        return
    pid = os.fork()
    if pid == 0:
        return
    time.sleep(.1)
    try:
        os.kill(os.getpid(), signal.SIGINT)
    except KeyboardInterrupt:
        pass

proc = multiprocessing.Process(target=kill_target_sub_proc)
proc.start()
proc.join()

# When the bug exists, the kill_target_sub_proc signal will kill both
# processes. sh_proc will be alive if the bug is fixed
assert sh_proc.is_alive()

evt.set()
sh_proc.join()

Versions Report

All current versions are affected.

@dmurphy18

This comment has been minimized.

Copy link
Contributor

commented Aug 12, 2019

@dwoz Giving this to you from Triage since you have fix for it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.