BIND 10 #698: ModuleCCSession object may group_unsubscribe in the closed CC session in being deleted

[BIND 10 Development]

#698: ModuleCCSession object may group_unsubscribe in the closed CC session in
being deleted
-------------------------------------+-------------------------------------
                 Reporter:           |                Owner:  vorner
  naokikambe                         |               Status:  reviewing
                     Type:  defect   |            Milestone:
                 Priority:  major    |  Sprint-20110405
                Component:           |           Resolution:
  message-library                    |            Sensitive:  0
                 Keywords:           |  Add Hours to Ticket:  0
Estimated Number of Hours:  3.0      |          Total Hours:  0
                Billable?:  1        |
                Internal?:  0        |
-------------------------------------+-------------------------------------
Changes (by naokikambe):

 * owner:  naokikambe => vorner


Comment:

 Hello,

 Replying to [comment:9 vorner]:
 > The fakesession is stub code, but the ccsession.py isn't, and that one
 should have a test (because if the test was passed before, this change is
 not tested).
 I think this change is already tested at test_close in ccsession_test.py.
 Is that correct?
 {{{
     def test_close(self):
         fake_session = FakeModuleCCSession()
         mccs = self.create_session("spec1.spec", None, None, fake_session)
         mccs.close()
         self.assertEqual("closed", fake_session._socket)
 }}}
 Further more, we may surely know this change is applied effectively if we
 add the following to unittest_fakesession.py. Is that okay?
 {{{
 @@ -44,6 +44,8 @@ class FakeModuleCCSession:
              self.subscriptions[group_name].append(instance_name)

      def group_unsubscribe(self, group_name, instance_name = None):
 +        if self._closed:
 +            raise isc.cc.SessionError("Session has been closed.")
          if group_name in self.subscriptions:
              if instance_name:
                  if len(self.subscriptions[group_name]) > 1:
 }}}
 However I think the unittest never fails without this change even if this
 exception is raised. Because this exception in deleting the object is just
 ignored like this. Is that correct?
 {{{
 for pytest in ccsession_test.py cfgmgr_test.py config_data_test.py
 module_spec_test.py ; do \
 echo Running test: $pytest ; \
 env
 PYTHONPATH=/home/kambe/git/src/lib/python:/home/kambe/git/src/lib/python \
 CONFIG_TESTDATA_PATH=/home/kambe/git/src/lib/config/tests/testdata \
 CONFIG_WR_TESTDATA_PATH=/home/kambe/git/src/lib/config/tests/testdata \
 python3-coverage run --branch --append
 /home/kambe/git/src/lib/python/isc/config/tests/$pytest || exit ; \
 done
 Running test: ccsession_test.py
 ...................[['ConfigManager', None, {'command': ['get_config',
 {'module_name': 'Spec2'}]}]]
 ..Exception isc.cc.session.SessionError: SessionError('Session has been
 closed.',) in <bound method ModuleCCSession.__del__ of
 <isc.config.ccsession.ModuleCCSession object at 0x9199c2c>> ignored
 ..........[Spec2] Error requesting configuration: just an error
 ....
 ----------------------------------------------------------------------
 Ran 35 tests in 0.038s

 OK
 }}}

 Thanks,

-- 
Ticket URL: <http://bind10.isc.org/ticket/698#comment:10>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development