From 578f7dac2c32e609b1bec106ca5d47c3c72ef6d0 Mon Sep 17 00:00:00 2001 From: Igor Kholopov Date: Nov 28 2022 19:25:47 +0000 Subject: Refactor _get_candidate_file_descriptor_ranges to iterate over excludes rather than all descriptors. --- diff --git a/daemon/daemon.py b/daemon/daemon.py index 7f0f9d6..4141557 100644 --- a/daemon/daemon.py +++ b/daemon/daemon.py @@ -865,7 +865,6 @@ def get_maximum_file_descriptors(): _total_file_descriptor_range = (0, get_maximum_file_descriptors()) -_total_file_descriptor_set = set(range(*_total_file_descriptor_range)) def _validate_fd_values(fds): @@ -884,27 +883,6 @@ def _validate_fd_values(fds): "not an integer file descriptor", value_to_complain_about) -def _get_candidate_file_descriptors(exclude): - """ Get the collection of candidate file descriptors. - - :param exclude: A collection of file descriptors that should - be excluded from the return set. - :return: The collection (a `set`) of file descriptors that are - candidates for files that may be open in this process. - - Determine the set of all `int` values that could be open file - descriptors in this process. A file descriptor is a candidate - if it is within the range (0, `maxfd`), excluding those - integers in the `exclude` collection. - - The `maxfd` value is determined from the standard library - `resource` module. - """ - _validate_fd_values(exclude) - candidates = _total_file_descriptor_set.difference(exclude) - return candidates - - def _get_candidate_file_descriptor_ranges(exclude): """ Get the collection of candidate file descriptor ranges. @@ -921,29 +899,26 @@ def _get_candidate_file_descriptor_ranges(exclude): in this process, excluding those integers in the `exclude` collection. """ - candidates_list = sorted(_get_candidate_file_descriptors(exclude)) + _validate_fd_values(exclude) + exclude = sorted(exclude) + upper_range = _total_file_descriptor_range ranges = [] - def append_range_if_needed(candidate_range): - (low, high) = candidate_range - if (low < high): - # The range is not empty. - ranges.append(candidate_range) - - this_range = ( - (min(candidates_list), (min(candidates_list) + 1)) - if candidates_list else (0, 0)) - for fd in candidates_list[1:]: - high = fd + 1 - if this_range[1] == fd: - # This file descriptor extends the current range. - this_range = (this_range[0], high) - else: - # The previous range has ended at a gap. - append_range_if_needed(this_range) - # This file descriptor begins a new range. - this_range = (fd, high) - append_range_if_needed(this_range) + for fd in exclude: + if fd > upper_range[1]: + # All other fds are outside of total range. + break + if fd < upper_range[0]: + # Underflow, skip. + continue + if fd != upper_range[0]: + ranges.append((upper_range[0], fd)) + upper_range = (fd + 1, upper_range[1]) + + if upper_range[0] < upper_range[1]: + # Append if there is still a valid range left. + ranges.append(upper_range) + return ranges diff --git a/test/test_daemon.py b/test/test_daemon.py index 71f5d83..ad99149 100644 --- a/test/test_daemon.py +++ b/test/test_daemon.py @@ -1474,58 +1474,6 @@ def make_total_file_descriptor_range_patch(testcase, fake_maxfd): return attr_patcher -def make_total_file_descriptor_set_patch(testcase, fake_maxfd): - """ Make a `_total_file_descriptor_set` patch for the `testcase`. - - :param testcase: The `unittest.TestCase` instance to patch. - :param fake_maxfd: The fake maximum file descriptor value. - :return: The `unittest.mock.patch` object. - """ - attr_patcher = unittest.mock.patch.object( - daemon.daemon, "_total_file_descriptor_set", - new=set(range(0, fake_maxfd))) - return attr_patcher - - -class _get_candidate_file_descriptors_TestCase(scaffold.TestCaseWithScenarios): - """ Test cases for function `_get_candidate_file_descriptors`. """ - - scenarios = [ - ('exclude-three', { - 'fake_maxfd': 10, - 'test_kwargs': { - 'exclude': {3, 5, 8}, - }, - 'expected_result': {0, 1, 2, 4, 6, 7, 9}, - }), - ('exclude-one', { - 'fake_maxfd': 5, - 'test_kwargs': { - 'exclude': {4}, - }, - 'expected_result': {0, 1, 2, 3}, - }), - ('exclude-none', { - 'fake_maxfd': 5, - 'test_kwargs': { - 'exclude': set(), - }, - 'expected_result': {0, 1, 2, 3, 4}, - }), - ] - - def test_returns_expected_file_descriptors(self): - """ Should return the expected set of file descriptors. """ - with contextlib.ExitStack() as patch_stack: - patch_stack.enter_context( - make_total_file_descriptor_range_patch(self, self.fake_maxfd)) - patch_stack.enter_context( - make_total_file_descriptor_set_patch(self, self.fake_maxfd)) - result = daemon.daemon._get_candidate_file_descriptors( - **self.test_kwargs) - self.assertEqual(result, self.expected_result) - - class _get_candidate_file_descriptor_ranges_TestCase( scaffold.TestCaseWithScenarios): """ Test cases for function `_get_candidate_file_descriptor_ranges`. """ @@ -1613,6 +1561,16 @@ class _get_candidate_file_descriptor_ranges_TestCase( (0, 5), ], }), + ('exclude-large-maxfd', { + 'fake_maxfd': 0x3FFFFFF0, + 'test_kwargs': { + 'exclude': {4}, + }, + 'expected_result': [ + (0, 4), + (5, 0x3FFFFFF0), + ], + }), ] def test_returns_expected_file_descriptors(self): @@ -1620,8 +1578,6 @@ class _get_candidate_file_descriptor_ranges_TestCase( with contextlib.ExitStack() as patch_stack: patch_stack.enter_context( make_total_file_descriptor_range_patch(self, self.fake_maxfd)) - patch_stack.enter_context( - make_total_file_descriptor_set_patch(self, self.fake_maxfd)) result = daemon.daemon._get_candidate_file_descriptor_ranges( **self.test_kwargs) self.assertEqual(result, self.expected_result) @@ -1725,11 +1681,6 @@ class close_all_open_files_TestCase(scaffold.TestCase): self, fake_maxfd=self.fake_maxfd)) total_file_descriptor_range_patch.start() self.addCleanup(total_file_descriptor_range_patch.stop) - total_file_descriptor_set_patch = ( - make_total_file_descriptor_set_patch( - self, fake_maxfd=self.fake_maxfd)) - total_file_descriptor_set_patch.start() - self.addCleanup(total_file_descriptor_set_patch.stop) self.patch_os_closerange()