BIND 10 trac2379_2, updated. 3fb09910a4c862fef0e8f3dc631df0fba49c8107 [2379] Explicitely clear loader after each test

BIND 10 source code commits bind10-changes at lists.isc.org
Fri Dec 14 17:35:35 UTC 2012


The branch, trac2379_2 has been updated
       via  3fb09910a4c862fef0e8f3dc631df0fba49c8107 (commit)
       via  94d0481e3dbbc16f3178bcdb71e4b02af6ef85bb (commit)
       via  3d8e49eb1d45b2a70b5ecb90d77116653a26b11f (commit)
       via  d7368c1d83dc9c6cbbfd4230efa03858307062d4 (commit)
       via  c9a786613ef05879fd0bcf88e6db0adf855e6ea1 (commit)
       via  a8b060aa956d679e7a4330450c9baf7e470ad6a5 (commit)
       via  bb85c71585b1bafb659287dba976940d37178630 (commit)
      from  52b93e86bad0bc578336b9fc15e2a76c79dbecb9 (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 3fb09910a4c862fef0e8f3dc631df0fba49c8107
Author: Jelte Jansen <jelte at isc.org>
Date:   Fri Dec 14 18:00:05 2012 +0100

    [2379] Explicitely clear loader after each test

commit 94d0481e3dbbc16f3178bcdb71e4b02af6ef85bb
Author: Jelte Jansen <jelte at isc.org>
Date:   Fri Dec 14 17:46:09 2012 +0100

    [2379] update pydoc for wrapper

commit 3d8e49eb1d45b2a70b5ecb90d77116653a26b11f
Author: Jelte Jansen <jelte at isc.org>
Date:   Fri Dec 14 17:32:55 2012 +0100

    [2379] Fix no_such_zone_in_source test

commit d7368c1d83dc9c6cbbfd4230efa03858307062d4
Author: Jelte Jansen <jelte at isc.org>
Date:   Fri Dec 14 15:36:13 2012 +0100

    [2379] Better refcounting

commit c9a786613ef05879fd0bcf88e6db0adf855e6ea1
Author: Jelte Jansen <jelte at isc.org>
Date:   Fri Dec 14 14:29:40 2012 +0100

    [2379] Cleanup some docs and variables

commit a8b060aa956d679e7a4330450c9baf7e470ad6a5
Author: Jelte Jansen <jelte at isc.org>
Date:   Fri Dec 14 11:38:26 2012 +0100

    [2379] add standard function for class initialization

commit bb85c71585b1bafb659287dba976940d37178630
Author: Jelte Jansen <jelte at isc.org>
Date:   Fri Dec 14 10:49:56 2012 +0100

    [2379] Explicitely clear loader after each test
    
    Instead of direct adds

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

Summary of changes:
 src/lib/dns/python/pydnspp.cc                      |  217 +++++++-------------
 src/lib/dns/python/pydnspp_common.cc               |   16 ++
 src/lib/dns/python/pydnspp_common.h                |   12 ++
 src/lib/python/isc/datasrc/client_python.cc        |    5 +-
 src/lib/python/isc/datasrc/client_python.h         |    8 +
 .../python/isc/datasrc/tests/zone_loader_test.py   |  106 +++++++---
 src/lib/python/isc/datasrc/zone_loader_inc.cc      |  155 ++++++++------
 src/lib/python/isc/datasrc/zone_loader_python.cc   |   33 ++-
 8 files changed, 302 insertions(+), 250 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/dns/python/pydnspp.cc b/src/lib/dns/python/pydnspp.cc
index 31aca42..44a3785 100644
--- a/src/lib/dns/python/pydnspp.cc
+++ b/src/lib/dns/python/pydnspp.cc
@@ -65,21 +65,13 @@ namespace {
 
 bool
 initModulePart_EDNS(PyObject* mod) {
-    // We initialize the static description object with PyType_Ready(),
-    // then add it to the module. This is not just a check! (leaving
-    // this out results in segmentation faults)
-    //
     // After the type has been initialized, we initialize any exceptions
     // that are defined in the wrapper for this class, and add constants
     // to the type, if any
 
-    if (PyType_Ready(&edns_type) < 0) {
+    if (!initClass(edns_type, "EDNS", mod)) {
         return (false);
     }
-    Py_INCREF(&edns_type);
-    void* p = &edns_type;
-    PyModule_AddObject(mod, "EDNS", static_cast<PyObject*>(p));
-
     addClassVariable(edns_type, "SUPPORTED_VERSION",
                      Py_BuildValue("B", EDNS::SUPPORTED_VERSION));
 
@@ -88,14 +80,9 @@ initModulePart_EDNS(PyObject* mod) {
 
 bool
 initModulePart_Message(PyObject* mod) {
-    if (PyType_Ready(&message_type) < 0) {
-        return (false);
-    }
-    void* p = &message_type;
-    if (PyModule_AddObject(mod, "Message", static_cast<PyObject*>(p)) < 0) {
+    if (!initClass(message_type, "Message", mod)) {
         return (false);
     }
-    Py_INCREF(&message_type);
 
     try {
         //
@@ -186,18 +173,15 @@ initModulePart_Message(PyObject* mod) {
 
 bool
 initModulePart_MessageRenderer(PyObject* mod) {
-    if (PyType_Ready(&messagerenderer_type) < 0) {
+    if (!initClass(messagerenderer_type, "MessageRenderer", mod)) {
         return (false);
     }
-    Py_INCREF(&messagerenderer_type);
 
     addClassVariable(messagerenderer_type, "CASE_INSENSITIVE",
                      Py_BuildValue("I", MessageRenderer::CASE_INSENSITIVE));
     addClassVariable(messagerenderer_type, "CASE_SENSITIVE",
                      Py_BuildValue("I", MessageRenderer::CASE_SENSITIVE));
 
-    PyModule_AddObject(mod, "MessageRenderer",
-                       reinterpret_cast<PyObject*>(&messagerenderer_type));
 
     return (true);
 }
@@ -208,10 +192,10 @@ initModulePart_Name(PyObject* mod) {
     //
     // NameComparisonResult
     //
-    if (PyType_Ready(&name_comparison_result_type) < 0) {
+    if (!initClass(name_comparison_result_type,
+                   "NameComparisonResult", mod)) {
         return (false);
     }
-    Py_INCREF(&name_comparison_result_type);
 
     // Add the enums to the module
     po_NameRelation = Py_BuildValue("{i:s,i:s,i:s,i:s}",
@@ -231,17 +215,13 @@ initModulePart_Name(PyObject* mod) {
     addClassVariable(name_comparison_result_type, "COMMONANCESTOR",
                      Py_BuildValue("I", NameComparisonResult::COMMONANCESTOR));
 
-    PyModule_AddObject(mod, "NameComparisonResult",
-        reinterpret_cast<PyObject*>(&name_comparison_result_type));
 
     //
     // Name
     //
-
-    if (PyType_Ready(&name_type) < 0) {
+    if (!initClass(name_type, "Name", mod)) {
         return (false);
     }
-    Py_INCREF(&name_type);
 
     // Add the constants to the module
     addClassVariable(name_type, "MAX_WIRE",
@@ -260,51 +240,46 @@ initModulePart_Name(PyObject* mod) {
     addClassVariable(name_type, "ROOT_NAME",
                      createNameObject(Name::ROOT_NAME()));
 
-    PyModule_AddObject(mod, "Name",
-                       reinterpret_cast<PyObject*>(&name_type));
 
 
     // Add the exceptions to the module
     po_EmptyLabel = PyErr_NewException("pydnspp.EmptyLabel", NULL, NULL);
-    PyModule_AddObject(mod, "EmptyLabel", po_EmptyLabel);
+    PyObjectContainer(po_EmptyLabel).installToModule(mod, "EmptyLabel");
 
     po_TooLongName = PyErr_NewException("pydnspp.TooLongName", NULL, NULL);
-    PyModule_AddObject(mod, "TooLongName", po_TooLongName);
+    PyObjectContainer(po_TooLongName).installToModule(mod, "TooLongName");
 
     po_TooLongLabel = PyErr_NewException("pydnspp.TooLongLabel", NULL, NULL);
-    PyModule_AddObject(mod, "TooLongLabel", po_TooLongLabel);
+    PyObjectContainer(po_TooLongLabel).installToModule(mod, "TooLongLabel");
 
     po_BadLabelType = PyErr_NewException("pydnspp.BadLabelType", NULL, NULL);
-    PyModule_AddObject(mod, "BadLabelType", po_BadLabelType);
+    PyObjectContainer(po_BadLabelType).installToModule(mod, "BadLabelType");
 
     po_BadEscape = PyErr_NewException("pydnspp.BadEscape", NULL, NULL);
-    PyModule_AddObject(mod, "BadEscape", po_BadEscape);
+    PyObjectContainer(po_BadEscape).installToModule(mod, "BadEscape");
 
-    po_IncompleteName = PyErr_NewException("pydnspp.IncompleteName", NULL, NULL);
-    PyModule_AddObject(mod, "IncompleteName", po_IncompleteName);
+    po_IncompleteName = PyErr_NewException("pydnspp.IncompleteName", NULL,
+                                           NULL);
+    PyObjectContainer(po_IncompleteName).installToModule(mod, "IncompleteName");
 
     po_InvalidBufferPosition =
         PyErr_NewException("pydnspp.InvalidBufferPosition", NULL, NULL);
-    PyModule_AddObject(mod, "InvalidBufferPosition", po_InvalidBufferPosition);
+    PyObjectContainer(po_InvalidBufferPosition).installToModule(
+        mod, "InvalidBufferPosition");
 
     // This one could have gone into the message_python.cc file, but is
     // already needed here.
     po_DNSMessageFORMERR = PyErr_NewException("pydnspp.DNSMessageFORMERR",
                                               NULL, NULL);
-    PyModule_AddObject(mod, "DNSMessageFORMERR", po_DNSMessageFORMERR);
+    PyObjectContainer(po_DNSMessageFORMERR).installToModule(
+        mod, "DNSMessageFORMERR");
 
     return (true);
 }
 
 bool
 initModulePart_Opcode(PyObject* mod) {
-    if (PyType_Ready(&opcode_type) < 0) {
-        return (false);
-    }
-    Py_INCREF(&opcode_type);
-    void* p = &opcode_type;
-    if (PyModule_AddObject(mod, "Opcode", static_cast<PyObject*>(p)) != 0) {
-        Py_DECREF(&opcode_type);
+    if (!initClass(opcode_type, "Opcode", mod)) {
         return (false);
     }
 
@@ -346,25 +321,17 @@ initModulePart_Opcode(PyObject* mod) {
 
 bool
 initModulePart_Question(PyObject* mod) {
-    if (PyType_Ready(&question_type) < 0) {
+    if (!initClass(question_type, "Question", mod)) {
         return (false);
     }
-    Py_INCREF(&question_type);
-    PyModule_AddObject(mod, "Question",
-                       reinterpret_cast<PyObject*>(&question_type));
 
+    Py_INCREF(&question_type);
     return (true);
 }
 
 bool
 initModulePart_Rcode(PyObject* mod) {
-    if (PyType_Ready(&rcode_type) < 0) {
-        return (false);
-    }
-    Py_INCREF(&rcode_type);
-    void* p = &rcode_type;
-    if (PyModule_AddObject(mod, "Rcode", static_cast<PyObject*>(p)) != 0) {
-        Py_DECREF(&rcode_type);
+    if (!initClass(rcode_type, "Rcode", mod)) {
         return (false);
     }
 
@@ -408,126 +375,108 @@ initModulePart_Rcode(PyObject* mod) {
 
 bool
 initModulePart_Rdata(PyObject* mod) {
-    if (PyType_Ready(&rdata_type) < 0) {
+    if (!initClass(rdata_type, "Rdata", mod)) {
         return (false);
     }
-    Py_INCREF(&rdata_type);
-    PyModule_AddObject(mod, "Rdata",
-                       reinterpret_cast<PyObject*>(&rdata_type));
 
     // Add the exceptions to the class
     po_InvalidRdataLength = PyErr_NewException("pydnspp.InvalidRdataLength",
                                                NULL, NULL);
-    PyModule_AddObject(mod, "InvalidRdataLength", po_InvalidRdataLength);
+    PyObjectContainer(po_InvalidRdataLength).installToModule(
+        mod, "InvalidRdataLength");
 
     po_InvalidRdataText = PyErr_NewException("pydnspp.InvalidRdataText",
                                              NULL, NULL);
-    PyModule_AddObject(mod, "InvalidRdataText", po_InvalidRdataText);
+    PyObjectContainer(po_InvalidRdataText).installToModule(
+        mod, "InvalidRdataText");
 
     po_CharStringTooLong = PyErr_NewException("pydnspp.CharStringTooLong",
                                               NULL, NULL);
-    PyModule_AddObject(mod, "CharStringTooLong", po_CharStringTooLong);
-
+    PyObjectContainer(po_CharStringTooLong).installToModule(
+        mod, "CharStringTooLong");
 
     return (true);
 }
 
 bool
 initModulePart_RRClass(PyObject* mod) {
+    if (!initClass(rrclass_type, "RRClass", mod)) {
+        return (false);
+    }
+
     po_InvalidRRClass = PyErr_NewException("pydnspp.InvalidRRClass",
                                            NULL, NULL);
-    Py_INCREF(po_InvalidRRClass);
-    PyModule_AddObject(mod, "InvalidRRClass", po_InvalidRRClass);
+    PyObjectContainer(po_InvalidRRClass).installToModule(
+        mod, "InvalidRRClass");
+
     po_IncompleteRRClass = PyErr_NewException("pydnspp.IncompleteRRClass",
                                               NULL, NULL);
-    Py_INCREF(po_IncompleteRRClass);
-    PyModule_AddObject(mod, "IncompleteRRClass", po_IncompleteRRClass);
-
-    if (PyType_Ready(&rrclass_type) < 0) {
-        return (false);
-    }
-    Py_INCREF(&rrclass_type);
-    PyModule_AddObject(mod, "RRClass",
-                       reinterpret_cast<PyObject*>(&rrclass_type));
+    PyObjectContainer(po_IncompleteRRClass).installToModule(
+        mod, "IncompleteRRClass");
 
     return (true);
 }
 
 bool
 initModulePart_RRset(PyObject* mod) {
-    po_EmptyRRset = PyErr_NewException("pydnspp.EmptyRRset", NULL, NULL);
-    PyModule_AddObject(mod, "EmptyRRset", po_EmptyRRset);
-
-    // NameComparisonResult
-    if (PyType_Ready(&rrset_type) < 0) {
+    if (!initClass(rrset_type, "RRset", mod)) {
         return (false);
     }
-    Py_INCREF(&rrset_type);
-    PyModule_AddObject(mod, "RRset",
-                       reinterpret_cast<PyObject*>(&rrset_type));
+
+    po_EmptyRRset = PyErr_NewException("pydnspp.EmptyRRset", NULL, NULL);
+    PyObjectContainer(po_EmptyRRset).installToModule(mod, "EmptyRRset");
 
     return (true);
 }
 
 bool
 initModulePart_RRTTL(PyObject* mod) {
+    if (!initClass(rrttl_type, "RRTTL", mod)) {
+        return (false);
+    }
+
     po_InvalidRRTTL = PyErr_NewException("pydnspp.InvalidRRTTL", NULL, NULL);
-    PyModule_AddObject(mod, "InvalidRRTTL", po_InvalidRRTTL);
+    PyObjectContainer(po_InvalidRRTTL).installToModule(mod, "InvalidRRTTL");
+
     po_IncompleteRRTTL = PyErr_NewException("pydnspp.IncompleteRRTTL",
                                             NULL, NULL);
-    PyModule_AddObject(mod, "IncompleteRRTTL", po_IncompleteRRTTL);
-
-    if (PyType_Ready(&rrttl_type) < 0) {
-        return (false);
-    }
-    Py_INCREF(&rrttl_type);
-    PyModule_AddObject(mod, "RRTTL",
-                       reinterpret_cast<PyObject*>(&rrttl_type));
+    PyObjectContainer(po_IncompleteRRTTL).installToModule(
+        mod, "IncompleteRRTTL");
 
     return (true);
 }
 
 bool
 initModulePart_RRType(PyObject* mod) {
-    // Add the exceptions to the module
+    if (!initClass(rrtype_type, "RRType", mod)) {
+        return (false);
+    }
+
     po_InvalidRRType = PyErr_NewException("pydnspp.InvalidRRType", NULL, NULL);
-    PyModule_AddObject(mod, "InvalidRRType", po_InvalidRRType);
+    PyObjectContainer(po_InvalidRRType).installToModule(mod, "InvalidRRType");
+
     po_IncompleteRRType = PyErr_NewException("pydnspp.IncompleteRRType",
                                              NULL, NULL);
-    PyModule_AddObject(mod, "IncompleteRRType", po_IncompleteRRType);
-
-    if (PyType_Ready(&rrtype_type) < 0) {
-        return (false);
-    }
-    Py_INCREF(&rrtype_type);
-    PyModule_AddObject(mod, "RRType",
-                       reinterpret_cast<PyObject*>(&rrtype_type));
+    PyObjectContainer(po_IncompleteRRType).installToModule(
+        mod, "IncompleteRRType");
 
     return (true);
 }
 
 bool
 initModulePart_Serial(PyObject* mod) {
-    if (PyType_Ready(&serial_type) < 0) {
+    if (!initClass(serial_type, "Serial", mod)) {
         return (false);
     }
-    Py_INCREF(&serial_type);
-    PyModule_AddObject(mod, "Serial",
-                       reinterpret_cast<PyObject*>(&serial_type));
 
     return (true);
 }
 
 bool
 initModulePart_TSIGError(PyObject* mod) {
-    if (PyType_Ready(&tsigerror_type) < 0) {
+    if (!initClass(tsigerror_type, "TSIGError", mod)) {
         return (false);
     }
-    void* p = &tsigerror_type;
-    if (PyModule_AddObject(mod, "TSIGError", static_cast<PyObject*>(p)) < 0) {
-        return (false);
-    }
-    Py_INCREF(&tsigerror_type);
 
     try {
         // Constant class variables
@@ -595,14 +544,9 @@ initModulePart_TSIGError(PyObject* mod) {
 
 bool
 initModulePart_TSIGKey(PyObject* mod) {
-    if (PyType_Ready(&tsigkey_type) < 0) {
-        return (false);
-    }
-    void* p = &tsigkey_type;
-    if (PyModule_AddObject(mod, "TSIGKey", static_cast<PyObject*>(p)) != 0) {
+    if (!initClass(tsigkey_type, "TSIGKey", mod)) {
         return (false);
     }
-    Py_INCREF(&tsigkey_type);
 
     try {
         // Constant class variables
@@ -635,14 +579,7 @@ initModulePart_TSIGKey(PyObject* mod) {
 
 bool
 initModulePart_TSIGKeyRing(PyObject* mod) {
-    if (PyType_Ready(&tsigkeyring_type) < 0) {
-        return (false);
-    }
-    Py_INCREF(&tsigkeyring_type);
-    void* p = &tsigkeyring_type;
-    if (PyModule_AddObject(mod, "TSIGKeyRing",
-                           static_cast<PyObject*>(p)) != 0) {
-        Py_DECREF(&tsigkeyring_type);
+    if (!initClass(tsigkeyring_type, "TSIGKeyRing", mod)) {
         return (false);
     }
 
@@ -658,15 +595,9 @@ initModulePart_TSIGKeyRing(PyObject* mod) {
 
 bool
 initModulePart_TSIGContext(PyObject* mod) {
-    if (PyType_Ready(&tsigcontext_type) < 0) {
+    if (!initClass(tsigcontext_type, "TSIGContext", mod)) {
         return (false);
     }
-    void* p = &tsigcontext_type;
-    if (PyModule_AddObject(mod, "TSIGContext",
-                           static_cast<PyObject*>(p)) < 0) {
-        return (false);
-    }
-    Py_INCREF(&tsigcontext_type);
 
     try {
         // Class specific exceptions
@@ -707,28 +638,18 @@ initModulePart_TSIGContext(PyObject* mod) {
 
 bool
 initModulePart_TSIG(PyObject* mod) {
-    if (PyType_Ready(&tsig_type) < 0) {
-        return (false);
-    }
-    void* p = &tsig_type;
-    if (PyModule_AddObject(mod, "TSIG", static_cast<PyObject*>(p)) < 0) {
+    if (!initClass(tsig_type, "TSIG", mod)) {
         return (false);
     }
-    Py_INCREF(&tsig_type);
 
     return (true);
 }
 
 bool
 initModulePart_TSIGRecord(PyObject* mod) {
-    if (PyType_Ready(&tsigrecord_type) < 0) {
-        return (false);
-    }
-    void* p = &tsigrecord_type;
-    if (PyModule_AddObject(mod, "TSIGRecord", static_cast<PyObject*>(p)) < 0) {
+    if (!initClass(tsigrecord_type, "TSIGRecord", mod)) {
         return (false);
     }
-    Py_INCREF(&tsigrecord_type);
 
     try {
         // Constant class variables
@@ -775,15 +696,17 @@ PyInit_pydnspp(void) {
 
     // Add the exceptions to the class
     po_IscException = PyErr_NewException("pydnspp.IscException", NULL, NULL);
-    PyModule_AddObject(mod, "IscException", po_IscException);
+    PyObjectContainer(po_IscException).installToModule(mod, "IscException");
 
     po_InvalidOperation = PyErr_NewException("pydnspp.InvalidOperation",
                                              NULL, NULL);
-    PyModule_AddObject(mod, "InvalidOperation", po_InvalidOperation);
+    PyObjectContainer(po_InvalidOperation).installToModule(
+        mod, "InvalidOperation");
 
     po_InvalidParameter = PyErr_NewException("pydnspp.InvalidParameter",
                                              NULL, NULL);
-    PyModule_AddObject(mod, "InvalidParameter", po_InvalidParameter);
+    PyObjectContainer(po_InvalidParameter).installToModule(
+        mod, "InvalidParameter");
 
     // for each part included above, we call its specific initializer
 
diff --git a/src/lib/dns/python/pydnspp_common.cc b/src/lib/dns/python/pydnspp_common.cc
index ad48ec7..6213a4a 100644
--- a/src/lib/dns/python/pydnspp_common.cc
+++ b/src/lib/dns/python/pydnspp_common.cc
@@ -92,6 +92,22 @@ addClassVariable(PyTypeObject& c, const char* name, PyObject* obj) {
     }
     return (PyDict_SetItemString(c.tp_dict, name, obj));
 }
+
+bool
+initClass(PyTypeObject& type, const char* name, PyObject* mod) {
+    // We initialize the static description object with PyType_Ready(),
+    // then add it to the module. This is not just a check! (leaving
+    // this out results in segmentation faults)
+    //
+    if (PyType_Ready(&type) < 0 ||
+        PyModule_AddObject(mod, name,
+                           reinterpret_cast<PyObject*>(&type)) < 0) {
+        return (false);
+    }
+    Py_INCREF(&type);
+    return (true);
+}
+
 }
 }
 }
diff --git a/src/lib/dns/python/pydnspp_common.h b/src/lib/dns/python/pydnspp_common.h
index b503682..4095f54 100644
--- a/src/lib/dns/python/pydnspp_common.h
+++ b/src/lib/dns/python/pydnspp_common.h
@@ -48,6 +48,18 @@ int readDataFromSequence(uint8_t *data, size_t len, PyObject* sequence);
 
 int addClassVariable(PyTypeObject& c, const char* name, PyObject* obj);
 
+/// \brief Initialize a wrapped class type, and add to module
+///
+/// The type object is initalized, and its refcount is increased after
+/// successful addition to the module.
+///
+/// \param type The type object to initialize
+/// \param name The python name of the class to add
+/// \param mod The python module to add the class to
+///
+/// \return true on success, false on failure
+bool initClass(PyTypeObject& type, const char* name, PyObject* mod);
+
 // Short term workaround for unifying the return type of tp_hash
 #if PY_MINOR_VERSION < 2
 typedef long Py_hash_t;
diff --git a/src/lib/python/isc/datasrc/client_python.cc b/src/lib/python/isc/datasrc/client_python.cc
index 1c60c57..8fea173 100644
--- a/src/lib/python/isc/datasrc/client_python.cc
+++ b/src/lib/python/isc/datasrc/client_python.cc
@@ -378,10 +378,9 @@ DataSourceClient&
 PyDataSourceClient_ToDataSourceClient(PyObject* client_obj) {
     if (client_obj == NULL) {
         isc_throw(PyCPPWrapperException,
-                  "obj argument NULL in Name PyObject conversion");
+                  "argument NULL in DataSourceClient PyObject conversion");
     }
-    const s_DataSourceClient* client =
-        static_cast<const s_DataSourceClient*>(client_obj);
+    s_DataSourceClient* client = static_cast<s_DataSourceClient*>(client_obj);
     return (*client->client);
 }
 
diff --git a/src/lib/python/isc/datasrc/client_python.h b/src/lib/python/isc/datasrc/client_python.h
index 09732cc..e700fde 100644
--- a/src/lib/python/isc/datasrc/client_python.h
+++ b/src/lib/python/isc/datasrc/client_python.h
@@ -44,6 +44,14 @@ wrapDataSourceClient(DataSourceClient* client,
                      LifeKeeper>& life_keeper = boost::shared_ptr<ClientList::
                      FindResult::LifeKeeper>());
 
+/// \brief Returns a reference to the DataSourceClient object contained
+///        in the given Python object.
+///
+/// \note The given object MUST be of type DataSourceClient; this can be
+///       checked with the right call to ParseTuple("O!")
+///
+/// \param client_obj Python object holding the DataSourceClient
+/// \return reference to the DataSourceClient object
 DataSourceClient&
 PyDataSourceClient_ToDataSourceClient(PyObject* client_obj);
 
diff --git a/src/lib/python/isc/datasrc/tests/zone_loader_test.py b/src/lib/python/isc/datasrc/tests/zone_loader_test.py
index f7fc2e0..a8095ba 100644
--- a/src/lib/python/isc/datasrc/tests/zone_loader_test.py
+++ b/src/lib/python/isc/datasrc/tests/zone_loader_test.py
@@ -47,6 +47,13 @@ class ZoneLoaderTests(unittest.TestCase):
         # Make a fresh copy of the database
         shutil.copy(ORIG_DB_FILE, DB_FILE)
 
+    def tearDown(self):
+        # We can only create 1 loader at a time (it locks the db), and it
+        # may not be destroyed immediately if there is an exception in a
+        # test. So the tests that do create one should put it in self, and
+        # we make sure to invalidate it here.
+        self.loader = None
+
     def test_bad_constructor(self):
         self.assertRaises(TypeError, isc.datasrc.ZoneLoader)
         self.assertRaises(TypeError, isc.datasrc.ZoneLoader, 1)
@@ -69,67 +76,95 @@ class ZoneLoaderTests(unittest.TestCase):
         self.assertEqual(finder.SUCCESS, result)
         self.assertEqual(soa_txt, rrset.to_text())
 
-    def check_load(self, loader):
+    def check_load(self):
         self.check_zone_soa(ORIG_SOA_TXT)
-        loader.load()
+        self.loader.load()
         self.check_zone_soa(NEW_SOA_TXT)
 
         # And after that, it should throw
-        self.assertRaises(isc.dns.InvalidOperation, loader.load)
+        self.assertRaises(isc.dns.InvalidOperation, self.loader.load)
 
     def test_load_from_file(self):
-        loader = isc.datasrc.ZoneLoader(self.client, self.test_name,
-                                        self.test_file)
-        self.check_load(loader)
+        self.loader = isc.datasrc.ZoneLoader(self.client, self.test_name,
+                                             self.test_file)
+        self.check_load()
 
     def test_load_from_client(self):
         source_client = isc.datasrc.DataSourceClient('sqlite3',
                                                      DB_SOURCE_CLIENT_CONFIG)
-        loader = isc.datasrc.ZoneLoader(self.client, self.test_name,
-                                        source_client)
-        self.check_load(loader)
-
-    def check_load_incremental(self, loader):
+        self.loader = isc.datasrc.ZoneLoader(self.client, self.test_name,
+                                             source_client)
+        self.check_load()
+
+    def test_load_from_file_checkrefs(self):
+        # A test to see the refcount is increased properly
+        self.loader = isc.datasrc.ZoneLoader(self.client, self.test_name,
+                                             self.test_file)
+        # Explicitely delete the objects here, so we trigger bad reference
+        # counting (best effort, if there are leaked references for these
+        # objects themselves, it still won't fail)
+        self.client = None
+        self.client = None
+        self.test_name = None
+        self.test_file = None
+        self.loader.load()
+
+    def test_load_from_client_checkrefs(self):
+        # A test to see the refcount is increased properly
+        source_client = isc.datasrc.DataSourceClient('sqlite3',
+                                                     DB_SOURCE_CLIENT_CONFIG)
+        self.loader = isc.datasrc.ZoneLoader(self.client, self.test_name,
+                                             source_client)
+        # Explicitely delete the objects here, so we trigger bad reference
+        # counting (best effort, if there are leaked references for these
+        # objects themselves, it still won't fail)
+        self.client = None
+        self.test_name = None
+        source_client = None
+        self.loader.load()
+
+    def check_load_incremental(self):
         # New zone has 8 RRs
         # After 5, it should return False
-        self.assertFalse(loader.load_incremental(5))
+        self.assertFalse(self.loader.load_incremental(5))
         # New zone should not have been loaded yet
         self.check_zone_soa(ORIG_SOA_TXT)
 
         # After 5 more, it should return True (only having read 3)
-        self.assertTrue(loader.load_incremental(5))
+        self.assertTrue(self.loader.load_incremental(5))
         # New zone should now be loaded
         self.check_zone_soa(NEW_SOA_TXT)
 
         # And after that, it should throw
-        self.assertRaises(isc.dns.InvalidOperation, loader.load_incremental, 5)
+        self.assertRaises(isc.dns.InvalidOperation,
+                          self.loader.load_incremental, 5)
 
     def test_load_from_file_incremental(self):
         # Create loader and load the zone
-        loader = isc.datasrc.ZoneLoader(self.client, self.test_name,
-                                        self.test_file)
-        self.check_load_incremental(loader)
+        self.loader = isc.datasrc.ZoneLoader(self.client, self.test_name,
+                                             self.test_file)
+        self.check_load_incremental()
 
     def test_load_from_client_incremental(self):
         source_client = isc.datasrc.DataSourceClient('sqlite3',
                                                      DB_SOURCE_CLIENT_CONFIG)
-        loader = isc.datasrc.ZoneLoader(self.client, self.test_name,
-                                        source_client)
-        self.check_load_incremental(loader)
+        self.loader = isc.datasrc.ZoneLoader(self.client, self.test_name,
+                                             source_client)
+        self.check_load_incremental()
 
     def test_bad_file(self):
         self.check_zone_soa(ORIG_SOA_TXT)
-        loader = isc.datasrc.ZoneLoader(self.client, self.test_name,
-                                        'no such file')
-        self.assertRaises(isc.datasrc.MasterFileError, loader.load)
+        self.loader = isc.datasrc.ZoneLoader(self.client, self.test_name,
+                                             'no such file')
+        self.assertRaises(isc.datasrc.MasterFileError, self.loader.load)
         self.check_zone_soa(ORIG_SOA_TXT)
 
     def test_bad_file_incremental(self):
         self.check_zone_soa(ORIG_SOA_TXT)
-        loader = isc.datasrc.ZoneLoader(self.client, self.test_name,
-                                        'no such file')
+        self.loader = isc.datasrc.ZoneLoader(self.client, self.test_name,
+                                             'no such file')
         self.assertRaises(isc.datasrc.MasterFileError,
-                          loader.load_incremental, 1)
+                          self.loader.load_incremental, 1)
         self.check_zone_soa(ORIG_SOA_TXT)
 
     def test_no_such_zone_in_target(self):
@@ -138,11 +173,20 @@ class ZoneLoaderTests(unittest.TestCase):
                           self.test_file)
 
     def test_no_such_zone_in_source(self):
+        # Reuse a zone that exists in target but not in source
+        zone_name = isc.dns.Name("sql1.example.com")
         source_client = isc.datasrc.DataSourceClient('sqlite3',
                                                      DB_SOURCE_CLIENT_CONFIG)
+
+        # make sure the zone exists in the target
+        found, _ = self.client.find_zone(zone_name)
+        self.assertEqual(self.client.SUCCESS, found)
+        # And that it does not in the source
+        found, _ = source_client.find_zone(zone_name)
+        self.assertNotEqual(source_client.SUCCESS, found)
+
         self.assertRaises(isc.datasrc.Error, isc.datasrc.ZoneLoader,
-                          self.client, isc.dns.Name("unknownzone"),
-                          source_client)
+                          self.client, zone_name, source_client)
 
     def test_no_ds_load_support(self):
         # This may change in the future, but atm, the in-mem ds does
@@ -155,9 +199,9 @@ class ZoneLoaderTests(unittest.TestCase):
 
     def test_wrong_class_from_file(self):
         # If the file has wrong class, it is not detected until load time
-        loader = isc.datasrc.ZoneLoader(self.client, self.test_name,
-                                        self.test_file + '.ch')
-        self.assertRaises(isc.datasrc.MasterFileError, loader.load)
+        self.loader = isc.datasrc.ZoneLoader(self.client, self.test_name,
+                                             self.test_file + '.ch')
+        self.assertRaises(isc.datasrc.MasterFileError, self.loader.load)
 
     def test_wrong_class_from_client(self):
         # For ds->ds loading, wrong class is detected upon construction
diff --git a/src/lib/python/isc/datasrc/zone_loader_inc.cc b/src/lib/python/isc/datasrc/zone_loader_inc.cc
index 274c7bf..405ad1a 100644
--- a/src/lib/python/isc/datasrc/zone_loader_inc.cc
+++ b/src/lib/python/isc/datasrc/zone_loader_inc.cc
@@ -1,83 +1,116 @@
 namespace {
 const char* const ZoneLoader_doc = "\
-\n\
 Class to load data into a data source client.\n\
 \n\
-This is a small wrapper class that is able to load data into a data source.\n\
-It can load either from another data source or from a master file. The\n\
-purpose of the class is only to hold the state for incremental loading.\n\
+This is a small wrapper class that is able to load data into a data\n\
+source. It can load either from another data source or from a master\n\
+file. The purpose of the class is only to hold the state for\n\
+incremental loading.\n\
 \n\
 The old content of zone is discarded and no journal is stored.\n\
 \n\
-The constructor takes three arguments:\n\
-- The datasource (isc.datasrc.DataSourceClient) to load the zone into\n\
-- The name (isc.dns.Name) to load\n\
-- either a string (for a file) or another DataSourceClient to load from\n\
-\n\
-Upon construction, no loading is done yet.\n\
-\n\
-It can throw:\n\
-DataSourceError, in case the zone does not exist in destination.\n\
-    This class does not support creating brand new zones, only loading\n\
-    data into them. In case a new zone is needed, it must be created\n\
-    beforehand (with create_zone()).\n\
-    DataSourceError is also thrown in case the zone is not present in the\n\
-    source DataSourceClient, and in case of other possibly low-level\n\
-    errors.\n\
-InvalidParameter, in case the class of destination and source\n\
-    differs.\n\
-NotImplemented in case target data source client doesn't provide an updater\n\
-    or the source data source client doesn't provide an iterator.\n\
+ZoneLoader(destination, zone_name, master_file)\n\
+\n\
+    Constructor from master file.\n\
+\n\
+    This initializes the zone loader to load from a master file.\n\
+\n\
+    Exceptions:\n\
+      DataSourceError in case the zone does not exist in destination.\n\
+                 This class does not support creating brand new zones,\n\
+                 only loading data into them. In case a new zone is\n\
+                 needed, it must be created beforehand.\n\
+      DataSourceError in case of other (possibly low-level) errors,\n\
+                 such as read-only data source or database error.\n\
+\n\
+    Parameters:\n\
+      destination (isc.datasrc.DataSourceClient) The data source into\n\
+                  which the loaded data should go.\n\
+      zone_name   (isc.dns.Name) The origin of the zone. The class is\n\
+                  implicit in the destination.\n\
+      master_file (string) Path to the master file to read data from.\n\
+\n\
+ZoneLoader(destination, zone_name, source)\n\
+\n\
+    Constructor from another data source.\n\
+\n\
+    Parameters:\n\
+      destination (isc.datasrc.DataSourceClient) The data source into\n\
+                  which the loaded data should go.\n\
+      zone_name   (isc.dns.Name) The origin of the zone. The class is\n\
+                  implicit in the destination.\n\
+      source      (isc.datasrc.DataSourceClient) The data source from\n\
+                  which the data would be read.\n\
+\n\
+    Exceptions:\n\
+      InvalidParameter in case the class of destination and source\n\
+                 differs.\n\
+      NotImplemented in case the source data source client doesn't\n\
+                 provide an iterator.\n\
+      DataSourceError in case the zone does not exist in destination.\n\
+                 This class does not support creating brand new zones,\n\
+                 only loading data into them. In case a new zone is\n\
+                 needed, it must be created beforehand.\n\
+      DataSourceError in case the zone does not exist in the source.\n\
+      DataSourceError in case of other (possibly low-level) errors,\n\
+                 such as read-only data source or database error.\n\
+\n\
+    Parameters:\n\
+      destination The data source into which the loaded data should\n\
+                 go.\n\
+      zone_name  The origin of the zone.\n\
+      source     The data source from which the data would be read.\n\
 \n\
 ";
 
-const char* const ZoneLoader_loadIncremental_doc = "\
-\n\
-Load up to limit RRs.\n\
-\n\
-This performs a part of the loading. In case there's enough data in the\n\
-source, it copies limit RRs. It can copy less RRs during the final call\n\
-(when there's less than limit left).\n\
-\n\
-This can be called repeatedly until the whole zone is loaded, having\n\
-pauses in the loading for some purposes (for example reporting\n\
-progress).\n\
-\n\
-It has one parameter: limit (integer), The maximum allowed number of RRs\n\
-to be loaded during this call.\n\
+const char* const ZoneLoader_load_doc = "\
+load() -> None\n\
 \n\
-Returns True in case the loading is completed, and False if there's more\n\
-to load.\n\
+Perform the whole load.\n\
 \n\
-It can throw:\n\
-InvalidOperation, in case the loading was already completed before this\n\
-    call (by load() or by a loadIncremental that returned true).\n\
-DataSourceError, in case some error (possibly low-level) happens.\n\
-MasterFileError when the master_file is badly formatted or some similar\n\
-    problem is found when loading the master file.\n\
+This performs the whole loading operation. It may take a long time.\n\
 \n\
-Note: If the limit is exactly the number of RRs available to be loaded,\n\
-      the method still returns false and true'll be returned on the next\n\
-      call (which will load 0 RRs). This is because the end of iterator or\n\
-      master file is detected when reading past the end, not when the last\n\
-      one is read.\n\
+Exceptions:\n\
+  InvalidOperation in case the loading was already completed before\n\
+             this call.\n\
+  DataSourceError in case some error (possibly low-level) happens.\n\
+  MasterFileError when the master_file is badly formatted or some\n\
+             similar problem is found when loading the master file.\n\
 \n\
 ";
 
-const char* const ZoneLoader_load_doc = "\
-\n\
-Performs the entire load operation.\n\
+const char* const ZoneLoader_loadIncremental_doc = "\
+load_incremental(limit) -> bool\n\
 \n\
-Depending on zone size, this could take a long time.\n\
+Load up to limit RRs.\n\
 \n\
-This method has no parameters and does not return anything.\n\
+This performs a part of the loading. In case there's enough data in\n\
+the source, it copies limit RRs. It can copy less RRs during the final\n\
+call (when there's less than limit left).\n\
 \n\
-It can throw:\n\
-InvalidOperation, in case the loading was already completed before this call.\n\
-MasterFileError, when the master_file is badly formatted or some\n\
-                 similar problem is found when loading the master file.\n\
-DataSourceError, in case some error (possibly low-level) happens.\n\
+This can be called repeatedly until the whole zone is loaded, having\n\
+pauses in the loading for some purposes (for example reporting\n\
+progress).\n\
 \n\
+Exceptions:\n\
+  InvalidOperation in case the loading was already completed before\n\
+             this call (by load() or by a load_incremental that\n\
+             returned true).\n\
+  DataSourceError in case some error (possibly low-level) happens.\n\
+  MasterFileError when the master_file is badly formatted or some\n\
+             similar problem is found when loading the master file.\n\
+\n\
+Parameters:\n\
+  limit      (integer) The maximum allowed number of RRs to be\n\
+             loaded during this call.\n\
+\n\
+Return Value(s): True in case the loading is completed, false if\n\
+there's more to load.\n\
+\n\
+Note that if the limit is exactly the number of RRs available to be\n\
+loaded, the method will still return False, and True will be returned\n\
+on the next call (which will load 0 RRs). This is because the end of\n\
+iterator or master file is detected when reading past the end, not\n\
+when the last one is read.\n\
 ";
-
 } // unnamed namespace
diff --git a/src/lib/python/isc/datasrc/zone_loader_python.cc b/src/lib/python/isc/datasrc/zone_loader_python.cc
index b785d80..98264b3 100644
--- a/src/lib/python/isc/datasrc/zone_loader_python.cc
+++ b/src/lib/python/isc/datasrc/zone_loader_python.cc
@@ -35,16 +35,19 @@ using namespace std;
 using namespace isc::dns::python;
 using namespace isc::datasrc;
 using namespace isc::datasrc::python;
+using namespace isc::util::python;
 
 namespace {
 // The s_* Class simply covers one instantiation of the object
 class s_ZoneLoader : public PyObject {
 public:
-    s_ZoneLoader() : cppobj(NULL), client(NULL) {};
+    s_ZoneLoader() : cppobj(NULL), target_client(NULL), source_client(NULL)
+        {};
     ZoneLoader* cppobj;
-    // a zoneloader should not survive its associated client,
+    // a zoneloader should not survive its associated client(s),
     // so add a ref to it at init
-    PyObject* client;
+    PyObject* target_client;
+    PyObject* source_client;
 };
 
 // General creation and destruction
@@ -62,23 +65,35 @@ ZoneLoader_init(PyObject* po_self, PyObject* args, PyObject*) {
                           &po_target_client, &name_type, &po_name,
                           &datasourceclient_type, &po_source_client)
        ) {
+        PyErr_SetString(PyExc_TypeError,
+                        "Invalid arguments to ZoneLoader constructor, "
+                        "expects isc.datasrc.DataSourceClient, isc.dns.Name, "
+                        "and either a string or another DataSourceClient");
         return (-1);
     }
     PyErr_Clear();
     try {
+        // The associated objects must be alive during the lifetime
+        // of this instance, so incref them (through a container in case
+        // of exceptions in this method)
         Py_INCREF(po_target_client);
-        self->client = po_target_client;
+        PyObjectContainer target_client(po_target_client);
         if (po_source_client != NULL) {
+            // See above
+            Py_INCREF(po_source_client);
+            PyObjectContainer source_client(po_source_client);
             self->cppobj = new ZoneLoader(
                 PyDataSourceClient_ToDataSourceClient(po_target_client),
                 PyName_ToName(po_name),
                 PyDataSourceClient_ToDataSourceClient(po_source_client));
+            self->source_client = source_client.release();
         } else {
             self->cppobj = new ZoneLoader(
                 PyDataSourceClient_ToDataSourceClient(po_target_client),
                 PyName_ToName(po_name),
                 master_file);
         }
+        self->target_client = target_client.release();
         return (0);
     } catch (const isc::InvalidParameter& ivp) {
         PyErr_SetString(po_InvalidParameter, ivp.what());
@@ -100,8 +115,11 @@ ZoneLoader_destroy(PyObject* po_self) {
     s_ZoneLoader* self = static_cast<s_ZoneLoader*>(po_self);
     delete self->cppobj;
     self->cppobj = NULL;
-    if (self->client != NULL) {
-        Py_DECREF(self->client);
+    if (self->target_client != NULL) {
+        Py_DECREF(self->target_client);
+    }
+    if (self->source_client != NULL) {
+        Py_DECREF(self->source_client);
     }
     Py_TYPE(self)->tp_free(self);
 }
@@ -145,8 +163,7 @@ ZoneLoader_loadIncremental(PyObject* po_self, PyObject* args) {
         return (NULL);
     }
     try {
-        const bool complete = self->cppobj->loadIncremental(limit);
-        if (complete) {
+        if (self->cppobj->loadIncremental(limit)) {
             Py_RETURN_TRUE;
         } else {
             Py_RETURN_FALSE;



More information about the bind10-changes mailing list