On 22.3.2012 16:35, Martin Kosek wrote:
On Tue, 2012-03-20 at 17:48 +0100, Jan Cholasta wrote:
Propagate SIGINT to child process in ipautil.run.
Wait for the child process to terminate before continuing.
Do cleanup on KeyboardInterrupt rather than in custom SIGINT handler in
ipa-replica-conncheck.
https://fedorahosted.org/freeipa/ticket/2127
Honza
This looks and works OK, I have just one minor issue. Isn't the extra
p.wait() you added to the standard run() path redundant? p.communicate()
should do the job of waiting until the child process terminates.
+ try:
+ p = subprocess.Popen(args, stdin=p_in, stdout=p_out,
stderr=p_err,
+ close_fds=True, env=env)
+ stdout,stderr = p.communicate(stdin)
+ stdout,stderr = str(stdout), str(stderr) # Make pylint happy
+ p.wait()
+ except KeyboardInterrupt:
You are of course right, I guess I should read documentation more carefully.
Martin
Also, SIGINT is already propagated to the child process, we just need to
wait for it to terminate.
Updated patch attached.
Honza
--
Jan Cholasta
>From d664a9fa9ab94a0dc65b8d1057685dba629627b7 Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Tue, 20 Mar 2012 12:29:36 -0400
Subject: [PATCH] Wait for child process to terminate after receiving SIGINT
in ipautil.run.
Do cleanup on KeyboardInterrupt rather than in custom SIGINT handler in
ipa-replica-conncheck.
https://fedorahosted.org/freeipa/ticket/2127
---
install/tools/ipa-replica-conncheck | 13 +++++--------
ipapython/ipautil.py | 19 +++++++++++--------
2 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/install/tools/ipa-replica-conncheck b/install/tools/ipa-replica-conncheck
index 44b3caa..23411a3 100755
--- a/install/tools/ipa-replica-conncheck
+++ b/install/tools/ipa-replica-conncheck
@@ -158,12 +158,10 @@ def clean_responders(responders):
responders.remove(responder)
def sigterm_handler(signum, frame):
- print_info("\nCleaning up...")
-
- global RESPONDERS
- clean_responders(RESPONDERS)
-
- sys.exit(1)
+ # do what SIGINT does (raise a KeyboardInterrupt)
+ sigint_handler = signal.getsignal(signal.SIGINT)
+ if callable(sigint_handler):
+ sigint_handler(signum, frame)
def configure_krb5_conf(realm, kdc, filename):
@@ -268,7 +266,6 @@ def main():
root_logger.debug("missing options might be asked for interactively later\n")
signal.signal(signal.SIGTERM, sigterm_handler)
- signal.signal(signal.SIGINT, sigterm_handler)
required_ports = BASE_PORTS
if options.check_ca:
@@ -384,6 +381,7 @@ if __name__ == "__main__":
except SystemExit, e:
sys.exit(e)
except KeyboardInterrupt:
+ print_info("\nCleaning up...")
sys.exit(1)
except RuntimeError, e:
sys.exit(e)
@@ -395,4 +393,3 @@ if __name__ == "__main__":
os.remove(file_name)
except OSError:
pass
-
diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 416ebf5..d3bb38a 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -26,7 +26,6 @@ IPA_BASEDN_INFO = 'ipa v2.0'
import string
import tempfile
-from ipapython.ipa_log_manager import *
import subprocess
import random
import os, sys, traceback, readline
@@ -37,14 +36,14 @@ import urllib2
import socket
import ldap
import struct
-
-from ipapython import ipavalidate
from types import *
-
import re
import xmlrpclib
import datetime
import netaddr
+
+from ipapython.ipa_log_manager import *
+from ipapython import ipavalidate
from ipapython import config
try:
from subprocess import CalledProcessError
@@ -259,10 +258,14 @@ def run(args, stdin=None, raiseonerr=True,
p_out = subprocess.PIPE
p_err = subprocess.PIPE
- p = subprocess.Popen(args, stdin=p_in, stdout=p_out, stderr=p_err,
- close_fds=True, env=env)
- stdout,stderr = p.communicate(stdin)
- stdout,stderr = str(stdout), str(stderr) # Make pylint happy
+ try:
+ p = subprocess.Popen(args, stdin=p_in, stdout=p_out, stderr=p_err,
+ close_fds=True, env=env)
+ stdout,stderr = p.communicate(stdin)
+ stdout,stderr = str(stdout), str(stderr) # Make pylint happy
+ except KeyboardInterrupt:
+ p.wait()
+ raise
# The command and its output may include passwords that we don't want
# to log. Run through the nolog items.
--
1.7.7.6
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
