From 9ebfa8438583eb9af4456bfa8983339935ac561d Mon Sep 17 00:00:00 2001 From: Jacob Hurwitz Date: Thu, 3 Jan 2013 13:21:19 -0500 Subject: [PATCH] zmirror: Allow duplicate zmirror processes to die gracefully. Fixes #602. I replaced the SIGKILL with a SIGINT, and then catch SIGINT with a handler. This handler calls cancelSubs if necessary, and can later be edited to perform other clean-up operations, too. I thought about, in this same commit, changing the SIGTERM in maybe_restart_mirroring_script to a SIGINT, but after tracing out the code paths, I realized that isn't necessary. (The SIGTERM is necessarily performed on a process that has not subscribed to any zephyr classes, so cancelSubs is unnecessary. If we do think that we may want to add additional clean-up operations in the future, though, then it might be worth investigating changing this SIGTERM.) (imported from commit 692b295be6cb40b0e4ec2ca0bc58c58056ed9bd9) --- bots/zephyr_mirror.py | 7 +++++++ bots/zephyr_mirror_backend.py | 27 ++++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/bots/zephyr_mirror.py b/bots/zephyr_mirror.py index 1e5b1a6..a012515 100755 --- a/bots/zephyr_mirror.py +++ b/bots/zephyr_mirror.py @@ -27,9 +27,16 @@ import time import optparse import os import traceback +import signal from zephyr_mirror_backend import parse_args +def die(signal, frame): + # We actually want to exit, so run os._exit (so as not to be caught and restarted) + os._exit(1) + +signal.signal(signal.SIGINT, die) + (options, args) = parse_args() args = [os.path.join(options.root_path, "user_root", "zephyr_mirror_backend.py")] diff --git a/bots/zephyr_mirror_backend.py b/bots/zephyr_mirror_backend.py index 302b6ac..f40cb6d 100755 --- a/bots/zephyr_mirror_backend.py +++ b/bots/zephyr_mirror_backend.py @@ -39,6 +39,10 @@ import tempfile DEFAULT_SITE = "https://humbughq.com" +class States: + Startup, HumbugToZephyr, ZephyrToHumbug, ChildSending = range(4) +CURRENT_STATE = States.Startup + def to_humbug_username(zephyr_username): if "@" in zephyr_username: (user, realm) = zephyr_username.split("@") @@ -334,6 +338,8 @@ def process_notice(notice, log): log.flush() if os.fork() == 0: + global CURRENT_STATE + CURRENT_STATE = States.ChildSending # Actually send the message in a child process, to avoid blocking. try: res = send_humbug(zeph) @@ -774,6 +780,21 @@ def parse_args(): default=os.path.join(os.environ["HOME"], "Private", ".humbug-api-key")) return parser.parse_args() +def die_gracefully(signal, frame): + if CURRENT_STATE == States.HumbugToZephyr or CURRENT_STATE == States.ChildSending: + # this is a child process, so we want os._exit (no clean-up necessary) + os._exit(1) + + if CURRENT_STATE == States.ZephyrToHumbug: + try: + # zephyr=>humbug processes may have added subs, so run cancelSubs + zephyr._z.cancelSubs() + except IOError: + # We don't care whether we failed to cancel subs properly, but we should log it + logging.exception("") + + sys.exit(1) + if __name__ == "__main__": # Set the SIGCHLD handler back to SIG_DFL to prevent these errors # when importing the "requests" module after being restarted using @@ -783,6 +804,8 @@ if __name__ == "__main__": # IOError: [Errno 10] No child processes signal.signal(signal.SIGCHLD, signal.SIG_DFL) + signal.signal(signal.SIGINT, die_gracefully) + (options, args) = parse_args() # The 'api' directory needs to go first, so that 'import humbug' won't pick @@ -842,7 +865,7 @@ or specify the --api-key-file option.""" % (options.api_key_file,))) # Another copy of zephyr_mirror.py! Kill it. print "Killing duplicate zephyr_mirror process %s" % (pid,) try: - os.kill(pid, signal.SIGKILL) + os.kill(pid, signal.SIGINT) except OSError: # We don't care if the target process no longer exists, so just print the error traceback.print_exc() @@ -856,6 +879,7 @@ or specify the --api-key-file option.""" % (options.api_key_file,))) if options.forward_from_humbug: child_pid = os.fork() if child_pid == 0: + CURRENT_STATE = States.HumbugToZephyr # Run the humbug => zephyr mirror in the child logger = configure_logger("humbug=>zephyr") zsig_fullname = fetch_fullname(options.user) @@ -863,6 +887,7 @@ or specify the --api-key-file option.""" % (options.api_key_file,))) sys.exit(0) else: child_pid = None + CURRENT_STATE = States.ZephyrToHumbug import zephyr while True: