From 7b274dc7e257ca19d101b2529fd898a446964d1f Mon Sep 17 00:00:00 2001 From: Ansgar Burchardt Date: Sun, 20 Oct 2013 11:02:04 +0200 Subject: [PATCH] Reset signal handler to default action in child processes Python changes the signal handler to SIG_IGN for a few signals which causes unexpected behaviour in child processes. The wrapper functions in the new daklib.daksubprocess module reset them to SIG_DFL in child processes. Reference: http://bugs.python.org/issue1652 --- dak/examine_package.py | 14 +++++---- dak/process_new.py | 4 +-- daklib/archive.py | 6 ++-- daklib/contents.py | 12 ++----- daklib/daksubprocess.py | 70 +++++++++++++++++++++++++++++++++++++++++ daklib/dbconn.py | 16 +++------- daklib/filewriter.py | 2 +- daklib/utils.py | 5 +-- 8 files changed, 93 insertions(+), 36 deletions(-) create mode 100644 daklib/daksubprocess.py diff --git a/dak/examine_package.py b/dak/examine_package.py index 56bb477e..4cc116bb 100755 --- a/dak/examine_package.py +++ b/dak/examine_package.py @@ -56,7 +56,7 @@ import md5 import apt_pkg import apt_inst import shutil -import commands +import subprocess import threading from daklib import utils @@ -67,6 +67,7 @@ from daklib.regexes import html_escaping, re_html_escaping, re_version, re_space re_contrib, re_nonfree, re_localhost, re_newlinespace, \ re_package, re_doc_directory from daklib.dak_exceptions import ChangesUnicodeError +import daklib.daksubprocess ################################################################################ @@ -508,13 +509,14 @@ def get_readme_source (dsc_filename): tempdir = utils.temp_dirname() os.rmdir(tempdir) - cmd = "dpkg-source --no-check --no-copy -x %s %s" % (dsc_filename, tempdir) - (result, output) = commands.getstatusoutput(cmd) - if (result != 0): + cmd = ('dpkg-source', '--no-check', '--no-copy', '-x', dsc_filename, tempdir) + try: + daklib.daksubprocess.check_output(cmd, stderr=1) + except subprocess.CalledProcessError as e: res = "How is education supposed to make me feel smarter? Besides, every time I learn something new, it pushes some\n old stuff out of my brain. Remember when I took that home winemaking course, and I forgot how to drive?\n" res += "Error, couldn't extract source, WTF?\n" - res += "'dpkg-source -x' failed. return code: %s.\n\n" % (result) - res += output + res += "'dpkg-source -x' failed. return code: %s.\n\n" % (e.returncode) + res += e.output return res path = os.path.join(tempdir, 'debian/README.source') diff --git a/dak/process_new.py b/dak/process_new.py index f33b53ba..c25027dc 100755 --- a/dak/process_new.py +++ b/dak/process_new.py @@ -53,7 +53,7 @@ import contextlib import pwd import apt_pkg, apt_inst import examine_package -import subprocess +import daklib.daksubprocess from sqlalchemy import or_ from daklib.dbconn import * @@ -449,7 +449,7 @@ def run_user_inspect_command(upload, upload_copy): changes=changes, ) - subprocess.check_call(shell_command, shell=True) + daklib.daksubprocess.check_call(shell_command, shell=True) ################################################################################ diff --git a/daklib/archive.py b/daklib/archive.py index 6071566c..372ab8a9 100644 --- a/daklib/archive.py +++ b/daklib/archive.py @@ -26,12 +26,12 @@ import daklib.upload as upload import daklib.utils as utils from daklib.fstransactions import FilesystemTransaction from daklib.regexes import re_changelog_versions, re_bin_only_nmu +import daklib.daksubprocess import apt_pkg from datetime import datetime import os import shutil -import subprocess from sqlalchemy.orm.exc import NoResultFound import sqlalchemy.exc import tempfile @@ -695,7 +695,7 @@ class ArchiveUpload(object): sourcedir = os.path.join(self.directory, 'source') if not os.path.exists(sourcedir): devnull = open('/dev/null', 'w') - subprocess.check_call(["dpkg-source", "--no-copy", "--no-check", "-x", dsc_path, sourcedir], shell=False, stdout=devnull) + daklib.daksubprocess.check_call(["dpkg-source", "--no-copy", "--no-check", "-x", dsc_path, sourcedir], shell=False, stdout=devnull) if not os.path.isdir(sourcedir): raise Exception("{0} is not a directory after extracting source package".format(sourcedir)) return sourcedir @@ -1095,7 +1095,7 @@ class ArchiveUpload(object): continue script = rule['Script'] - retcode = subprocess.call([script, os.path.join(self.directory, f.filename), control['Version'], arch, os.path.join(self.directory, self.changes.filename)], shell=False) + retcode = daklib.daksubprocess.call([script, os.path.join(self.directory, f.filename), control['Version'], arch, os.path.join(self.directory, self.changes.filename)], shell=False) if retcode != 0: print "W: error processing {0}.".format(f.filename) remaining.append(f) diff --git a/daklib/contents.py b/daklib/contents.py index e5abcac6..f9c1feb1 100644 --- a/daklib/contents.py +++ b/daklib/contents.py @@ -31,11 +31,10 @@ from daklib.filewriter import BinaryContentsFileWriter, SourceContentsFileWriter from multiprocessing import Pool from shutil import rmtree -from subprocess import Popen, PIPE, check_call from tempfile import mkdtemp +import daklib.daksubprocess import os.path -import signal class BinaryContentsWriter(object): ''' @@ -383,12 +382,6 @@ def binary_scan_helper(binary_id): scanner = BinaryContentsScanner(binary_id) scanner.scan() - -def subprocess_setup(): - # Python installs a SIGPIPE handler by default. This is usually not what - # non-Python subprocesses expect. - signal.signal(signal.SIGPIPE, signal.SIG_DFL) - class UnpackedSource(object): ''' UnpackedSource extracts a source package into a temporary location and @@ -403,7 +396,7 @@ class UnpackedSource(object): self.root_directory = os.path.join(temp_directory, 'root') command = ('dpkg-source', '--no-copy', '--no-check', '-q', '-x', dscfilename, self.root_directory) - check_call(command, preexec_fn = subprocess_setup) + daklib.daksubprocess.check_call(command) def get_root_directory(self): ''' @@ -506,4 +499,3 @@ def source_scan_helper(source_id): scanner.scan() except Exception as e: print e - diff --git a/daklib/daksubprocess.py b/daklib/daksubprocess.py new file mode 100644 index 00000000..ff1df775 --- /dev/null +++ b/daklib/daksubprocess.py @@ -0,0 +1,70 @@ +"""subprocess management for dak + +@copyright: 2013, Ansgar Burchardt +@license: GPL-2+ +""" + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + +import signal +import subprocess + +# +def fix_signal_handlers(): + """reset signal handlers to default action. + + Python changes the signal handler to SIG_IGN for a few signals which + causes unexpected behaviour in child processes. This function resets + them to their default action. + + Reference: http://bugs.python.org/issue1652 + """ + for signal_name in ('SIGPIPE', 'SIGXFZ', 'SIGXFSZ'): + try: + signal_number = getattr(signal, signal_name) + signal.signal(signal_number, signal.SIG_DFL) + except AttributeError: + pass + +def _generate_preexec_fn(other_preexec_fn=None): + def preexec_fn(): + fix_signal_handlers() + if other_preexec_fn is not None: + other_preexec_fn() + return preexec_fn + +def call(*args, **kwargs): + """wrapper around subprocess.call that fixes signal handling""" + preexec_fn = _generate_preexec_fn(kwargs.get('preexec_fn')) + kwargs['preexec_fn'] = preexec_fn + return subprocess.call(*args, **kwargs) + +def check_call(*args, **kwargs): + """wrapper around subprocess.check_call that fixes signal handling""" + preexec_fn = _generate_preexec_fn(kwargs.get('preexec_fn')) + kwargs['preexec_fn'] = preexec_fn + return subprocess.check_call(*args, **kwargs) + +def check_output(*args, **kwargs): + """wrapper around subprocess.check_output that fixes signal handling""" + preexec_fn = _generate_preexec_fn(kwargs.get('preexec_fn')) + kwargs['preexec_fn'] = preexec_fn + return subprocess.check_output(*args, **kwargs) + +def Popen(*args, **kwargs): + """wrapper around subprocess.Popen that fixes signal handling""" + preexec_fn = _generate_preexec_fn(kwargs.get('preexec_fn')) + kwargs['preexec_fn'] = preexec_fn + return subprocess.Popen(*args, **kwargs) diff --git a/daklib/dbconn.py b/daklib/dbconn.py index a900a0e6..7d5a7481 100644 --- a/daklib/dbconn.py +++ b/daklib/dbconn.py @@ -34,13 +34,13 @@ ################################################################################ import apt_pkg +import daklib.daksubprocess import os from os.path import normpath import re import psycopg2 +import subprocess import traceback -import commands -import signal try: # python >= 2.6 @@ -52,7 +52,6 @@ except: from datetime import datetime, timedelta from errno import ENOENT from tempfile import mkstemp, mkdtemp -from subprocess import Popen, PIPE from tarfile import TarFile from inspect import getargspec @@ -498,11 +497,6 @@ __all__.append('BinContents') ################################################################################ -def subprocess_setup(): - # Python installs a SIGPIPE handler by default. This is usually not what - # non-Python subprocesses expect. - signal.signal(signal.SIGPIPE, signal.SIG_DFL) - class DBBinary(ORMObject): def __init__(self, package = None, source = None, version = None, \ maintainer = None, architecture = None, poolfile = None, \ @@ -539,8 +533,8 @@ class DBBinary(ORMObject): package does not contain any regular file. ''' fullpath = self.poolfile.fullpath - dpkg = Popen(['dpkg-deb', '--fsys-tarfile', fullpath], stdout = PIPE, - preexec_fn = subprocess_setup) + dpkg_cmd = ('dpkg-deb', '--fsys-tarfile', fullpath) + dpkg = daklib.daksubprocess.Popen(dpkg_cmd, stdout=subprocess.PIPE) tar = TarFile.open(fileobj = dpkg.stdout, mode = 'r|') for member in tar.getmembers(): if not member.isdir(): @@ -2939,5 +2933,3 @@ class DBConn(object): return session __all__.append('DBConn') - - diff --git a/daklib/filewriter.py b/daklib/filewriter.py index 7fa68289..148dd171 100644 --- a/daklib/filewriter.py +++ b/daklib/filewriter.py @@ -27,7 +27,7 @@ Helper code for file writing with optional compression. from daklib.config import Config -from subprocess import check_call +from daklib.daksubprocess import check_call import os, os.path diff --git a/daklib/utils.py b/daklib/utils.py index bc524aa9..fbe3b1a0 100755 --- a/daklib/utils.py +++ b/daklib/utils.py @@ -44,6 +44,7 @@ import subprocess import ldap import daklib.config as config +import daklib.daksubprocess from dbconn import DBConn, get_architecture, get_component, get_suite, \ get_override_type, Keyring, session_wrapper, \ get_active_keyring_paths, get_primary_keyring_path, \ @@ -77,7 +78,7 @@ known_hashes = [("sha1", apt_pkg.sha1sum, (1, 8)), # code in lenny's Python. This also affects commands.getoutput and # commands.getstatus. def dak_getstatusoutput(cmd): - pipe = subprocess.Popen(cmd, shell=True, universal_newlines=True, + pipe = daklib.daksubprocess.Popen(cmd, shell=True, universal_newlines=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT) output = pipe.stdout.read() @@ -1689,7 +1690,7 @@ def call_editor(text="", suffix=".txt"): try: print >>tmp, text, tmp.close() - subprocess.check_call([editor, tmp.name]) + daklib.daksubprocess.check_call([editor, tmp.name]) return open(tmp.name, 'r').read() finally: os.unlink(tmp.name) -- 2.39.5