From: Stephen Morris Date: Wed, 29 May 2013 23:51:53 +0000 (+0100) Subject: [2974] Added callbacks to context_create and context_destroy hooks X-Git-Tag: bind10-1.2.0beta1-release~402^2~13 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=64bc2cfab72d67bf9e4f3cd19fd6ebb586208e67;p=thirdparty%2Fkea.git [2974] Added callbacks to context_create and context_destroy hooks These are called in the CalloutHandle's constructor and destructor. --- diff --git a/src/lib/util/hooks/callout_handle.cc b/src/lib/util/hooks/callout_handle.cc index 6d2f8674d3..38058e9ece 100644 --- a/src/lib/util/hooks/callout_handle.cc +++ b/src/lib/util/hooks/callout_handle.cc @@ -14,6 +14,7 @@ #include #include +#include #include #include @@ -25,6 +26,43 @@ using namespace isc::util; namespace isc { namespace util { +// Constructor. +CalloutHandle::CalloutHandle( + boost::shared_ptr& collection) + : arguments_(), context_collection_(), library_collection_(collection), + skip_(false) { + + // Call the "context_create" hook. We should be OK doing this - although + // the constructor has not finished running, all the member variables + // have been created. + int status = library_collection_->callCallouts(ServerHooks::CONTEXT_CREATE, + *this); + if (status > 0) { + isc_throw(ContextCreateFail, "error code of " << status << " returned " + "from context_create callout during the creation of a " + "ContextHandle object"); + } +} + +// Destructor +CalloutHandle::~CalloutHandle() { + + // Call the "context_destroy" hook. We should be OK doing this - although + // the destructor is being called, all the member variables are still in + // existence. + int status = library_collection_->callCallouts(ServerHooks::CONTEXT_DESTROY, + *this); + if (status > 0) { + // An exception is thrown on failure. This may be severe, but if + // none is thrown a resoucre leak in a library (signalled by the + // context_destroy callout returning an error) may be difficult to + // trace. + isc_throw(ContextDestroyFail, "error code of " << status << " returned " + "from context_destroy callout during the destruction of a " + "ContextHandle object"); + } +} + // Return the name of all argument items. vector diff --git a/src/lib/util/hooks/callout_handle.h b/src/lib/util/hooks/callout_handle.h index 35d0e31408..3d09c457c9 100644 --- a/src/lib/util/hooks/callout_handle.h +++ b/src/lib/util/hooks/callout_handle.h @@ -49,6 +49,28 @@ public: isc::Exception(file, line, what) {} }; +/// @brief Context creation failure +/// +/// Thrown if, during the running of the constructor, the call to the +/// context_create hook returns an error. + +class ContextCreateFail : public Exception { +public: + ContextCreateFail(const char* file, size_t line, const char* what) : + isc::Exception(file, line, what) {} +}; + +/// @brief Context destruction failure +/// +/// Thrown if, during the running of the desstructor, the call to the +/// context_destroy hook returns an error. + +class ContextDestroyFail : public Exception { +public: + ContextDestroyFail(const char* file, size_t line, const char* what) : + isc::Exception(file, line, what) {} +}; + // Forward declaration of the library handle and related collection classes. class LibraryHandle; @@ -110,11 +132,16 @@ public: /// @brief Constructor /// + /// Creates the object and calls the callouts on the "context_create" + /// hook. + /// /// @param manager Pointer to the collection of library handles. - CalloutHandle(boost::shared_ptr& collection) - : arguments_(), context_collection_(), library_collection_(collection), - skip_(false) - {} + CalloutHandle(boost::shared_ptr& collection); + + /// @brief Destructor + /// + /// Calls the context_destroy callback to release any per-packet context. + ~CalloutHandle(); /// @brief Set argument /// diff --git a/src/lib/util/tests/handles_unittest.cc b/src/lib/util/tests/handles_unittest.cc index 4759824136..cb0ac1b3f3 100644 --- a/src/lib/util/tests/handles_unittest.cc +++ b/src/lib/util/tests/handles_unittest.cc @@ -57,20 +57,24 @@ namespace { // the type of data manipulated). // // For the string item, each callout shifts data to the left and inserts its own -// data. The aata is a string of the form "nmwc", where "n" is the number of +// data. The data is a string of the form "nmwc", where "n" is the number of // the library, "m" is the callout number and "w" is an indication of what is // being altered (library context ["x"] or callout context ["c"]) and "y" is the -// indication of what callout was passed as an argument ("x" or "b"). ("x" is -// used instead of "l" to indicate that library context is being altered since -// in the results, these single characters will be mixed with digits and "l" -// " looks too much like "1".) Hence we have: +// indication of what callout was passed as an argument ("a" or "b" - "" is +// entered if no argument is supplied). ("x" is used instead of "l" to indicate +// that library context is being altered since in the results, these single +// characters will be mixed with digits and "l" " looks too much like "1".) +// Hence we have: // // - "xa" if library context is being altered from a callout made with the -// first callout handle passed as argument. +// first callout handle indicator passed as argument. // - "xb" if library context is being altered from a callout made with the -// second callout handle passed as argument. +// second callout handle indicator passed as argument. +// - "x" if library context is being altered and no argument is set. // - "ca" if the first callout handle's context is being manipulated. // - "cb" if the second callout handle's context is being manipulated. +// - "c" if the a callout handle's context is being manipulated and it is not +// possible to identify the callout handle. // // For simplicity, and to cut down the number of functions actually written, // the callout indicator ("a" or "b") ) used in the in the CalloutHandle @@ -84,11 +88,11 @@ namespace { // 1000 * library number + 100 * callout_number + 10 * lib/callout + indicator // // where "lib/callout" is 1 if a library context is updated and 2 if a -// a callout context is changed. "indicator" is 1 for callout a and 2 for -// callout b. This scheme gives a direct correspondence between the characters -// appended to the string context item and the amount by which the integers -// context item is incremented. For example, the string "21cb" corresponds to -// a value of 2122. +// callout context is changed. "indicator" is 1 for callout a, 2 for callout +// b and 0 if unknown. This scheme gives a direct correspondence between the +// characters appended to the string context item and the amount by which the +// integer context item is incremented. For example, the string "21cb" +// corresponds to a value of 2122. // // Although this gives less information than the string value, the reasons for // using it are: @@ -144,9 +148,15 @@ static void zero_results() { int execute(CalloutHandle& callout_handle, int library_num, int callout_num) { - // Obtain the callout handle indicator. - string indicator; - callout_handle.getArgument("string", indicator); + // Obtain the callout handle indicator and set a number for it. + string sindicator = ""; + int indicator = 0; + try { + callout_handle.getArgument("string", sindicator); + indicator = (sindicator == "a") ? 1 : 2; + } catch (const NoSuchArgument&) { + indicator = 0; + } // Create the basic data to be appended to the context value. int idata = 1000 * library_num + 100 * callout_num; @@ -171,10 +181,10 @@ execute(CalloutHandle& callout_handle, int library_num, int callout_num) { // Update the context value with the library/callout indication (and the // suffix "x" to denote library) and set it. - string_value += (sdata + string("x") + indicator); + string_value += (sdata + string("x") + sindicator); callout_handle.getLibraryHandle().setContext("string", string_value); - int_value += (idata + 10 + (indicator == "a" ? 1 : 2)); + int_value += (idata + 10 + indicator); callout_handle.getLibraryHandle().setContext("int", int_value); // Get the context data. As before, this will not exist for the first @@ -196,10 +206,10 @@ execute(CalloutHandle& callout_handle, int library_num, int callout_num) { } // Update the values and set them. - string_value += (sdata + string("c") + indicator); + string_value += (sdata + string("c") + sindicator); callout_handle.setContext("string", string_value); - int_value += (idata + 20 + (indicator == "a" ? 1 : 2)); + int_value += (idata + 20 + indicator); callout_handle.setContext("int", int_value); return (0); @@ -295,7 +305,7 @@ print3(CalloutHandle& callout_handle) { TEST(HandlesTest, ContextAccessCheck) { // Create the LibraryHandleCollection with a set of four callouts - // (the test does not use the ContextCreate and ContextDestroy callouts.) + // (the test does not use the context_create and context_destroy callouts.) boost::shared_ptr server_hooks(new ServerHooks()); const int one_index = server_hooks->registerHook("one"); @@ -494,7 +504,7 @@ printContextNames3(CalloutHandle& handle) { TEST(HandlesTest, ContextDeletionCheck) { // Create the LibraryHandleCollection with a set of four callouts - // (the test does not use the ContextCreate and ContextDestroy callouts.) + // (the test does not use the context_create and context_destroy callouts.) boost::shared_ptr server_hooks(new ServerHooks()); const int one_index = server_hooks->registerHook("one"); @@ -618,5 +628,67 @@ TEST(HandlesTest, ContextDeletionCheck) { EXPECT_EQ(0, getItemNames(2).size()); } +// Tests that the CalloutHandle's constructor and destructor call the +// context_create and context_destroy callbacks (if registered). For +// simplicity, we'll use the same callout functions as used above, plus +// the following that returns an error: + +int returnError(CalloutHandle&) { + return (1); +} + +TEST(HandlesTest, ConstructionDestructionCallouts) { + // Create the LibraryHandleCollection comprising two LibraryHandles. + // Register callouts for the context_create and context_destroy hooks. + + boost::shared_ptr server_hooks(new ServerHooks()); + + // Create the library handle collection and the library handles. + boost::shared_ptr + collection(new LibraryHandleCollection()); + + boost::shared_ptr handle(new LibraryHandle(server_hooks)); + handle->registerCallout("context_create", callout11); + handle->registerCallout("context_create", print1); + handle->registerCallout("context_destroy", callout12); + handle->registerCallout("context_destroy", print1); + collection->addLibraryHandle(handle); + + // Create the CalloutHandle and check that the constructor callout + // has run. + zero_results(); + boost::shared_ptr + callout_handle(new CalloutHandle(collection)); + + EXPECT_EQ("11x", resultLibraryString(0)); + EXPECT_EQ(1110, resultLibraryInt(0)); + + // Check that the destructor callout runs. Note that the "print1" callout + // didn't destroy the library context - it only copied it to where it + // could be examined. As a result, the destructor callout appends its + // elements to the constructor's values and the result is printed. + zero_results(); + callout_handle.reset(); + + EXPECT_EQ("11x12x", resultLibraryString(0)); + EXPECT_EQ((1110 + 1210), resultLibraryInt(0)); + + // Test that the destructor throws an error if the context_destroy + // callout returns an error. + handle->registerCallout("context_destroy", returnError); + callout_handle.reset(new CalloutHandle(collection)); + EXPECT_THROW(callout_handle.reset(), ContextDestroyFail); + + // We don't know what callout_handle is pointing to - it could be to a + // half-destroyed object - so use a new CalloutHandle to test construction + // failure. + handle->registerCallout("context_create", returnError); + boost::shared_ptr callout_handle2; + EXPECT_THROW(callout_handle2.reset(new CalloutHandle(collection)), + ContextCreateFail); + +} + + } // Anonymous namespace