From 9ada74e700421fe02cf992641126075378954bc6 Mon Sep 17 00:00:00 2001 From: Stephen Gallagher Date: Wed, 29 May 2019 16:34:09 -0400 Subject: [PATCH] Fix memory issue with Module.search_streams() in the python bindings Signed-off-by: Stephen Gallagher --- 0001-Double-valgrind-timeout.patch | 29 ++++ 0002-Parallelize-the-valgrind-tests.patch | 136 ++++++++++++++++ ...nsfer-type-for-Module.search_streams.patch | 150 ++++++++++++++++++ 0004-Extend-timeout-for-header-test.patch | 31 ++++ libmodulemd.spec | 9 +- 5 files changed, 354 insertions(+), 1 deletion(-) create mode 100644 0001-Double-valgrind-timeout.patch create mode 100644 0002-Parallelize-the-valgrind-tests.patch create mode 100644 0003-Fix-transfer-type-for-Module.search_streams.patch create mode 100644 0004-Extend-timeout-for-header-test.patch diff --git a/0001-Double-valgrind-timeout.patch b/0001-Double-valgrind-timeout.patch new file mode 100644 index 0000000..e7f128a --- /dev/null +++ b/0001-Double-valgrind-timeout.patch @@ -0,0 +1,29 @@ +From 1a7bf143761ff8e3f4f6585b82c0be4dbd511fca Mon Sep 17 00:00:00 2001 +From: Stephen Gallagher +Date: Thu, 23 May 2019 14:00:36 -0400 +Subject: [PATCH 1/3] Double valgrind timeout + +On some architectures under heavy load, the valgrind check on v2 +is taking a long time. + +Signed-off-by: Stephen Gallagher +--- + modulemd/meson.build | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/modulemd/meson.build b/modulemd/meson.build +index 47bd1f58e6401d2634d8ff737c2b347f5ebc6bf5..e49c7a9df76dcf37a18ddeba3150d6c914aa4e25 100644 +--- a/modulemd/meson.build ++++ b/modulemd/meson.build +@@ -313,7 +313,7 @@ endif + if valgrind.found() + modulemd_valgrind_scripts = files('common/tests/test-valgrind.py') + test ('valgrind', python3, + env : test_env, + args : modulemd_valgrind_scripts, +- timeout : 600) ++ timeout : 1200) + endif +-- +2.21.0 + diff --git a/0002-Parallelize-the-valgrind-tests.patch b/0002-Parallelize-the-valgrind-tests.patch new file mode 100644 index 0000000..ba160a5 --- /dev/null +++ b/0002-Parallelize-the-valgrind-tests.patch @@ -0,0 +1,136 @@ +From d9b41f72d4b2d545b2600aff7bd8a27ed0093750 Mon Sep 17 00:00:00 2001 +From: Stephen Gallagher +Date: Wed, 29 May 2019 11:33:57 -0400 +Subject: [PATCH 2/3] Parallelize the valgrind tests + +This considerably reduces the time needed to perform the valgrind +memory tests on systems with multiple available processors. In the +case of my laptop, the duration was reduced from ~200s to 90s. + +Signed-off-by: Stephen Gallagher +--- + modulemd/common/tests/test-valgrind.py | 74 ++++++++++++++------------ + 1 file changed, 41 insertions(+), 33 deletions(-) + +diff --git a/modulemd/common/tests/test-valgrind.py b/modulemd/common/tests/test-valgrind.py +index 9be72c705fde79aa305d831eaa4f31f7e2cc663f..9349749658ccca0529776f3d534664d614c1cb4d 100644 +--- a/modulemd/common/tests/test-valgrind.py ++++ b/modulemd/common/tests/test-valgrind.py +@@ -16,13 +16,16 @@ import os + import sys + import subprocess + import tempfile + import xml.etree.ElementTree as ET + ++from multiprocessing import Pool, TimeoutError ++ + if os.getenv('MMD_SKIP_VALGRIND'): + sys.exit(77) + ++ + failed = False + + # Get the list of tests to run + proc_result = subprocess.run(['meson', 'test', '--list'], + stdout=subprocess.PIPE, +@@ -47,13 +50,13 @@ for test in unfiltered_tests: + test == 'test_dirty_repo' or + test == 'valgrind'): + continue + tests.append(test) + ++ + with tempfile.TemporaryDirectory(prefix="libmodulemd_valgrind_") as tmpdirname: +- for test in tests: +- # TODO: auto-detect the location of the suppression file ++ def exec_valgrind(test): + valgrind_command = "/usr/bin/valgrind " \ + "--leak-check=full " \ + "--suppressions=/usr/share/glib-2.0/valgrind/glib.supp " \ + "--xml=yes " \ + "--xml-file=%s/%s.xml " % (tmpdirname, test) +@@ -64,45 +67,50 @@ with tempfile.TemporaryDirectory(prefix="libmodulemd_valgrind_") as tmpdirname: + '-t', '10', + '--logbase=%s' % test, + '--wrap=%s' % valgrind_command, + test]) + +- if proc_result.returncode != 0: +- print("Valgrind exited with an error on %s" % test, +- file=sys.stderr) +- failed = True +- continue ++ return proc_result.returncode, test + +- # Process the XML for leaks +- tree = ET.parse('%s/%s.xml' % (tmpdirname, test)) +- root = tree.getroot() ++ with Pool() as pool: ++ for returncode, test in pool.map(exec_valgrind, tests): ++ if returncode != 0: ++ print("Valgrind exited with an error on %s" % test, ++ file=sys.stderr) ++ failed = True ++ continue + +- for root_child in root: +- if (root_child.tag == "error"): +- for error_child in root_child: +- if error_child.tag == 'kind': +- if error_child.text == 'Leak_DefinitelyLost': +- print("Memory leak detected in %s" % test, +- file=sys.stderr) +- failed = True ++ # Process the XML for leaks ++ tree = ET.parse('%s/%s.xml' % (tmpdirname, test)) ++ root = tree.getroot() + +- elif error_child.text == 'InvalidFree': +- print("Invalid free() detected in %s" % test, +- file=sys.stderr) +- failed = True ++ for root_child in root: ++ if (root_child.tag == "error"): ++ for error_child in root_child: ++ if error_child.tag == 'kind': ++ if error_child.text == 'Leak_DefinitelyLost': ++ print("Memory leak detected in %s" % test, ++ file=sys.stderr) ++ failed = True + +- elif error_child.text == 'InvalidRead': +- print("Invalid read detected in %s" % test, +- file=sys.stderr) +- failed = True ++ elif error_child.text == 'InvalidFree': ++ print("Invalid free() detected in %s" % test, ++ file=sys.stderr) ++ failed = True + +- elif error_child.text == 'UninitCondition': +- print("Uninitialized usage detected in %s" % test, +- file=sys.stderr) +- failed = True +- if failed: +- with open('%s/%s.xml' % (tmpdirname, test), 'r') as xml: +- print(xml.read()) ++ elif error_child.text == 'InvalidRead': ++ print("Invalid read detected in %s" % test, ++ file=sys.stderr) ++ failed = True ++ ++ elif error_child.text == 'UninitCondition': ++ print( ++ "Uninitialized usage detected in %s" % ++ test, file=sys.stderr) ++ failed = True ++ if failed: ++ with open('%s/%s.xml' % (tmpdirname, test), 'r') as xml: ++ print(xml.read()) + + + if failed: + sys.exit(1) +-- +2.21.0 + diff --git a/0003-Fix-transfer-type-for-Module.search_streams.patch b/0003-Fix-transfer-type-for-Module.search_streams.patch new file mode 100644 index 0000000..40ba286 --- /dev/null +++ b/0003-Fix-transfer-type-for-Module.search_streams.patch @@ -0,0 +1,150 @@ +From 340e316bd6384562086b4e381c8cd42b1ccd0781 Mon Sep 17 00:00:00 2001 +From: Stephen Gallagher +Date: Tue, 28 May 2019 14:28:30 -0400 +Subject: [PATCH 3/3] Fix transfer type for Module.search_streams() + +Technically this is an API-breaking change, but no one is using it +yet and it was always expected to be managed this way. + +Fixes: https://github.com/fedora-modularity/libmodulemd/issues/308 + +Signed-off-by: Stephen Gallagher +--- + modulemd/meson.build | 1 + + .../v2/include/modulemd-2.0/modulemd-module.h | 4 +- + modulemd/v2/meson.build | 17 ++++++- + modulemd/v2/tests/ModulemdTests/module.py | 46 +++++++++++++++++++ + 4 files changed, 65 insertions(+), 3 deletions(-) + create mode 100644 modulemd/v2/tests/ModulemdTests/module.py + +diff --git a/modulemd/meson.build b/modulemd/meson.build +index e49c7a9df76dcf37a18ddeba3150d6c914aa4e25..e5912d4041ba3e427d13b98c0eeca5217d48244b 100644 +--- a/modulemd/meson.build ++++ b/modulemd/meson.build +@@ -229,10 +229,11 @@ test_v2_python_scripts = files( + 'v2/tests/ModulemdTests/componentrpm.py', + 'v2/tests/ModulemdTests/defaults.py', + 'v2/tests/ModulemdTests/defaultsv1.py', + 'v2/tests/ModulemdTests/dependencies.py', + 'v2/tests/ModulemdTests/merger.py', ++ 'v2/tests/ModulemdTests/module.py', + 'v2/tests/ModulemdTests/moduleindex.py', + 'v2/tests/ModulemdTests/modulestream.py', + 'v2/tests/ModulemdTests/profile.py', + 'v2/tests/ModulemdTests/rpmmap.py', + 'v2/tests/ModulemdTests/servicelevel.py', +diff --git a/modulemd/v2/include/modulemd-2.0/modulemd-module.h b/modulemd/v2/include/modulemd-2.0/modulemd-module.h +index 45be9c0ae96e203707da61d84f18c71c4a826035..0219cfe227652813d20bdf736f9033782084c5ad 100644 +--- a/modulemd/v2/include/modulemd-2.0/modulemd-module.h ++++ b/modulemd/v2/include/modulemd-2.0/modulemd-module.h +@@ -129,12 +129,12 @@ modulemd_module_get_stream_by_NSVC (ModulemdModule *self, + * @context: (nullable): The context of the stream to retrieve. If NULL, the + * context is not included in the search. + * @arch: (nullable): The processor architecture of the stream to retrieve. If + * NULL, the architecture is not included in the search. + * +- * Returns: (transfer full) (element-type ModulemdModuleStream): The list of +- * stream objects matching the requested parameters. This function cannot ++ * Returns: (transfer container) (element-type ModulemdModuleStream): The list ++ * of stream objects matching the requested parameters. This function cannot + * fail, but it may return a zero-length list if no matches were found. The + * returned streams will be in a predictable order, sorted first by stream + * name, then by version (highest to lowest), then by context and finally by + * architecture. + * +diff --git a/modulemd/v2/meson.build b/modulemd/v2/meson.build +index 3f45db2c4e0e9a8996c74dffd949d5276082dc6f..e8a5a38f0528c4f860f0b84ef63609ff5fd89caa 100644 +--- a/modulemd/v2/meson.build ++++ b/modulemd/v2/meson.build +@@ -390,20 +390,35 @@ test ('dependencies_python2_debug', python2, + args : dependencies_python_script) + test ('dependencies_python2_release', python2, + env : py_test_release_env, + args : dependencies_python_script) + ++# -- Test Modulemd.Module (Python) -- # ++module_python_script = files('tests/ModulemdTests/module.py') ++test ('module_python3_debug', python3, ++ env : py_test_env, ++ args : module_python_script) ++test ('module_python3_release', python3, ++ env : py_test_release_env, ++ args : module_python_script) ++ ++test ('module_python2_debug', python2, ++ env : py_test_env, ++ args : module_python_script) ++test ('module_python2_release', python2, ++ env : py_test_release_env, ++ args : module_python_script) ++ + # -- Test Modulemd.ModuleIndex (Python) -- # + moduleindex_python_script = files('tests/ModulemdTests/moduleindex.py') + test ('moduleindex_python3_debug', python3, + env : py_test_env, + args : moduleindex_python_script) + test ('moduleindex_python3_release', python3, + env : py_test_release_env, + args : moduleindex_python_script) + +-moduleindex_python_script = files('tests/ModulemdTests/moduleindex.py') + test ('moduleindex_python2_debug', python2, + env : py_test_env, + args : moduleindex_python_script) + test ('moduleindex_python2_release', python2, + env : py_test_release_env, +diff --git a/modulemd/v2/tests/ModulemdTests/module.py b/modulemd/v2/tests/ModulemdTests/module.py +new file mode 100644 +index 0000000000000000000000000000000000000000..b604d9c9b357c4a5211d3ba5b4d0aba08c3a42bd +--- /dev/null ++++ b/modulemd/v2/tests/ModulemdTests/module.py +@@ -0,0 +1,46 @@ ++#!/usr/bin/python3 ++ ++# This file is part of libmodulemd ++# Copyright (C) 2017-2018 Stephen Gallagher ++# ++# Fedora-License-Identifier: MIT ++# SPDX-2.0-License-Identifier: MIT ++# SPDX-3.0-License-Identifier: MIT ++# ++# This program is free software. ++# For more information on the license, see COPYING. ++# For more information on free software, see ++# . ++ ++from os import path ++import sys ++try: ++ import unittest ++ import gi ++ gi.require_version('Modulemd', '2.0') ++ from gi.repository import Modulemd ++ from gi.repository.Modulemd import ModuleIndex ++ from gi.repository import GLib ++except ImportError: ++ # Return error 77 to skip this test on platforms without the necessary ++ # python modules ++ sys.exit(77) ++ ++from base import TestBase ++ ++ ++class TestModule(TestBase): ++ ++ def test_search_streams(self): ++ idx = Modulemd.ModuleIndex.new() ++ idx.update_from_file(path.join(self.source_root, ++ "modulemd/v2/tests/test_data/f29.yaml"), ++ True) ++ module = idx.get_module('nodejs') ++ ++ self.assertEquals(len(module.search_streams('8', 0)), 1) ++ self.assertEquals(len(module.search_streams('10', 0)), 1) ++ ++ ++if __name__ == '__main__': ++ unittest.main() +-- +2.21.0 + diff --git a/0004-Extend-timeout-for-header-test.patch b/0004-Extend-timeout-for-header-test.patch new file mode 100644 index 0000000..b4ba12c --- /dev/null +++ b/0004-Extend-timeout-for-header-test.patch @@ -0,0 +1,31 @@ +From 82e56d78e46504aab5917b606eae67e1a3b54603 Mon Sep 17 00:00:00 2001 +From: Stephen Gallagher +Date: Wed, 29 May 2019 16:18:30 -0400 +Subject: [PATCH 4/4] Extend timeout for header test + +Signed-off-by: Stephen Gallagher +--- + modulemd/meson.build | 3 ++- + 1 file changed, 2 insertions(+), 1 deletion(-) + +diff --git a/modulemd/meson.build b/modulemd/meson.build +index e5912d4041ba3e427d13b98c0eeca5217d48244b..0741736a12b4457559ade0a5372ff4b25743fce3 100644 +--- a/modulemd/meson.build ++++ b/modulemd/meson.build +@@ -306,11 +306,12 @@ endif + if build_api_v2 + subdir('v2') + import_header_script = find_program('common/tests/test-import-headers.sh') + test ('test_v2_import_headers', import_header_script, + env : test_env, +- args : modulemd_v2_hdrs) ++ args : modulemd_v2_hdrs, ++ timeout : 300) + endif + + if valgrind.found() + modulemd_valgrind_scripts = files('common/tests/test-valgrind.py') + test ('valgrind', python3, +-- +2.21.0 + diff --git a/libmodulemd.spec b/libmodulemd.spec index 5cbd9ad..eb136d9 100644 --- a/libmodulemd.spec +++ b/libmodulemd.spec @@ -12,7 +12,7 @@ Name: libmodulemd Version: %{libmodulemd_version} -Release: 1%{?dist} +Release: 2%{?dist} Summary: Module metadata manipulation library License: MIT @@ -42,6 +42,10 @@ Conflicts: libmodulemd1 < %{libmodulemd_v1_version}-%{release} # Patches +Patch0001: 0001-Double-valgrind-timeout.patch +Patch0002: 0002-Parallelize-the-valgrind-tests.patch +Patch0003: 0003-Fix-transfer-type-for-Module.search_streams.patch +Patch0004: 0004-Extend-timeout-for-header-test.patch %description @@ -231,6 +235,9 @@ ln -s libmodulemd.so.%{libmodulemd_v1_version} \ %{_datadir}/gtk-doc/html/modulemd-1.0/ %changelog +* Wed May 29 2019 Stephen Gallagher - 2.5.0-2 +- Fix memory issue with Module.search_streams() in the python bindings + * Wed May 22 2019 Stephen Gallagher - 2.5.0-1 - Update to 2.5.0 and 1.8.11 - Ensure that XMD is always emitted in the same order