From ad3ac5c47c3107de274ecf2e5e5dd8b83ddacb00 Mon Sep 17 00:00:00 2001
From: Ben Finney <ben+python@benfinney.id.au>
Date: Oct 18 2017 17:19:40 +0000
Subject: [PATCH 1/13] Document changes made in this branch.


---

diff --git a/ChangeLog b/ChangeLog
index 0aa5fdf..89b4b52 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -26,6 +26,12 @@ Changes:
 * The test suite now relies on the test discovery feature in
   ‘unittest’. This feature is in Python version 2.7 and later.
 
+* Improve performance of `daemon.close_all_open_files`.
+
+  Thanks to Darek Działak for the implementation.
+
+  Closes: Pagure #10.
+
 
 Version 2.1.2
 =============
diff --git a/doc/CREDITS b/doc/CREDITS
index 434457a..95aad47 100644
--- a/doc/CREDITS
+++ b/doc/CREDITS
@@ -32,6 +32,7 @@ Additional contributors
 
 People who have also contributed substantial improvements:
 
+* Darek Działak
 * Malcolm Purvis
 
 

From ebf99a0feacbb83844cc442a296c6c9c491ad03f Mon Sep 17 00:00:00 2001
From: Ben Finney <ben+python@benfinney.id.au>
Date: Oct 18 2017 17:25:17 +0000
Subject: [PATCH 2/13] Extract a separate function to close each file descriptor in turn.


---

diff --git a/daemon/daemon.py b/daemon/daemon.py
index 809f808..e420ed5 100644
--- a/daemon/daemon.py
+++ b/daemon/daemon.py
@@ -848,8 +848,8 @@ def get_maximum_file_descriptors():
     return result
 
 
-def close_all_open_files(exclude=None):
-    """ Close all open file descriptors.
+def _close_each_open_file_descriptor(exclude):
+    """ Close each open file descriptor.
 
         :param exclude: Collection of file descriptors to skip when closing
             files.
@@ -858,15 +858,28 @@ def close_all_open_files(exclude=None):
         Closes every file descriptor (if open) of this process. If
         specified, `exclude` is a set of file descriptors to *not*
         close.
-
         """
-    if exclude is None:
-        exclude = set()
     maxfd = get_maximum_file_descriptors()
     for fd in reversed(range(maxfd)):
         if fd not in exclude:
             close_file_descriptor_if_open(fd)
 
+
+def close_all_open_files(exclude=None):
+    """ Close all open file descriptors.
+
+        :param exclude: Collection of file descriptors to skip when closing
+            files.
+        :return: ``None``.
+
+        Closes every file descriptor (if open) of this process. If
+        specified, `exclude` is a set of file descriptors to *not*
+        close.
+        """
+    if exclude is None:
+        exclude = set()
+    _close_each_open_file_descriptor(exclude=exclude)
+
 
 def redirect_stream(system_stream, target_stream):
     """ Redirect a system stream to a specified file.
diff --git a/test/test_daemon.py b/test/test_daemon.py
index 6e6b888..33afe1f 100644
--- a/test/test_daemon.py
+++ b/test/test_daemon.py
@@ -1366,43 +1366,62 @@ class get_maximum_file_descriptors_TestCase(scaffold.TestCase):
 def fake_get_maximum_file_descriptors():
     return fake_default_maxfd
 
+
 @mock.patch.object(resource, "RLIMIT_NOFILE", new=fake_RLIMIT_NOFILE)
 @mock.patch.object(resource, "RLIM_INFINITY", new=fake_RLIM_INFINITY)
 @mock.patch.object(
         resource, "getrlimit",
         new=fake_getrlimit_nofile_soft_infinity)
-@mock.patch.object(
-        daemon.daemon, "get_maximum_file_descriptors",
-        new=fake_get_maximum_file_descriptors)
 @mock.patch.object(daemon.daemon, "close_file_descriptor_if_open")
-class close_all_open_files_TestCase(scaffold.TestCase):
-    """ Test cases for close_all_open_files function. """
+class _close_each_open_file_descriptor_TestCase(
+        scaffold.TestCaseWithScenarios):
+    """ Test cases for function `_close_each_open_file_descriptor`. """
+
+    fake_maxfd = 10
+
+    scenarios = [
+            ('exclude-empty', {
+                'test_kwargs': dict(
+                    exclude=set(),
+                    ),
+                'expected_file_descriptors': set(range(fake_maxfd)),
+                }),
+            ('exclude-two', {
+                'test_kwargs': dict(
+                    exclude={3, 7},
+                    ),
+                'expected_file_descriptors': {0, 1, 2, 4, 5, 6, 8, 9},
+                }),
+            ]
 
-    def test_requests_all_open_files_to_close(
+    def test_requests_close_of_expected_file_descriptors(
             self, mock_func_close_file_descriptor_if_open):
-        """ Should request close of all open files. """
-        expected_file_descriptors = range(fake_default_maxfd)
+        """ Should request close of each expected file descriptor. """
+        with mock.patch.object(
+                daemon.daemon, "get_maximum_file_descriptors",
+                return_value=self.fake_maxfd):
+            daemon.daemon._close_each_open_file_descriptor(**self.test_kwargs)
         expected_calls = [
-                mock.call(fd) for fd in expected_file_descriptors]
-        daemon.daemon.close_all_open_files()
-        mock_func_close_file_descriptor_if_open.assert_has_calls(
-                expected_calls, any_order=True)
+                mock.call(fd) for fd in self.expected_file_descriptors]
+        self.assertEqual(
+                sorted(mock_func_close_file_descriptor_if_open.mock_calls),
+                sorted(expected_calls))
 
-    def test_requests_all_but_excluded_files_to_close(
-            self, mock_func_close_file_descriptor_if_open):
-        """ Should request close of all open files but those excluded. """
+
+@mock.patch.object(daemon.daemon, "_close_each_open_file_descriptor")
+class close_all_open_files_TestCase(scaffold.TestCase):
+    """ Test cases for function `close_all_open_files`. """
+
+    def test_closes_each_open_file_descriptor(
+            self, mock_func_close_each_open_file_descriptor):
+        """ Should close each file descriptor that is open. """
         test_exclude = set([3, 7])
-        args = dict(
+        test_kwargs = dict(
                 exclude=test_exclude,
                 )
-        expected_file_descriptors = set(
-                fd for fd in range(fake_default_maxfd)
-                if fd not in test_exclude)
-        expected_calls = [
-                mock.call(fd) for fd in expected_file_descriptors]
-        daemon.daemon.close_all_open_files(**args)
-        mock_func_close_file_descriptor_if_open.assert_has_calls(
-                expected_calls, any_order=True)
+        daemon.daemon.close_all_open_files(**test_kwargs)
+        mock_func_close_each_open_file_descriptor.assert_called_with(
+                exclude=test_exclude)
 
 
 class detach_process_context_TestCase(scaffold.TestCase):

From 0443451bacb68d378c680630029eac5963b26461 Mon Sep 17 00:00:00 2001
From: Ben Finney <ben+python@benfinney.id.au>
Date: Oct 18 2017 17:25:19 +0000
Subject: [PATCH 3/13] Implement a helper function to close all non-standard files.


---

diff --git a/daemon/daemon.py b/daemon/daemon.py
index e420ed5..310afd6 100644
--- a/daemon/daemon.py
+++ b/daemon/daemon.py
@@ -865,6 +865,19 @@ def _close_each_open_file_descriptor(exclude):
             close_file_descriptor_if_open(fd)
 
 
+def _close_all_nonstandard_file_descriptors():
+    """ Close file descriptors of all non-standard files.
+
+        :return: ``None``.
+
+        Closes every file descriptor of non-standard files. Standard
+        files are `sys.stdin`, `sys.stdout`, `sys.stderr`.
+        """
+    fd_min = 3
+    fd_max = get_maximum_file_descriptors()
+    os.closerange(fd_min, fd_max)
+
+
 def close_all_open_files(exclude=None):
     """ Close all open file descriptors.
 
diff --git a/test/test_daemon.py b/test/test_daemon.py
index 33afe1f..1812560 100644
--- a/test/test_daemon.py
+++ b/test/test_daemon.py
@@ -1408,6 +1408,23 @@ class _close_each_open_file_descriptor_TestCase(
                 sorted(expected_calls))
 
 
+@mock.patch.object(
+        daemon.daemon, "get_maximum_file_descriptors",
+        new=fake_get_maximum_file_descriptors)
+@mock.patch.object(os, "closerange")
+class _close_all_nonstandard_file_descriptors_TestCase(scaffold.TestCase):
+    """ Test cases for function `_close_all_nonstandard_file_descriptors`. """
+
+    def test_requests_all_open_files_to_close(
+            self, mock_func_os_closerange):
+        """ Should request close of all file descriptors in range. """
+        expected_fd_min = 3
+        expected_fd_max = daemon.daemon.get_maximum_file_descriptors()
+        expected_args = (expected_fd_min, expected_fd_max)
+        daemon.daemon._close_all_nonstandard_file_descriptors()
+        mock_func_os_closerange.assert_called_with(*expected_args)
+
+
 @mock.patch.object(daemon.daemon, "_close_each_open_file_descriptor")
 class close_all_open_files_TestCase(scaffold.TestCase):
     """ Test cases for function `close_all_open_files`. """

From d9efec157dbe86de8556e94d25de539a4948fc8f Mon Sep 17 00:00:00 2001
From: Ben Finney <ben+python@benfinney.id.au>
Date: Oct 18 2017 17:25:19 +0000
Subject: [PATCH 4/13] Close all non-standard files quickly, when `exclude` is empty or None.


---

diff --git a/daemon/daemon.py b/daemon/daemon.py
index 310afd6..be4dbe4 100644
--- a/daemon/daemon.py
+++ b/daemon/daemon.py
@@ -890,8 +890,9 @@ def close_all_open_files(exclude=None):
         close.
         """
     if exclude is None:
-        exclude = set()
-    _close_each_open_file_descriptor(exclude=exclude)
+        _close_all_nonstandard_file_descriptors()
+    else:
+        _close_each_open_file_descriptor(exclude=exclude)
 
 
 def redirect_stream(system_stream, target_stream):
diff --git a/test/test_daemon.py b/test/test_daemon.py
index 1812560..4b842c2 100644
--- a/test/test_daemon.py
+++ b/test/test_daemon.py
@@ -1425,20 +1425,60 @@ class _close_all_nonstandard_file_descriptors_TestCase(scaffold.TestCase):
         mock_func_os_closerange.assert_called_with(*expected_args)
 
 
-@mock.patch.object(daemon.daemon, "_close_each_open_file_descriptor")
 class close_all_open_files_TestCase(scaffold.TestCase):
     """ Test cases for function `close_all_open_files`. """
 
-    def test_closes_each_open_file_descriptor(
-            self, mock_func_close_each_open_file_descriptor):
-        """ Should close each file descriptor that is open. """
+    def setUp(self):
+        """ Set up test fixtures. """
+        super(close_all_open_files_TestCase, self).setUp()
+
+        self.patch_close_helpers()
+
+    def patch_close_helpers(self):
+        """ Patch close helper functions for this test case. """
+        self.mock_close_helpers = mock.MagicMock()
+
+        for func_qualname in [
+                "daemon.daemon._close_each_open_file_descriptor",
+                "daemon.daemon._close_all_nonstandard_file_descriptors",
+                ]:
+            func_patcher = mock.patch(func_qualname)
+            mock_func = func_patcher.start()
+            self.addCleanup(func_patcher.stop)
+            self.mock_close_helpers.attach_mock(mock_func, func_qualname)
+
+    def test_closes_each_open_file_descriptor_when_exclude(self):
+        """ Should close each open file, when `exclude` specified. """
         test_exclude = set([3, 7])
         test_kwargs = dict(
                 exclude=test_exclude,
                 )
         daemon.daemon.close_all_open_files(**test_kwargs)
-        mock_func_close_each_open_file_descriptor.assert_called_with(
-                exclude=test_exclude)
+        self.mock_close_helpers.assert_has_calls([
+                mock.call.daemon.daemon._close_each_open_file_descriptor(
+                    exclude=test_exclude),
+                ])
+
+    def test_closes_all_file_descriptors_when_exclude_empty(self):
+        """ Should close all files, when `exclude` is empty. """
+        test_exclude = set()
+        test_kwargs = dict(
+                exclude=test_exclude,
+                )
+        daemon.daemon.close_all_open_files(**test_kwargs)
+        self.mock_close_helpers.assert_has_calls([
+                mock.call.daemon.daemon._close_each_open_file_descriptor(
+                    exclude=test_exclude),
+                ])
+
+    def test_closes_all_nonstandard_file_descriptors_when_no_exclude(self):
+        """ Should close all non-standard files, when no `exclude`. """
+        test_kwargs = dict()
+        daemon.daemon.close_all_open_files(**test_kwargs)
+        self.mock_close_helpers.assert_has_calls([
+                mock.call.daemon.daemon._close_all_nonstandard_file_descriptors(
+                    ),
+                ])
 
 
 class detach_process_context_TestCase(scaffold.TestCase):

From b2e2d3fa9d5513b18173a620c1a609517f14671d Mon Sep 17 00:00:00 2001
From: Ben Finney <ben+python@benfinney.id.au>
Date: Oct 18 2017 17:25:19 +0000
Subject: [PATCH 5/13] Implement a helper function to get file descriptors of standard streams.


---

diff --git a/daemon/daemon.py b/daemon/daemon.py
index be4dbe4..2ba6cd4 100644
--- a/daemon/daemon.py
+++ b/daemon/daemon.py
@@ -524,6 +524,37 @@ class DaemonContext:
         return signal_handler_map
 
 
+def get_stream_file_descriptors(
+        stdin=sys.stdin,
+        stdout=sys.stdout,
+        stderr=sys.stderr,
+        ):
+    """ Get the set of file descriptors for the process streams.
+
+        :stdin: The input stream for the process (default:
+            `sys.stdin`).
+        :stdout: The ouput stream for the process (default:
+            `sys.stdout`).
+        :stderr: The diagnostic stream for the process (default:
+            `sys.stderr`).
+        :return: A `set` of each file descriptor (integer) for the
+            streams.
+
+        The standard streams are the files `sys.stdin`, `sys.stdout`,
+        `sys.stderr`.
+
+        Streams might in some circumstances be non-file objects.
+        Include in the result only those streams that actually have a
+        file descriptor (as returned by the `fileno` method).
+        """
+    file_descriptors = set(
+            fd for fd in set(
+                _get_file_descriptor(stream)
+                for stream in {stdin, stdout, stderr})
+            if fd is not None)
+    return file_descriptors
+
+
 def _get_file_descriptor(obj):
     """ Get the file descriptor, if the object has one.
 
@@ -534,7 +565,6 @@ def _get_file_descriptor(obj):
         The object may be a non-file object. It may also be a
         file-like object with no support for a file descriptor. In
         either case, return ``None``.
-
         """
     file_descriptor = None
     if hasattr(obj, 'fileno'):
diff --git a/test/test_daemon.py b/test/test_daemon.py
index 4b842c2..857afa8 100644
--- a/test/test_daemon.py
+++ b/test/test_daemon.py
@@ -21,6 +21,7 @@ import functools
 import io
 import os
 import pwd
+import random
 import resource
 import signal
 import socket
@@ -1230,6 +1231,69 @@ class prevent_core_dump_TestCase(scaffold.TestCase):
         self.assertEqual(test_error, exc.__cause__)
 
 
+class get_stream_file_descriptors_TestCase(scaffold.TestCase):
+    """ Test cases for function `get_stream_file_descriptors`. """
+
+    fake_maxfd = 1000
+
+    def setUp(self):
+        """ Set up fixtures for this test case. """
+        super(get_stream_file_descriptors_TestCase, self).setUp()
+
+        self.patch_get_maximum_file_descriptors()
+        self.patch_standard_streams_fileno()
+
+    def patch_get_maximum_file_descriptors(self):
+        """ Patch the function `get_maximum_file_descriptors`. """
+        func_patcher = mock.patch.object(
+                daemon.daemon, "get_maximum_file_descriptors",
+                return_value=self.fake_maxfd)
+        self.mock_get_maximum_file_descriptors = func_patcher.start()
+        self.addCleanup(func_patcher.stop)
+
+    def patch_standard_streams_fileno(self):
+        """ Patch the method `fileno` of standard streams. """
+        available_fileno_results = list(range(0, (self.fake_maxfd + 1)))
+        random.shuffle(available_fileno_results)
+        for stream_name in ["stdin", "stdout", "stderr"]:
+            fake_fileno = available_fileno_results.pop()
+            stream = getattr(sys, stream_name)
+            func_patcher = mock.patch.object(
+                    stream, "fileno",
+                    return_value=fake_fileno)
+            func_patcher.start()
+            self.addCleanup(func_patcher.stop)
+
+    def test_returns_standard_stream_file_descriptors(self):
+        """ Should return the file descriptors of all standard streams. """
+        result = daemon.daemon.get_stream_file_descriptors()
+        expected_fds = set(
+            stream.fileno() for stream in {sys.stdin, sys.stdout, sys.stderr})
+        self.assertEqual(result, expected_fds)
+
+    def test_returns_specified_stream_file_descriptors(self):
+        """ Should return the file descriptors of specified streams. """
+        fake_streams = dict(
+                stdin=FakeFileDescriptorStringIO(),
+                stdout=FakeFileDescriptorStringIO(),
+                stderr=FakeFileDescriptorStringIO(),
+                )
+        test_kwargs = dict(**fake_streams)
+        result = daemon.daemon.get_stream_file_descriptors(
+                **test_kwargs)
+        expected_fds = set(
+            stream.fileno() for stream in fake_streams.values())
+        self.assertEqual(result, expected_fds)
+
+    def test_omits_stream_if_stream_has_no_fileno(self):
+        """ Should omit a stream that has no `fileno` method. """
+        with mock.patch.object(sys.stdin, "fileno", return_value=None):
+            result = daemon.daemon.get_stream_file_descriptors()
+        expected_fds = set(
+            stream.fileno() for stream in [sys.stdout, sys.stderr])
+        self.assertEqual(result, expected_fds)
+
+
 @mock.patch.object(os, "close")
 class close_file_descriptor_if_open_TestCase(scaffold.TestCase):
     """ Test cases for close_file_descriptor_if_open function. """

From f4d6d0cc00dec6ca38a3e5de21b76c1bad1c05f2 Mon Sep 17 00:00:00 2001
From: Ben Finney <ben+python@benfinney.id.au>
Date: Oct 18 2017 17:25:19 +0000
Subject: [PATCH 6/13] Use a set for an un-ordered collection of streams.


---

diff --git a/daemon/daemon.py b/daemon/daemon.py
index 2ba6cd4..fe69ad0 100644
--- a/daemon/daemon.py
+++ b/daemon/daemon.py
@@ -458,20 +458,18 @@ class DaemonContext:
             items in `files_preserve`, and also each of `stdin`,
             `stdout`, and `stderr`. For each item:
 
-            * If the item is ``None``, it is omitted from the return
-              set.
+            * If the item is ``None``, omit it from the return set.
 
-            * If the item's ``fileno()`` method returns a value, that
-              value is in the return set.
-
-            * Otherwise, the item is in the return set verbatim.
+            * If the item's `fileno` method returns a value, include
+              that value in the return set.
 
+            * Otherwise, include the item verbatim in the return set.
             """
         files_preserve = self.files_preserve
         if files_preserve is None:
             files_preserve = []
         files_preserve.extend(
-                item for item in [self.stdin, self.stdout, self.stderr]
+                item for item in {self.stdin, self.stdout, self.stderr}
                 if hasattr(item, 'fileno'))
 
         exclude_descriptors = set()

From 89d0a29b8e02b1a1edf21daf30145ec38cfab2b6 Mon Sep 17 00:00:00 2001
From: Ben Finney <ben+python@benfinney.id.au>
Date: Oct 18 2017 17:37:18 +0000
Subject: [PATCH 7/13] Implement a helper function to get candidate open file descriptors.


---

diff --git a/daemon/daemon.py b/daemon/daemon.py
index fe69ad0..ee8ecfa 100644
--- a/daemon/daemon.py
+++ b/daemon/daemon.py
@@ -876,6 +876,27 @@ def get_maximum_file_descriptors():
     return result
 
 
+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.
+        """
+    maxfd = get_maximum_file_descriptors()
+    candidates = set(range(0, maxfd)).difference(exclude)
+    return candidates
+
+
 def _close_each_open_file_descriptor(exclude):
     """ Close each open file descriptor.
 
diff --git a/test/test_daemon.py b/test/test_daemon.py
index 857afa8..ca1888d 100644
--- a/test/test_daemon.py
+++ b/test/test_daemon.py
@@ -1431,6 +1431,43 @@ def fake_get_maximum_file_descriptors():
     return fake_default_maxfd
 
 
+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 mock.patch.object(
+                daemon.daemon, "get_maximum_file_descriptors",
+                return_value=self.fake_maxfd):
+            result = daemon.daemon._get_candidate_file_descriptors(
+                    **self.test_kwargs)
+        self.assertEqual(result, self.expected_result)
+
+
 @mock.patch.object(resource, "RLIMIT_NOFILE", new=fake_RLIMIT_NOFILE)
 @mock.patch.object(resource, "RLIM_INFINITY", new=fake_RLIM_INFINITY)
 @mock.patch.object(

From 0b8e0cb0b6ad0675cb9b86e089c8528ca010e82a Mon Sep 17 00:00:00 2001
From: Ben Finney <ben+python@benfinney.id.au>
Date: Oct 18 2017 17:38:04 +0000
Subject: [PATCH 8/13] Iterate over candidate file descriptors to close them.


---

diff --git a/daemon/daemon.py b/daemon/daemon.py
index ee8ecfa..b943028 100644
--- a/daemon/daemon.py
+++ b/daemon/daemon.py
@@ -908,10 +908,9 @@ def _close_each_open_file_descriptor(exclude):
         specified, `exclude` is a set of file descriptors to *not*
         close.
         """
-    maxfd = get_maximum_file_descriptors()
-    for fd in reversed(range(maxfd)):
-        if fd not in exclude:
-            close_file_descriptor_if_open(fd)
+    candidate_fds = _get_candidate_file_descriptors(exclude)
+    for fd in reversed(sorted(list(candidate_fds))):
+        close_file_descriptor_if_open(fd)
 
 
 def _close_all_nonstandard_file_descriptors():

From c47cc91b5d419806db4cb4674823b31f25756442 Mon Sep 17 00:00:00 2001
From: Ben Finney <ben+python@benfinney.id.au>
Date: Oct 18 2017 17:48:26 +0000
Subject: [PATCH 9/13] Implement a helper function to get ranges of candidate file descriptors.


---

diff --git a/daemon/daemon.py b/daemon/daemon.py
index b943028..77251a3 100644
--- a/daemon/daemon.py
+++ b/daemon/daemon.py
@@ -21,6 +21,7 @@
 from __future__ import (absolute_import, unicode_literals)
 
 import atexit
+import collections
 import errno
 import os
 import pwd
@@ -897,6 +898,45 @@ def _get_candidate_file_descriptors(exclude):
     return candidates
 
 
+FileDescriptorRange = collections.namedtuple(
+        'FileDescriptorRange', ['low', 'high'])
+
+
+def _get_candidate_file_descriptor_ranges(exclude):
+    """ Get the collection of candidate file descriptor ranges.
+
+        :param exclude: A collection of file descriptors that should
+            be excluded from the return ranges.
+        :return: The collection (a `set`) of ranges that contain the
+            file descriptors that are candidates for files that may be
+            open in this process.
+
+        Determine the ranges – pairs (`low`, `high`) – of `int` values
+        that are candidate file descriptors.
+
+        A value is a candidate if it could be an open file descriptors
+        in this process, excluding those integers in the `exclude`
+        collection.
+        """
+    candidates_list = sorted(_get_candidate_file_descriptors(exclude))
+    ranges = []
+    this_range = FileDescriptorRange(
+            low=min(candidates_list),
+            high=(min(candidates_list) + 1))
+    for fd in candidates_list[1:]:
+        high = fd + 1
+        if this_range.high == fd:
+            # This file descriptor extends the current range.
+            this_range = this_range._replace(high=high)
+        else:
+            # The previous range has ended at a gap.
+            ranges.append(this_range)
+            # This file descriptor begins a new range.
+            this_range = FileDescriptorRange(low=fd, high=high)
+    ranges.append(this_range)
+    return ranges
+
+
 def _close_each_open_file_descriptor(exclude):
     """ Close each open file descriptor.
 
diff --git a/test/test_daemon.py b/test/test_daemon.py
index ca1888d..a02683e 100644
--- a/test/test_daemon.py
+++ b/test/test_daemon.py
@@ -1468,6 +1468,51 @@ class _get_candidate_file_descriptors_TestCase(scaffold.TestCaseWithScenarios):
         self.assertEqual(result, self.expected_result)
 
 
+class _get_candidate_file_descriptor_ranges_TestCase(
+        scaffold.TestCaseWithScenarios):
+    """ Test cases for function `_get_candidate_file_descriptor_ranges`. """
+
+    scenarios = [
+            ('exclude-three', {
+                'fake_maxfd': 10,
+                'test_kwargs': {
+                    'exclude': {3, 5, 8},
+                    },
+                'expected_result': [
+                    (0, 3),
+                    (4, 5),
+                    (6, 8),
+                    (9, 10),
+                    ],
+                }),
+            ('exclude-highest', {
+                'fake_maxfd': 5,
+                'test_kwargs': {
+                    'exclude': {4},
+                    },
+                'expected_result': [
+                    (0, 4),
+                    ],
+                }),
+            ('exclude-none', {
+                'fake_maxfd': 5,
+                'test_kwargs': {
+                    'exclude': set(),
+                },
+                'expected_result': [(0, 5)],
+                }),
+            ]
+
+    def test_returns_expected_file_descriptors(self):
+        """ Should return the expected set of file descriptors. """
+        with mock.patch.object(
+                daemon.daemon, "get_maximum_file_descriptors",
+                return_value=self.fake_maxfd):
+            result = daemon.daemon._get_candidate_file_descriptor_ranges(
+                    **self.test_kwargs)
+        self.assertEqual(result, self.expected_result)
+
+
 @mock.patch.object(resource, "RLIMIT_NOFILE", new=fake_RLIMIT_NOFILE)
 @mock.patch.object(resource, "RLIM_INFINITY", new=fake_RLIM_INFINITY)
 @mock.patch.object(

From 6daaad826bb4225d37136b00f84e5e9d1cec9b08 Mon Sep 17 00:00:00 2001
From: Ben Finney <ben+python@benfinney.id.au>
Date: Oct 18 2017 18:00:10 +0000
Subject: [PATCH 10/13] Implement a helper function to close file descriptors by range.


---

diff --git a/daemon/daemon.py b/daemon/daemon.py
index 77251a3..1342bd8 100644
--- a/daemon/daemon.py
+++ b/daemon/daemon.py
@@ -966,6 +966,20 @@ def _close_all_nonstandard_file_descriptors():
     os.closerange(fd_min, fd_max)
 
 
+def _close_file_descriptor_ranges(ranges):
+    """ Close file descriptors described by `ranges`.
+
+        :param ranges: A sequence of `FileDescriptorRange` instances,
+            each describing a range of file descriptors to close.
+        :return: ``None``.
+
+        Attempt to close each open file descriptor – starting from
+        `low` and ending before `high` – from each range in `ranges`.
+        """
+    for range in ranges:
+        os.closerange(range.low, range.high)
+
+
 def close_all_open_files(exclude=None):
     """ Close all open file descriptors.
 
diff --git a/test/test_daemon.py b/test/test_daemon.py
index a02683e..4555413 100644
--- a/test/test_daemon.py
+++ b/test/test_daemon.py
@@ -1571,6 +1571,45 @@ class _close_all_nonstandard_file_descriptors_TestCase(scaffold.TestCase):
         mock_func_os_closerange.assert_called_with(*expected_args)
 
 
+@mock.patch.object(os, "closerange")
+class _close_file_descriptor_ranges_TestCase(scaffold.TestCaseWithScenarios):
+    """ Test cases for function `_close_file_descriptor_ranges`. """
+
+    scenarios = [
+            ('ranges-one', {
+                'test_kwargs': {
+                    'ranges': [
+                        daemon.daemon.FileDescriptorRange(0, 10),
+                        ],
+                    },
+                'expected_os_closerange_calls': [
+                    mock.call(0, 10),
+                    ],
+                }),
+            ('ranges-three', {
+                'test_kwargs': {
+                    'ranges': [
+                        daemon.daemon.FileDescriptorRange(5, 10),
+                        daemon.daemon.FileDescriptorRange(0, 3),
+                        daemon.daemon.FileDescriptorRange(15, 20),
+                        ],
+                    },
+                'expected_os_closerange_calls': [
+                    mock.call(5, 10),
+                    mock.call(0, 3),
+                    mock.call(15, 20),
+                    ],
+                }),
+            ]
+
+    def test_calls_os_closerange_with_expected_ranges(
+            self, mock_func_os_closerange):
+        """ Should request close of all file descriptors in range. """
+        daemon.daemon._close_file_descriptor_ranges(**self.test_kwargs)
+        mock_func_os_closerange.assert_has_calls(
+                self.expected_os_closerange_calls)
+
+
 class close_all_open_files_TestCase(scaffold.TestCase):
     """ Test cases for function `close_all_open_files`. """
 

From 425be0d84b836a7c7c1dcf7af92a50bf3e5bf6bd Mon Sep 17 00:00:00 2001
From: Ben Finney <ben+python@benfinney.id.au>
Date: Oct 18 2017 18:00:11 +0000
Subject: [PATCH 11/13] Migrate to `_close_file_descriptor_ranges` helper.


---

diff --git a/daemon/daemon.py b/daemon/daemon.py
index 1342bd8..65cb2bc 100644
--- a/daemon/daemon.py
+++ b/daemon/daemon.py
@@ -948,9 +948,8 @@ def _close_each_open_file_descriptor(exclude):
         specified, `exclude` is a set of file descriptors to *not*
         close.
         """
-    candidate_fds = _get_candidate_file_descriptors(exclude)
-    for fd in reversed(sorted(list(candidate_fds))):
-        close_file_descriptor_if_open(fd)
+    candidate_fd_ranges = _get_candidate_file_descriptor_ranges(exclude)
+    _close_file_descriptor_ranges(candidate_fd_ranges)
 
 
 def _close_all_nonstandard_file_descriptors():
@@ -961,9 +960,9 @@ def _close_all_nonstandard_file_descriptors():
         Closes every file descriptor of non-standard files. Standard
         files are `sys.stdin`, `sys.stdout`, `sys.stderr`.
         """
-    fd_min = 3
-    fd_max = get_maximum_file_descriptors()
-    os.closerange(fd_min, fd_max)
+    exclude = get_stream_file_descriptors()
+    candidate_fd_ranges = _get_candidate_file_descriptor_ranges(exclude)
+    _close_file_descriptor_ranges(candidate_fd_ranges)
 
 
 def _close_file_descriptor_ranges(ranges):
diff --git a/test/test_daemon.py b/test/test_daemon.py
index 4555413..e42fb41 100644
--- a/test/test_daemon.py
+++ b/test/test_daemon.py
@@ -1518,7 +1518,7 @@ class _get_candidate_file_descriptor_ranges_TestCase(
 @mock.patch.object(
         resource, "getrlimit",
         new=fake_getrlimit_nofile_soft_infinity)
-@mock.patch.object(daemon.daemon, "close_file_descriptor_if_open")
+@mock.patch.object(os, "closerange")
 class _close_each_open_file_descriptor_TestCase(
         scaffold.TestCaseWithScenarios):
     """ Test cases for function `_close_each_open_file_descriptor`. """
@@ -1530,27 +1530,34 @@ class _close_each_open_file_descriptor_TestCase(
                 'test_kwargs': dict(
                     exclude=set(),
                     ),
-                'expected_file_descriptors': set(range(fake_maxfd)),
+                'expected_file_descriptor_ranges': [
+                    daemon.daemon.FileDescriptorRange(0, fake_maxfd),
+                    ],
                 }),
             ('exclude-two', {
                 'test_kwargs': dict(
                     exclude={3, 7},
                     ),
-                'expected_file_descriptors': {0, 1, 2, 4, 5, 6, 8, 9},
+                'expected_file_descriptor_ranges': [
+                    daemon.daemon.FileDescriptorRange(0, 3),
+                    daemon.daemon.FileDescriptorRange(4, 7),
+                    daemon.daemon.FileDescriptorRange(8, fake_maxfd),
+                    ],
                 }),
             ]
 
     def test_requests_close_of_expected_file_descriptors(
-            self, mock_func_close_file_descriptor_if_open):
+            self, mock_func_os_closerange):
         """ Should request close of each expected file descriptor. """
         with mock.patch.object(
                 daemon.daemon, "get_maximum_file_descriptors",
                 return_value=self.fake_maxfd):
             daemon.daemon._close_each_open_file_descriptor(**self.test_kwargs)
         expected_calls = [
-                mock.call(fd) for fd in self.expected_file_descriptors]
+                mock.call(range.low, range.high)
+                for range in self.expected_file_descriptor_ranges]
         self.assertEqual(
-                sorted(mock_func_close_file_descriptor_if_open.mock_calls),
+                sorted(mock_func_os_closerange.mock_calls),
                 sorted(expected_calls))
 
 

From 649f78afe83984c9f97cf933709007d6cf8275c2 Mon Sep 17 00:00:00 2001
From: Ben Finney <ben+python@benfinney.id.au>
Date: Oct 18 2017 18:00:11 +0000
Subject: [PATCH 12/13] Migrate to `_get_candidate_file_descriptor_ranges` helper.


---

diff --git a/daemon/daemon.py b/daemon/daemon.py
index 65cb2bc..ae1c1a7 100644
--- a/daemon/daemon.py
+++ b/daemon/daemon.py
@@ -991,9 +991,9 @@ def close_all_open_files(exclude=None):
         close.
         """
     if exclude is None:
-        _close_all_nonstandard_file_descriptors()
-    else:
-        _close_each_open_file_descriptor(exclude=exclude)
+        exclude = set()
+    fd_ranges = _get_candidate_file_descriptor_ranges(exclude=exclude)
+    _close_file_descriptor_ranges(ranges=fd_ranges)
 
 
 def redirect_stream(system_stream, target_stream):
diff --git a/test/test_daemon.py b/test/test_daemon.py
index e42fb41..969ef21 100644
--- a/test/test_daemon.py
+++ b/test/test_daemon.py
@@ -1620,24 +1620,28 @@ class _close_file_descriptor_ranges_TestCase(scaffold.TestCaseWithScenarios):
 class close_all_open_files_TestCase(scaffold.TestCase):
     """ Test cases for function `close_all_open_files`. """
 
+    fake_maxfd = 10
+
     def setUp(self):
         """ Set up test fixtures. """
         super(close_all_open_files_TestCase, self).setUp()
 
-        self.patch_close_helpers()
+        self.patch_get_maximum_file_descriptors(self.fake_maxfd)
+        self.patch_os_closerange()
 
-    def patch_close_helpers(self):
-        """ Patch close helper functions for this test case. """
-        self.mock_close_helpers = mock.MagicMock()
+    def patch_get_maximum_file_descriptors(self, fake_maxfd):
+        """ Patch `get_maximum_file_descriptors` for this test case. """
+        func_patcher = mock.patch.object(
+            daemon.daemon, "get_maximum_file_descriptors",
+            return_value=fake_maxfd)
+        self.mock_func_get_maximum_file_descriptors = func_patcher.start()
+        self.addCleanup(func_patcher.stop)
 
-        for func_qualname in [
-                "daemon.daemon._close_each_open_file_descriptor",
-                "daemon.daemon._close_all_nonstandard_file_descriptors",
-                ]:
-            func_patcher = mock.patch(func_qualname)
-            mock_func = func_patcher.start()
-            self.addCleanup(func_patcher.stop)
-            self.mock_close_helpers.attach_mock(mock_func, func_qualname)
+    def patch_os_closerange(self):
+        """ Patch `os.closerange` function for this test case. """
+        func_patcher = mock.patch.object(os, "closerange")
+        self.mock_func_os_closerange = func_patcher.start()
+        self.addCleanup(func_patcher.stop)
 
     def test_closes_each_open_file_descriptor_when_exclude(self):
         """ Should close each open file, when `exclude` specified. """
@@ -1646,10 +1650,13 @@ class close_all_open_files_TestCase(scaffold.TestCase):
                 exclude=test_exclude,
                 )
         daemon.daemon.close_all_open_files(**test_kwargs)
-        self.mock_close_helpers.assert_has_calls([
-                mock.call.daemon.daemon._close_each_open_file_descriptor(
-                    exclude=test_exclude),
-                ])
+        expected_os_closerange_calls = [
+                mock.call(0, 3),
+                mock.call(4, 7),
+                mock.call(8, self.fake_maxfd),
+                ]
+        self.mock_func_os_closerange.assert_has_calls(
+                expected_os_closerange_calls, any_order=True)
 
     def test_closes_all_file_descriptors_when_exclude_empty(self):
         """ Should close all files, when `exclude` is empty. """
@@ -1658,19 +1665,21 @@ class close_all_open_files_TestCase(scaffold.TestCase):
                 exclude=test_exclude,
                 )
         daemon.daemon.close_all_open_files(**test_kwargs)
-        self.mock_close_helpers.assert_has_calls([
-                mock.call.daemon.daemon._close_each_open_file_descriptor(
-                    exclude=test_exclude),
-                ])
+        expected_os_closerange_calls = [
+                mock.call(0, self.fake_maxfd),
+                ]
+        self.mock_func_os_closerange.assert_has_calls(
+                expected_os_closerange_calls, any_order=True)
 
-    def test_closes_all_nonstandard_file_descriptors_when_no_exclude(self):
-        """ Should close all non-standard files, when no `exclude`. """
+    def test_closes_all_file_descriptors_when_no_exclude(self):
+        """ Should close all files, when no `exclude`. """
         test_kwargs = dict()
         daemon.daemon.close_all_open_files(**test_kwargs)
-        self.mock_close_helpers.assert_has_calls([
-                mock.call.daemon.daemon._close_all_nonstandard_file_descriptors(
-                    ),
-                ])
+        expected_os_closerange_calls = [
+                mock.call(0, self.fake_maxfd),
+                ]
+        self.mock_func_os_closerange.assert_has_calls(
+                expected_os_closerange_calls, any_order=True)
 
 
 class detach_process_context_TestCase(scaffold.TestCase):

From 3624ca7da36d8e020c1b212edcc97d77844a06c7 Mon Sep 17 00:00:00 2001
From: Ben Finney <ben+python@benfinney.id.au>
Date: Oct 18 2017 18:00:11 +0000
Subject: [PATCH 13/13] Remove unused private helper functions.


---

diff --git a/daemon/daemon.py b/daemon/daemon.py
index ae1c1a7..729a5ed 100644
--- a/daemon/daemon.py
+++ b/daemon/daemon.py
@@ -937,34 +937,6 @@ def _get_candidate_file_descriptor_ranges(exclude):
     return ranges
 
 
-def _close_each_open_file_descriptor(exclude):
-    """ Close each open file descriptor.
-
-        :param exclude: Collection of file descriptors to skip when closing
-            files.
-        :return: ``None``.
-
-        Closes every file descriptor (if open) of this process. If
-        specified, `exclude` is a set of file descriptors to *not*
-        close.
-        """
-    candidate_fd_ranges = _get_candidate_file_descriptor_ranges(exclude)
-    _close_file_descriptor_ranges(candidate_fd_ranges)
-
-
-def _close_all_nonstandard_file_descriptors():
-    """ Close file descriptors of all non-standard files.
-
-        :return: ``None``.
-
-        Closes every file descriptor of non-standard files. Standard
-        files are `sys.stdin`, `sys.stdout`, `sys.stderr`.
-        """
-    exclude = get_stream_file_descriptors()
-    candidate_fd_ranges = _get_candidate_file_descriptor_ranges(exclude)
-    _close_file_descriptor_ranges(candidate_fd_ranges)
-
-
 def _close_file_descriptor_ranges(ranges):
     """ Close file descriptors described by `ranges`.
 
diff --git a/test/test_daemon.py b/test/test_daemon.py
index 969ef21..91a88aa 100644
--- a/test/test_daemon.py
+++ b/test/test_daemon.py
@@ -1513,71 +1513,6 @@ class _get_candidate_file_descriptor_ranges_TestCase(
         self.assertEqual(result, self.expected_result)
 
 
-@mock.patch.object(resource, "RLIMIT_NOFILE", new=fake_RLIMIT_NOFILE)
-@mock.patch.object(resource, "RLIM_INFINITY", new=fake_RLIM_INFINITY)
-@mock.patch.object(
-        resource, "getrlimit",
-        new=fake_getrlimit_nofile_soft_infinity)
-@mock.patch.object(os, "closerange")
-class _close_each_open_file_descriptor_TestCase(
-        scaffold.TestCaseWithScenarios):
-    """ Test cases for function `_close_each_open_file_descriptor`. """
-
-    fake_maxfd = 10
-
-    scenarios = [
-            ('exclude-empty', {
-                'test_kwargs': dict(
-                    exclude=set(),
-                    ),
-                'expected_file_descriptor_ranges': [
-                    daemon.daemon.FileDescriptorRange(0, fake_maxfd),
-                    ],
-                }),
-            ('exclude-two', {
-                'test_kwargs': dict(
-                    exclude={3, 7},
-                    ),
-                'expected_file_descriptor_ranges': [
-                    daemon.daemon.FileDescriptorRange(0, 3),
-                    daemon.daemon.FileDescriptorRange(4, 7),
-                    daemon.daemon.FileDescriptorRange(8, fake_maxfd),
-                    ],
-                }),
-            ]
-
-    def test_requests_close_of_expected_file_descriptors(
-            self, mock_func_os_closerange):
-        """ Should request close of each expected file descriptor. """
-        with mock.patch.object(
-                daemon.daemon, "get_maximum_file_descriptors",
-                return_value=self.fake_maxfd):
-            daemon.daemon._close_each_open_file_descriptor(**self.test_kwargs)
-        expected_calls = [
-                mock.call(range.low, range.high)
-                for range in self.expected_file_descriptor_ranges]
-        self.assertEqual(
-                sorted(mock_func_os_closerange.mock_calls),
-                sorted(expected_calls))
-
-
-@mock.patch.object(
-        daemon.daemon, "get_maximum_file_descriptors",
-        new=fake_get_maximum_file_descriptors)
-@mock.patch.object(os, "closerange")
-class _close_all_nonstandard_file_descriptors_TestCase(scaffold.TestCase):
-    """ Test cases for function `_close_all_nonstandard_file_descriptors`. """
-
-    def test_requests_all_open_files_to_close(
-            self, mock_func_os_closerange):
-        """ Should request close of all file descriptors in range. """
-        expected_fd_min = 3
-        expected_fd_max = daemon.daemon.get_maximum_file_descriptors()
-        expected_args = (expected_fd_min, expected_fd_max)
-        daemon.daemon._close_all_nonstandard_file_descriptors()
-        mock_func_os_closerange.assert_called_with(*expected_args)
-
-
 @mock.patch.object(os, "closerange")
 class _close_file_descriptor_ranges_TestCase(scaffold.TestCaseWithScenarios):
     """ Test cases for function `_close_file_descriptor_ranges`. """