[Python-checkins] [3.6] bpo-33978: Close existing handlers before logging (re-)configuration. (GH-8008). (GH-8045)

[Vinay Sajip]

https://github.com/python/cpython/commit/b6c1989168efeb8b6320bec958e7e339270ac0ce
commit: b6c1989168efeb8b6320bec958e7e339270ac0ce
branch: 3.6
author: Xtreak <tirkarthi at users.noreply.github.com>
committer: Vinay Sajip <vinay_sajip at yahoo.co.uk>
date: 2018-07-02T10:35:52+01:00
summary:

[3.6] bpo-33978: Close existing handlers before logging (re-)configuration. (GH-8008). (GH-8045)

(cherry picked from commit 087570af6d5d39b51bdd5e660a53903960e58678)

Co-authored-by: Xtreak <tirkarthi at users.noreply.github.com>

files:
A Misc/NEWS.d/next/Library/2018-06-29-12-23-34.bpo-33978.y4csIw.rst
M Lib/logging/config.py
M Lib/test/test_logging.py

diff --git a/Lib/logging/config.py b/Lib/logging/config.py
index 8a99923bf31e..cabddc58cf05 100644
--- a/Lib/logging/config.py
+++ b/Lib/logging/config.py
@@ -78,8 +78,8 @@ def fileConfig(fname, defaults=None, disable_existing_loggers=True):
     # critical section
     logging._acquireLock()
     try:
-        logging._handlers.clear()
-        del logging._handlerList[:]
+        _clearExistingHandlers()
+
         # Handlers add themselves to logging._handlers
         handlers = _install_handlers(cp, formatters)
         _install_loggers(cp, handlers, disable_existing_loggers)
@@ -268,6 +268,14 @@ def _install_loggers(cp, handlers, disable_existing):
     #        logger.disabled = 1
     _handle_existing_loggers(existing, child_loggers, disable_existing)
 
+
+def _clearExistingHandlers():
+    """Clear and close existing handlers"""
+    logging._handlers.clear()
+    logging.shutdown(logging._handlerList[:])
+    del logging._handlerList[:]
+
+
 IDENTIFIER = re.compile('^[a-z_][a-z0-9_]*$', re.I)
 
 
@@ -527,8 +535,7 @@ def configure(self):
             else:
                 disable_existing = config.pop('disable_existing_loggers', True)
 
-                logging._handlers.clear()
-                del logging._handlerList[:]
+                _clearExistingHandlers()
 
                 # Do formatters first - they don't refer to anything else
                 formatters = config.get('formatters', EMPTY_DICT)
diff --git a/Lib/test/test_logging.py b/Lib/test/test_logging.py
index d341ef8779bd..36d88ac63781 100644
--- a/Lib/test/test_logging.py
+++ b/Lib/test/test_logging.py
@@ -1076,6 +1076,7 @@ class ConfigFileTest(BaseTest):
 
     """Reading logging config from a .ini-style config file."""
 
+    check_no_resource_warning = support.check_no_resource_warning
     expected_log_pat = r"^(\w+) \+\+ (\w+)$"
 
     # config0 is a standard configuration.
@@ -1284,6 +1285,27 @@ class ConfigFileTest(BaseTest):
     datefmt=
     """
 
+    # config 8, check for resource warning
+    config8 = r"""
+    [loggers]
+    keys=root
+
+    [handlers]
+    keys=file
+
+    [formatters]
+    keys=
+
+    [logger_root]
+    level=DEBUG
+    handlers=file
+
+    [handler_file]
+    class=FileHandler
+    level=DEBUG
+    args=("{tempfile}",)
+    """
+
     disable_test = """
     [loggers]
     keys=root
@@ -1429,6 +1451,29 @@ def test_config7_ok(self):
             # Original logger output is empty.
             self.assert_log_lines([])
 
+    def test_config8_ok(self):
+
+        def cleanup(h1, fn):
+            h1.close()
+            os.remove(fn)
+
+        with self.check_no_resource_warning():
+            fd, fn = tempfile.mkstemp(".log", "test_logging-X-")
+            os.close(fd)
+
+            # Replace single backslash with double backslash in windows
+            # to avoid unicode error during string formatting
+            if os.name == "nt":
+                fn = fn.replace("\\", "\\\\")
+
+            config8 = self.config8.format(tempfile=fn)
+
+            self.apply_config(config8)
+            self.apply_config(config8)
+
+        handler = logging.root.handlers[0]
+        self.addCleanup(cleanup, handler, fn)
+
     def test_logger_disabling(self):
         self.apply_config(self.disable_test)
         logger = logging.getLogger('some_pristine_logger')
@@ -1977,6 +2022,7 @@ class ConfigDictTest(BaseTest):
 
     """Reading logging config from a dictionary."""
 
+    check_no_resource_warning = support.check_no_resource_warning
     expected_log_pat = r"^(\w+) \+\+ (\w+)$"
 
     # config0 is a standard configuration.
@@ -2851,6 +2897,35 @@ def test_config14_ok(self):
             logging.warning('Exclamation')
             self.assertTrue(output.getvalue().endswith('Exclamation!\n'))
 
+    def test_config15_ok(self):
+
+        def cleanup(h1, fn):
+            h1.close()
+            os.remove(fn)
+
+        with self.check_no_resource_warning():
+            fd, fn = tempfile.mkstemp(".log", "test_logging-X-")
+            os.close(fd)
+
+            config = {
+                "version": 1,
+                "handlers": {
+                    "file": {
+                        "class": "logging.FileHandler",
+                        "filename": fn
+                    }
+                },
+                "root": {
+                    "handlers": ["file"]
+                }
+            }
+
+            self.apply_config(config)
+            self.apply_config(config)
+
+        handler = logging.root.handlers[0]
+        self.addCleanup(cleanup, handler, fn)
+
     @unittest.skipUnless(threading, 'listen() needs threading to work')
     def setup_via_listener(self, text, verify=None):
         text = text.encode("utf-8")
diff --git a/Misc/NEWS.d/next/Library/2018-06-29-12-23-34.bpo-33978.y4csIw.rst b/Misc/NEWS.d/next/Library/2018-06-29-12-23-34.bpo-33978.y4csIw.rst
new file mode 100644
index 000000000000..12638957ee38
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2018-06-29-12-23-34.bpo-33978.y4csIw.rst
@@ -0,0 +1,2 @@
+Closed existing logging handlers before reconfiguration via fileConfig
+and dictConfig. Patch by Karthikeyan Singaravelan.