From 454048f2f678c78e8405aaf5b85ac1e75c8e77d5 Mon Sep 17 00:00:00 2001 From: ohemorange Date: Thu, 14 Aug 2025 17:13:33 -0700 Subject: [PATCH] Fix memory leak in apache unit tests (#10421) This was causing oldest tests to fail on my mac, which has an open file limit of 256. Locks were being released at exit, but there were more than 256 tests being run at once. Holding onto the file descriptor for temporary files was making us keep the files open. I also removed unnecessary setUps and tearDowns in subclasses so that this could be fixed in only one spot. If you wanted to do any testing locally, I was throwing this in places: ``` import errno, os, resource open_file_handles = [] for fd in range(resource.getrlimit(resource.RLIMIT_NOFILE)[0]): try: os.fstat(fd) except OSError as e: if e.errno == errno.EBADF: continue open_file_handles.append(fd) print(f'location description: {len(open_file_handles)}') ``` --- .../_internal/tests/complex_parsing_test.py | 6 ------ .../tests/configurator_reverter_test.py | 6 ------ .../_internal/tests/parser_test.py | 16 ---------------- .../src/certbot_apache/_internal/tests/util.py | 2 ++ 4 files changed, 2 insertions(+), 28 deletions(-) diff --git a/certbot-apache/src/certbot_apache/_internal/tests/complex_parsing_test.py b/certbot-apache/src/certbot_apache/_internal/tests/complex_parsing_test.py index a07362b02..16ab367c4 100644 --- a/certbot-apache/src/certbot_apache/_internal/tests/complex_parsing_test.py +++ b/certbot-apache/src/certbot_apache/_internal/tests/complex_parsing_test.py @@ -1,5 +1,4 @@ """Tests for certbot_apache._internal.parser.""" -import shutil import sys import pytest @@ -20,11 +19,6 @@ class ComplexParserTest(util.ParserTest): # until after self.parser.parse_modules() # pylint: disable=protected-access - def tearDown(self): - shutil.rmtree(self.temp_dir) - shutil.rmtree(self.config_dir) - shutil.rmtree(self.work_dir) - def setup_variables(self): """Set up variables for parser.""" self.parser.variables.update( diff --git a/certbot-apache/src/certbot_apache/_internal/tests/configurator_reverter_test.py b/certbot-apache/src/certbot_apache/_internal/tests/configurator_reverter_test.py index f8c77308a..1c69334bf 100644 --- a/certbot-apache/src/certbot_apache/_internal/tests/configurator_reverter_test.py +++ b/certbot-apache/src/certbot_apache/_internal/tests/configurator_reverter_test.py @@ -1,5 +1,4 @@ """Test for certbot_apache._internal.configurator implementations of reverter""" -import shutil import sys from unittest import mock @@ -20,11 +19,6 @@ class ConfiguratorReverterTest(util.ApacheTest): self.vh_truth = util.get_vh_truth(self.temp_dir, "debian_apache_2_4/multiple_vhosts") - def tearDown(self): - shutil.rmtree(self.config_dir) - shutil.rmtree(self.work_dir) - shutil.rmtree(self.temp_dir) - def test_bad_save_checkpoint(self): self.config.reverter.add_to_checkpoint = mock.Mock(side_effect=errors.ReverterError) self.config.parser.add_dir(self.vh_truth[0].path, "Test", "bad_save_ckpt") diff --git a/certbot-apache/src/certbot_apache/_internal/tests/parser_test.py b/certbot-apache/src/certbot_apache/_internal/tests/parser_test.py index 0711128bb..20cbd50aa 100644 --- a/certbot-apache/src/certbot_apache/_internal/tests/parser_test.py +++ b/certbot-apache/src/certbot_apache/_internal/tests/parser_test.py @@ -1,5 +1,4 @@ """Tests for certbot_apache._internal.parser.""" -import shutil import sys from unittest import mock @@ -13,14 +12,6 @@ from certbot_apache._internal.tests import util class BasicParserTest(util.ParserTest): """Apache Parser Test.""" - def setUp(self): # pylint: disable=arguments-differ - super().setUp() - - def tearDown(self): - shutil.rmtree(self.temp_dir) - shutil.rmtree(self.config_dir) - shutil.rmtree(self.work_dir) - def test_bad_parse(self): self.parser.parse_file(os.path.join(self.parser.root, "conf-available", "bad_conf_file.conf")) @@ -341,13 +332,6 @@ class BasicParserTest(util.ParserTest): class ParserInitTest(util.ApacheTest): - def setUp(self): # pylint: disable=arguments-differ - super().setUp() - - def tearDown(self): - shutil.rmtree(self.temp_dir) - shutil.rmtree(self.config_dir) - shutil.rmtree(self.work_dir) @mock.patch("certbot_apache._internal.parser.init_augeas") def test_prepare_no_augeas(self, mock_init_augeas): diff --git a/certbot-apache/src/certbot_apache/_internal/tests/util.py b/certbot-apache/src/certbot_apache/_internal/tests/util.py index b13e1afd7..faef22bdf 100644 --- a/certbot-apache/src/certbot_apache/_internal/tests/util.py +++ b/certbot-apache/src/certbot_apache/_internal/tests/util.py @@ -7,6 +7,7 @@ from unittest import mock import augeas import josepy as jose +from certbot import util from certbot.compat import os from certbot.plugins import common from certbot.tests import util as test_util @@ -53,6 +54,7 @@ class ApacheTest(unittest.TestCase): os.symlink(target, vhost) def tearDown(self) -> None: + util._release_locks() shutil.rmtree(self.temp_dir) shutil.rmtree(self.config_dir) shutil.rmtree(self.work_dir)