BIND 10 trac1828, updated. 7d741f7ea2ddc3ab22e37b12132b7e029e6da573 [1828] Cleanup code by using with

BIND 10 source code commits bind10-changes at lists.isc.org
Thu Jun 14 07:44:12 UTC 2012


The branch, trac1828 has been updated
       via  7d741f7ea2ddc3ab22e37b12132b7e029e6da573 (commit)
       via  d295c9ec3b875e61d4453d69a569ffeabf132a69 (commit)
       via  74dbc765acd11e59d8a144033e9202349a03157f (commit)
       via  9040c49022e84e7b1398e84a14abd54f8d99ec8c (commit)
       via  bbeee5330ca6f6619b371b11ce8e54b705abc4ed (commit)
       via  86b4a931b5857952c7036fb3c043b59d015269b0 (commit)
       via  57449046356fda9a21f3cf2dc1e5faadaaf42d7c (commit)
       via  d96e91d14a55046b741dbc49eaa1b5fac5efc8d0 (commit)
       via  3dd46b79138e68f3054430ae9f37a38a80ba0913 (commit)
       via  1ece2eadd41752ca89bc9c6932942b9c39dc91ac (commit)
      from  933d2b6f17ab8a634f392e94747074d2515fceb0 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 7d741f7ea2ddc3ab22e37b12132b7e029e6da573
Author: Mukund Sivaraman <muks at isc.org>
Date:   Thu Jun 14 13:13:33 2012 +0530

    [1828] Cleanup code by using with

commit d295c9ec3b875e61d4453d69a569ffeabf132a69
Author: Mukund Sivaraman <muks at isc.org>
Date:   Thu Jun 14 13:09:36 2012 +0530

    [1828] Close sockets created by socketpair when done

commit 74dbc765acd11e59d8a144033e9202349a03157f
Author: Mukund Sivaraman <muks at isc.org>
Date:   Thu Jun 14 13:09:14 2012 +0530

    [1828] Don't redirect stderr in tests

commit 9040c49022e84e7b1398e84a14abd54f8d99ec8c
Author: Mukund Sivaraman <muks at isc.org>
Date:   Thu Jun 14 13:02:56 2012 +0530

    [1828] Cleanup code by using with

commit bbeee5330ca6f6619b371b11ce8e54b705abc4ed
Author: Mukund Sivaraman <muks at isc.org>
Date:   Thu Jun 14 12:58:42 2012 +0530

    [1828] Don't redirect stdout in some tests

commit 86b4a931b5857952c7036fb3c043b59d015269b0
Author: Mukund Sivaraman <muks at isc.org>
Date:   Thu Jun 14 12:49:44 2012 +0530

    [1828] Make code safer by using with

commit 57449046356fda9a21f3cf2dc1e5faadaaf42d7c
Author: Mukund Sivaraman <muks at isc.org>
Date:   Thu Jun 14 12:45:17 2012 +0530

    [1828] Make code safer by using with

commit d96e91d14a55046b741dbc49eaa1b5fac5efc8d0
Author: Mukund Sivaraman <muks at isc.org>
Date:   Thu Jun 14 12:35:17 2012 +0530

    [1828] Close MsgQ socket in except block

commit 3dd46b79138e68f3054430ae9f37a38a80ba0913
Author: Mukund Sivaraman <muks at isc.org>
Date:   Thu Jun 14 12:34:36 2012 +0530

    [1828] Cleanup listen socket in DDNSServer.shutdown_cleanup()

commit 1ece2eadd41752ca89bc9c6932942b9c39dc91ac
Author: Mukund Sivaraman <muks at isc.org>
Date:   Thu Jun 14 11:22:45 2012 +0530

    [1828] Make code safer by using with

-----------------------------------------------------------------------

Summary of changes:
 src/bin/bindctl/tests/bindctl_test.py              |   31 +++++++-------
 src/bin/ddns/ddns.py.in                            |    1 +
 src/bin/ddns/tests/ddns_test.py                    |    6 ++-
 src/bin/msgq/msgq.py.in                            |    1 +
 src/bin/msgq/tests/msgq_test.py                    |    1 -
 src/bin/xfrout/tests/xfrout_test.py.in             |   44 ++++++++------------
 src/bin/xfrout/xfrout.py.in                        |   26 ++++++------
 src/bin/zonemgr/tests/zonemgr_test.py              |    6 ---
 src/bin/zonemgr/zonemgr.py.in                      |    2 +
 .../python/isc/bind10/tests/sockcreator_test.py    |   14 +++----
 10 files changed, 62 insertions(+), 70 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/bin/bindctl/tests/bindctl_test.py b/src/bin/bindctl/tests/bindctl_test.py
index 6252ca9..bcfb6c5 100644
--- a/src/bin/bindctl/tests/bindctl_test.py
+++ b/src/bin/bindctl/tests/bindctl_test.py
@@ -425,6 +425,12 @@ class FakeBindCmdInterpreter(bindcmd.BindCmdInterpreter):
 
 class TestBindCmdInterpreter(unittest.TestCase):
 
+    def setUp(self):
+        self.old_stdout = sys.stdout
+
+    def tearDown(self):
+        sys.stdout = self.old_stdout
+
     def _create_invalid_csv_file(self, csvfilename):
         import csv
         csvfile = open(csvfilename, 'w')
@@ -447,20 +453,17 @@ class TestBindCmdInterpreter(unittest.TestCase):
         self.assertEqual(new_csv_dir, custom_cmd.csv_file_dir)
 
     def test_get_saved_user_info(self):
-        old_stdout = sys.stdout
-        sys.stdout = open(os.devnull, 'w')
-        cmd = bindcmd.BindCmdInterpreter()
-        users = cmd._get_saved_user_info('/notexist', 'csv_file.csv')
-        self.assertEqual([], users)
-
-        csvfilename = 'csv_file.csv'
-        self._create_invalid_csv_file(csvfilename)
-        users = cmd._get_saved_user_info('./', csvfilename)
-        self.assertEqual([], users)
-        os.remove(csvfilename)
-        sys.stdout.close()
-        sys.stdout = old_stdout
-
+        with open(os.devnull, 'w') as f:
+            sys.stdout = f
+            cmd = bindcmd.BindCmdInterpreter()
+            users = cmd._get_saved_user_info('/notexist', 'csv_file.csv')
+            self.assertEqual([], users)
+
+            csvfilename = 'csv_file.csv'
+            self._create_invalid_csv_file(csvfilename)
+            users = cmd._get_saved_user_info('./', csvfilename)
+            self.assertEqual([], users)
+            os.remove(csvfilename)
 
 class TestCommandLineOptions(unittest.TestCase):
     def setUp(self):
diff --git a/src/bin/ddns/ddns.py.in b/src/bin/ddns/ddns.py.in
index 22e4e9c..44385ea 100755
--- a/src/bin/ddns/ddns.py.in
+++ b/src/bin/ddns/ddns.py.in
@@ -154,6 +154,7 @@ class DDNSServer:
         this module is stopping.
         '''
         self._cc.send_stopping()
+        self._listen_socket.close()
 
     def accept(self):
         """
diff --git a/src/bin/ddns/tests/ddns_test.py b/src/bin/ddns/tests/ddns_test.py
index 52b8d09..25cf43b 100755
--- a/src/bin/ddns/tests/ddns_test.py
+++ b/src/bin/ddns/tests/ddns_test.py
@@ -37,6 +37,8 @@ class FakeSocket:
         return "fake_unix_socket"
     def accept(self):
         return FakeSocket(self.__fileno + 1)
+    def close(self):
+        return
 
 class FakeSessionReceiver:
     """
@@ -111,6 +113,8 @@ class TestDDNSServer(unittest.TestCase):
         self.__select_answer = None
         self.__select_exception = None
         self.__hook_called = False
+        # Because we overwrite the _listen_socket, close any existing
+        # socket object.
         if self.ddns_server._listen_socket is not None:
             self.ddns_server._listen_socket.close()
         self.ddns_server._listen_socket = FakeSocket(2)
@@ -141,7 +145,7 @@ class TestDDNSServer(unittest.TestCase):
         self.assertIsNone(self.__hook_called)
         # Now make sure the clear_socket really works
         ddns.clear_socket()
-        ddnss._listen_socket.close()
+        ddnss.shutdown_cleanup()
         self.assertFalse(os.path.exists(ddns.SOCKET_FILE))
 
     def test_config_handler(self):
diff --git a/src/bin/msgq/msgq.py.in b/src/bin/msgq/msgq.py.in
index 333ae89..58b1d87 100755
--- a/src/bin/msgq/msgq.py.in
+++ b/src/bin/msgq/msgq.py.in
@@ -177,6 +177,7 @@ class MsgQ:
             # (note this is a catch-all, but we reraise it)
             if os.path.exists(self.socket_file):
                 os.remove(self.socket_file)
+            self.listen_socket.close()
             raise e
 
         if self.poller:
diff --git a/src/bin/msgq/tests/msgq_test.py b/src/bin/msgq/tests/msgq_test.py
index 4059ea9..cc078f4 100644
--- a/src/bin/msgq/tests/msgq_test.py
+++ b/src/bin/msgq/tests/msgq_test.py
@@ -111,7 +111,6 @@ class TestSubscriptionManager(unittest.TestCase):
     def test_open_socket_bad(self):
         msgq = MsgQ("/does/not/exist")
         self.assertRaises(socket.error, msgq.setup)
-        msgq.listen_socket.close()
 
 class SendNonblock(unittest.TestCase):
     """
diff --git a/src/bin/xfrout/tests/xfrout_test.py.in b/src/bin/xfrout/tests/xfrout_test.py.in
index ab463d2..6f38b75 100644
--- a/src/bin/xfrout/tests/xfrout_test.py.in
+++ b/src/bin/xfrout/tests/xfrout_test.py.in
@@ -1196,31 +1196,30 @@ class TestUnixSockServer(unittest.TestCase):
         # We test with UDP, as it can be "connected" without other
         # endpoint.  Note that in the current implementation _guess_remote()
         # unconditionally returns SOCK_STREAM.
-        sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
-        sock.connect(('127.0.0.1', 12345))
-        self.assertEqual((socket.AF_INET, socket.SOCK_STREAM,
-                          ('127.0.0.1', 12345)),
-                         self.unix._guess_remote(sock.fileno()))
-        sock.close()
+        with socket.socket(socket.AF_INET, socket.SOCK_DGRAM) as sock:
+            sock.connect(('127.0.0.1', 12345))
+            self.assertEqual((socket.AF_INET, socket.SOCK_STREAM,
+                              ('127.0.0.1', 12345)),
+                             self.unix._guess_remote(sock.fileno()))
+
         if socket.has_ipv6:
             # Don't check IPv6 address on hosts not supporting them
-            sock = socket.socket(socket.AF_INET6, socket.SOCK_DGRAM)
-            sock.connect(('::1', 12345))
-            self.assertEqual((socket.AF_INET6, socket.SOCK_STREAM,
-                              ('::1', 12345, 0, 0)),
-                             self.unix._guess_remote(sock.fileno()))
-            sock.close()
+            with socket.socket(socket.AF_INET6, socket.SOCK_DGRAM) as sock:
+                sock.connect(('::1', 12345))
+                self.assertEqual((socket.AF_INET6, socket.SOCK_STREAM,
+                                  ('::1', 12345, 0, 0)),
+                                 self.unix._guess_remote(sock.fileno()))
+
             # Try when pretending there's no IPv6 support
             # (No need to pretend when there's really no IPv6)
             xfrout.socket.has_ipv6 = False
-            sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
-            sock.connect(('127.0.0.1', 12345))
-            self.assertEqual((socket.AF_INET, socket.SOCK_STREAM,
-                              ('127.0.0.1', 12345)),
-                             self.unix._guess_remote(sock.fileno()))
+            with socket.socket(socket.AF_INET, socket.SOCK_DGRAM) as sock:
+                sock.connect(('127.0.0.1', 12345))
+                self.assertEqual((socket.AF_INET, socket.SOCK_STREAM,
+                                  ('127.0.0.1', 12345)),
+                                 self.unix._guess_remote(sock.fileno()))
             # Return it back
             xfrout.socket.has_ipv6 = True
-            sock.close()
 
     def test_receive_query_message(self):
         send_msg = b"\xd6=\x00\x00\x00\x01\x00"
@@ -1378,20 +1377,13 @@ class TestUnixSockServer(unittest.TestCase):
         self._remove_file(sock_file)
         self.assertFalse(self.unix._sock_file_in_use(sock_file))
         self._start_unix_sock_server(sock_file)
-
-        old_stdout = sys.stdout
-        sys.stdout = open(os.devnull, 'w')
         self.assertTrue(self.unix._sock_file_in_use(sock_file))
-        sys.stdout.close()
-        sys.stdout = old_stdout
 
     def test_remove_unused_sock_file_in_use(self):
         sock_file = 'temp.sock.file'
         self._remove_file(sock_file)
         self.assertFalse(self.unix._sock_file_in_use(sock_file))
         self._start_unix_sock_server(sock_file)
-        old_stdout = sys.stdout
-        sys.stdout = open(os.devnull, 'w')
         try:
             self.unix._remove_unused_sock_file(sock_file)
         except SystemExit:
@@ -1399,8 +1391,6 @@ class TestUnixSockServer(unittest.TestCase):
         else:
             # This should never happen
             self.assertTrue(False)
-        sys.stdout.close()
-        sys.stdout = old_stdout
 
     def test_remove_unused_sock_file_dir(self):
         import tempfile
diff --git a/src/bin/xfrout/xfrout.py.in b/src/bin/xfrout/xfrout.py.in
index bbc6081..2a19ca7 100755
--- a/src/bin/xfrout/xfrout.py.in
+++ b/src/bin/xfrout/xfrout.py.in
@@ -741,12 +741,14 @@ class UnixSockServer(socketserver_mixin.NoPollMixIn,
         # to care about the SOCK_STREAM parameter at all (which it really is,
         # except for testing)
         if socket.has_ipv6:
-            sock = socket.fromfd(sock_fd, socket.AF_INET6, socket.SOCK_STREAM)
+            socktype = socket.AF_INET6
         else:
             # To make it work even on hosts without IPv6 support
             # (Any idea how to simulate this in test?)
-            sock = socket.fromfd(sock_fd, socket.AF_INET, socket.SOCK_STREAM)
-        peer = sock.getpeername()
+            socktype = socket.AF_INET
+
+        with socket.fromfd(sock_fd, socktype, socket.SOCK_STREAM) as sock:
+            peer = sock.getpeername()
 
         # Identify the correct socket family.  Due to the above "trick",
         # we cannot simply use sock.family.
@@ -756,8 +758,6 @@ class UnixSockServer(socketserver_mixin.NoPollMixIn,
         except socket.error:
             family = socket.AF_INET
 
-        sock.close()
-
         return (family, socket.SOCK_STREAM, peer)
 
     def finish_request(self, sock_fd, request_data):
@@ -798,15 +798,13 @@ class UnixSockServer(socketserver_mixin.NoPollMixIn,
         '''Check whether the socket file 'sock_file' exists and
         is being used by one running xfrout process. If it is,
         return True, or else return False. '''
-        try:
-            sock = socket.socket(socket.AF_UNIX)
-            sock.connect(sock_file)
-        except socket.error as err:
-            sock.close()
-            return False
-        else:
-            sock.close()
-            return True
+        with socket.socket(socket.AF_UNIX) as sock:
+            try:
+                sock.connect(sock_file)
+            except socket.error as err:
+                return False
+            else:
+                return True
 
     def shutdown(self):
         self._write_sock.send(b"shutdown") #terminate the xfrout session thread
diff --git a/src/bin/zonemgr/tests/zonemgr_test.py b/src/bin/zonemgr/tests/zonemgr_test.py
index a32dd98..b7b5961 100644
--- a/src/bin/zonemgr/tests/zonemgr_test.py
+++ b/src/bin/zonemgr/tests/zonemgr_test.py
@@ -99,8 +99,6 @@ class MyZonemgrRefresh(ZonemgrRefresh):
 
 class TestZonemgrRefresh(unittest.TestCase):
     def setUp(self):
-        self.stderr_backup = sys.stderr
-        sys.stderr = open(os.devnull, 'w')
         self.zone_refresh = MyZonemgrRefresh()
         self.cc_session = FakeCCSession()
 
@@ -602,10 +600,6 @@ class TestZonemgrRefresh(unittest.TestCase):
                           self.zone_refresh.update_config_data,
                           config, self.cc_session)
 
-    def tearDown(self):
-        sys.stderr.close()
-        sys.stderr = self.stderr_backup
-
 class MyZonemgr(Zonemgr):
 
     def __init__(self):
diff --git a/src/bin/zonemgr/zonemgr.py.in b/src/bin/zonemgr/zonemgr.py.in
index 7b16f1b..efa6572 100755
--- a/src/bin/zonemgr/zonemgr.py.in
+++ b/src/bin/zonemgr/zonemgr.py.in
@@ -423,6 +423,8 @@ class ZonemgrRefresh:
         self._thread.join()
         # Wipe out what we do not need
         self._thread = None
+        self._read_sock.close()
+        self._write_sock.close()
         self._read_sock = None
         self._write_sock = None
 
diff --git a/src/lib/python/isc/bind10/tests/sockcreator_test.py b/src/lib/python/isc/bind10/tests/sockcreator_test.py
index daf7d09..5e844f3 100644
--- a/src/lib/python/isc/bind10/tests/sockcreator_test.py
+++ b/src/lib/python/isc/bind10/tests/sockcreator_test.py
@@ -304,12 +304,14 @@ class WrapTests(unittest.TestCase):
         # Transfer the descriptor
         send_fd(t1.fileno(), p1.fileno())
         p1.close()
-        p1 = socket.fromfd(t2.read_fd(), socket.AF_UNIX, socket.SOCK_STREAM)
 
-        # Now, pass some data trough the socket
-        p1.send(b'A')
-        data = p2.recv(1)
-        self.assertEqual(b'A', data)
+        with socket.fromfd(t2.read_fd(), socket.AF_UNIX, socket.SOCK_STREAM) as p1:
+            # Now, pass some data trough the socket
+            p1.send(b'A')
+            data = p2.recv(1)
+            self.assertEqual(b'A', data)
+
+        p2.close()
 
         # Test the wrapping didn't hurt the socket's usual methods
         t1.send(b'B')
@@ -319,8 +321,6 @@ class WrapTests(unittest.TestCase):
         data = t1.recv(1)
         self.assertEqual(b'C', data)
 
-        p1.close()
-        p2.close()
         t1.close()
         t2.close()
 



More information about the bind10-changes mailing list