From 1fc79b932eaa88be33c224e4eea3fc58907e98bd Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Wed, 26 Oct 2011 16:07:01 -0700 Subject: [PATCH] [1028] removed self reference in XfrinConnection, which apparently caused reference leak. To test this case, introduced a framework for testing process_xfrin(). This setup will be used for subseuent related fixes. --- src/bin/xfrin/tests/xfrin_test.py | 59 +++++++++++++++++++++++++++---- src/bin/xfrin/xfrin.py.in | 13 +++---- 2 files changed, 58 insertions(+), 14 deletions(-) diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py index 65bd968125..bddb0f9377 100644 --- a/src/bin/xfrin/tests/xfrin_test.py +++ b/src/bin/xfrin/tests/xfrin_test.py @@ -16,6 +16,7 @@ import unittest import shutil import socket +import sys import io from isc.testutils.tsigctx_mock import MockTSIGContext from xfrin import * @@ -216,8 +217,8 @@ class MockXfrin(Xfrin): request_type, check_soa) class MockXfrinConnection(XfrinConnection): - def __init__(self, sock_map, zone_name, rrclass, shutdown_event, - master_addr): + def __init__(self, sock_map, zone_name, rrclass, datasrc_client, + shutdown_event, master_addr, tsig_key=None): super().__init__(sock_map, zone_name, rrclass, MockDataSourceClient(), shutdown_event, master_addr) self.query_data = b'' @@ -300,7 +301,7 @@ class TestXfrinState(unittest.TestCase): def setUp(self): self.sock_map = {} self.conn = MockXfrinConnection(self.sock_map, TEST_ZONE_NAME, - TEST_RRCLASS, threading.Event(), + TEST_RRCLASS, None, threading.Event(), TEST_MASTER_IPV4_ADDRINFO) self.begin_soa = RRset(TEST_ZONE_NAME, TEST_RRCLASS, RRType.SOA(), RRTTL(3600)) @@ -585,7 +586,7 @@ class TestXfrinConnection(unittest.TestCase): os.remove(TEST_DB_FILE) self.sock_map = {} self.conn = MockXfrinConnection(self.sock_map, TEST_ZONE_NAME, - TEST_RRCLASS, threading.Event(), + TEST_RRCLASS, None, threading.Event(), TEST_MASTER_IPV4_ADDRINFO) self.soa_response_params = { 'questions': [example_soa_question], @@ -720,13 +721,13 @@ class TestAXFR(TestXfrinConnection): # to confirm an AF_INET6 socket has been created. A naive application # tends to assume it's IPv4 only and hardcode AF_INET. This test # uncovers such a bug. - c = MockXfrinConnection({}, TEST_ZONE_NAME, TEST_RRCLASS, + c = MockXfrinConnection({}, TEST_ZONE_NAME, TEST_RRCLASS, None, threading.Event(), TEST_MASTER_IPV6_ADDRINFO) c.bind(('::', 0)) c.close() def test_init_chclass(self): - c = MockXfrinConnection({}, TEST_ZONE_NAME, RRClass.CH(), + c = MockXfrinConnection({}, TEST_ZONE_NAME, RRClass.CH(), None, threading.Event(), TEST_MASTER_IPV4_ADDRINFO) axfrmsg = c._create_query(RRType.AXFR()) self.assertEqual(axfrmsg.get_question()[0].get_class(), @@ -1679,6 +1680,52 @@ class TestXfrinRecorder(unittest.TestCase): self.recorder.decrement(TEST_ZONE_NAME) self.assertEqual(self.recorder.xfrin_in_progress(TEST_ZONE_NAME), False) +class TestXfrinProcess(unittest.TestCase): + def setUp(self): + self.unlocked = False + + def tearDown(self): + self.assertTrue(self.unlocked) + + def increment(self, zone_name): + '''Fake method of xfrin_recorder.increment. + + ''' + pass + + def decrement(self, zone_name): + '''Fake method of xfrin_recorder.decrement. + + ''' + self.unlocked = True + + def publish_xfrin_news(self, zone_name, rrclass, ret): + '''Fake method of serve.publish_xfrin_news + + ''' + pass + + def create_xfrinconn(self, sock_map, zone_name, rrclass, datasrc_client, + shutdown_event, master_addrinfo, tsig_key): + conn = MockXfrinConnection(sock_map, zone_name, rrclass, + datasrc_client, shutdown_event, + master_addrinfo, tsig_key) + + # An awkward check that would specifically identify an old bug + # where initialziation of XfrinConnection._tsig_ctx_creator caused + # self reference and subsequently led to reference leak. + orig_ref = sys.getrefcount(conn) + conn._tsig_ctx_creator = None + self.assertEqual(orig_ref, sys.getrefcount(conn)) + + return conn + + def test_process_xfrin_normal(self): + process_xfrin(self, self, TEST_ZONE_NAME, TEST_RRCLASS, None, None, + (socket.AF_INET, socket.SOCK_STREAM, + TEST_MASTER_IPV4_ADDRESS), False, None, RRType.AXFR(), + self.create_xfrinconn) + class TestXfrin(unittest.TestCase): def setUp(self): # redirect output diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index 1f5d9a1518..339f6d5e2d 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -323,6 +323,7 @@ class XfrinFirstData(XfrinState): conn.zone_str()) # We are now going to add RRs to the new zone. We need create # a Diff object. It will be used throughtout the XFR session. + # DISABLE FOR DEBUG conn._diff = Diff(conn._datasrc_client, conn._zone_name, True) self.set_xfrstate(conn, XfrinAXFR()) return False @@ -479,10 +480,7 @@ class XfrinConnection(asyncore.dispatcher): self._tsig_ctx = None # tsig_ctx_creator is introduced to allow tests to use a mock class for # easier tests (in normal case we always use the default) - self._tsig_ctx_creator = self.__create_tsig_ctx - - def __create_tsig_ctx(self, key): - return TSIGContext(key) + self._tsig_ctx_creator = lambda key : TSIGContext(key) def __set_xfrstate(self, new_state): self.__state = new_state @@ -800,7 +798,7 @@ class XfrinConnection(asyncore.dispatcher): def process_xfrin(server, xfrin_recorder, zone_name, rrclass, db_file, shutdown_event, master_addrinfo, check_soa, tsig_key, - request_type): + request_type, conn_class=XfrinConnection): xfrin_recorder.increment(zone_name) # Create a data source client used in this XFR session. Right now we @@ -820,8 +818,8 @@ def process_xfrin(server, xfrin_recorder, zone_name, rrclass, db_file, # Create a TCP connection for the XFR session and perform the operation. sock_map = {} - conn = XfrinConnection(sock_map, zone_name, rrclass, datasrc_client, - shutdown_event, master_addrinfo, tsig_key) + conn = conn_class(sock_map, zone_name, rrclass, datasrc_client, + shutdown_event, master_addrinfo, tsig_key) # XXX: We still need _db_file for temporary workaround in _create_query(). # This should be removed when we eliminate the need for the workaround. conn._db_file = db_file @@ -835,7 +833,6 @@ def process_xfrin(server, xfrin_recorder, zone_name, rrclass, db_file, server.publish_xfrin_news(zone_name, rrclass, ret) xfrin_recorder.decrement(zone_name) - class XfrinRecorder: def __init__(self): self._lock = threading.Lock()