Browse Source

[1359] throw InternalError when PyObject_Str fails (rather than letting
PyObjectContainer throw PyCPPWrapperException, which would result in
RuntimeError) since it can for other reasons than memory shortage.
added a test case to confirm this.

JINMEI Tatuya 13 years ago
parent
commit
21887dffc4
2 changed files with 21 additions and 6 deletions
  1. 8 1
      src/lib/python/isc/log/log.cc
  2. 13 5
      src/lib/python/isc/log/tests/log_test.py

+ 8 - 1
src/lib/python/isc/log/log.cc

@@ -483,7 +483,14 @@ string
 objectToStr(PyObject* object, bool convert) {
 objectToStr(PyObject* object, bool convert) {
     PyObjectContainer objstr_container;
     PyObjectContainer objstr_container;
     if (convert) {
     if (convert) {
-        objstr_container.reset(PyObject_Str(object));
+        PyObject* text_obj = PyObject_Str(object);
+        if (text_obj == NULL) {
+            // PyObject_Str could fail for various reasons, including because
+            // the object cannot be converted to a string.  We exit with
+            // InternalError to preserve the PyErr set in PyObject_Str.
+            throw InternalError();
+        }
+        objstr_container.reset(text_obj);
         object = objstr_container.get();
         object = objstr_container.get();
     }
     }
 
 

+ 13 - 5
src/lib/python/isc/log/tests/log_test.py

@@ -90,6 +90,7 @@ class Logger(unittest.TestCase):
     def setUp(self):
     def setUp(self):
         isc.log.init("root", "DEBUG", 50)
         isc.log.init("root", "DEBUG", 50)
         self.sevs = ['INFO', 'WARN', 'ERROR', 'FATAL']
         self.sevs = ['INFO', 'WARN', 'ERROR', 'FATAL']
+        self.TEST_MSG = isc.log.create_message('TEST_MESSAGE', '%1')
 
 
     # Checks defaults of the logger
     # Checks defaults of the logger
     def defaults(self, logger):
     def defaults(self, logger):
@@ -174,23 +175,30 @@ class Logger(unittest.TestCase):
         """
         """
         Check whether passing a parameter to a logger causes a reference leak.
         Check whether passing a parameter to a logger causes a reference leak.
         """
         """
-        MSG = isc.log.create_message('TEST_MESSAGE', '%1')
         class LogParam:
         class LogParam:
             def __str__(self):
             def __str__(self):
                 return 'LogParam'
                 return 'LogParam'
         logger = isc.log.Logger("child")
         logger = isc.log.Logger("child")
         param = LogParam()
         param = LogParam()
         orig_msgrefcnt = sys.getrefcount(param)
         orig_msgrefcnt = sys.getrefcount(param)
-        orig_idrefcnt = sys.getrefcount(MSG)
-        logger.info(MSG, param);
-        self.assertEqual(sys.getrefcount(MSG), orig_idrefcnt)
+        orig_idrefcnt = sys.getrefcount(self.TEST_MSG)
+        logger.info(self.TEST_MSG, param);
+        self.assertEqual(sys.getrefcount(self.TEST_MSG), orig_idrefcnt)
         self.assertEqual(sys.getrefcount(param), orig_msgrefcnt)
         self.assertEqual(sys.getrefcount(param), orig_msgrefcnt)
 
 
         # intentionally pass an invalid type for debug level.  It will
         # intentionally pass an invalid type for debug level.  It will
         # result in TypeError.  The passed object still shouldn't leak a
         # result in TypeError.  The passed object still shouldn't leak a
         # reference.
         # reference.
-        self.assertRaises(TypeError, logger.debug, param, MSG, param)
+        self.assertRaises(TypeError, logger.debug, param, self.TEST_MSG, param)
         self.assertEqual(sys.getrefcount(param), orig_msgrefcnt)
         self.assertEqual(sys.getrefcount(param), orig_msgrefcnt)
 
 
+    def test_bad_parameter(self):
+        # a log parameter cannot be converted to a string object.
+        class LogParam:
+            def __str__(self):
+                raise ValueError("LogParam can't be converted to string")
+        logger = isc.log.Logger("child")
+        self.assertRaises(ValueError, logger.info, self.TEST_MSG, LogParam())
+
 if __name__ == '__main__':
 if __name__ == '__main__':
     unittest.main()
     unittest.main()