[#1282] Updated doc

diff --git a/src/lib/hooks/hooks_component_developer.dox b/src/lib/hooks/hooks_component_developer.dox
index 779a5408ff4bddef3e77297f3a5ef22b804f15d2..baeb5ef94ec9885fbdd6e8772f3d57e1e47fdb54 100644 (file)
--- a/src/lib/hooks/hooks_component_developer.dox
+++ b/src/lib/hooks/hooks_component_developer.dox
@@ -354,6 +354,14 @@ pointer to the returned CalloutHandle, so it gets destroyed when the
 shared pointer to it is cleared or destroyed.  However, this may change
 in a future version.)
 
+When the handle is not created locally it is not destroyed so it can
+keep ownership on arguments. In such case the code must call @c
+isc::hooks::CalloutHandle::deleteAllArguments or simply use the RAII
+helper @c isc::hooks::ScopedCalloutHandleState as in:
+@code
+    CalloutHandlePtr handle_ptr = getCalloutHandle(query);
+    ScopedCalloutHandleState state(handle_ptr);
+@endcode
 
 @subsection hooksComponentCallingCallout Calling the Callout
 
@@ -394,7 +402,6 @@ if (HooksManager::calloutsPresent(lease_hook_index)) {
 }
 @endcode
 
-
 @section hooksComponentLoadLibraries Loading the User Libraries
 
 Once hooks are defined, all the hooks code described above will
@@ -421,11 +428,23 @@ error messages will have been logged in the latter case, the status
 being more to allow the developer to decide whether the execution
 should proceed in such circumstances.
 
-If @c loadLibraries() is called a second or subsequent time (as a result
-of a reconfiguration), all existing libraries are unloaded and a new
-set loaded.  Libraries can be explicitly unloaded either by calling
-@c isc::hooks::HooksManager::unloadLibraries() or by calling
-@c loadLibraries() with an empty vector as an argument.
+Before @c loadLibraries() can be called a second or subsequent time
+(as a result of a reconfiguration), all existing libraries must be
+successfully unloaded. If a library stays in memory from a programming
+bug in Kea (for instance when no libraries were loaded) or in a
+library (@ref hooksMemoryManagement) @c loadLibraries() throws a not
+recoverable error.
+
+Unloading is done in two phases since Kea version 1.7.10:
+
+- call to @c isc::hooks::HooksManager::prepareUnloadLibraries() which
+calls all unload() entry points and deregisters callout points.
+
+- call to @c isc::hooks::HooksManager::unloadLibraries() even when
+the prepare failed.
+
+If a failure of @c unloadLibraries() is ignored any call to @c loadLibraries()
+will throw.
 
 
 @subsection hooksComponentUnloadIssues Unload and Reload Issues
@@ -466,6 +485,13 @@ suspending all processing of incoming requests until all currently
 executing requests have completed and data object destroyed, reloading
 the libraries, then resuming processing.
 
+Since Kea 1.7.10 the unload() entry point is called as the first phase
+of unloading. This gives more chance to hooks writer to perform
+necessary cleanup actions so the second phase, memory unmapping
+can safely happen. The @c isc::hooks::unloadLibraries() function
+was updated too to return false when at least one active callout
+handle remained.
+
 
 @section hooksComponentCallouts Component-Defined Callouts
 

diff --git a/src/lib/hooks/hooks_maintenance.dox b/src/lib/hooks/hooks_maintenance.dox
index 7e32918bdd5685cf17d4f3dfd8c74d0ad1588415..75e122795af6dc01f5ea70e68b67466994e29f83 100644 (file)
--- a/src/lib/hooks/hooks_maintenance.dox
+++ b/src/lib/hooks/hooks_maintenance.dox
@@ -1,4 +1,4 @@
-// Copyright (C) 2013-2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2020 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -264,6 +264,13 @@
  until all currently executing requests have completed and data object
  destroyed, reloading the libraries, then resuming processing.
 
+ Since Kea 1.7.10 the unload() entry point is called as the first phase
+ of unloading. This gives more chance to hooks writer to perform
+ necessary cleanup actions so the second phase, memory unmapping
+ can safely happen. The @c isc::hooks::unloadLibraries() function
+ was updated too to return false when at least one active callout
+ handle remained.
+
  @subsection hooksmgStaticLinking Hooks and Statically-Linked Kea
 
  Kea has the configuration option to allow static linking.  What this

diff --git a/src/lib/hooks/hooks_user.dox b/src/lib/hooks/hooks_user.dox
index 7f2ff8caab7b7b519c3a3f65c94c37ef99c8a257..be8ec31a12e579121a0511343d3ece1811563c75 100644 (file)
--- a/src/lib/hooks/hooks_user.dox
+++ b/src/lib/hooks/hooks_user.dox
@@ -243,9 +243,9 @@ in this example, the argument specification omits the variable name
 (whilst retaining the type) to avoid an "unused variable" compiler
 warning. (The LibraryHandle and its use is discussed in the section
 @ref hooksdgLibraryHandle.)
-- In the current version of the hooks framework, it is not possible to pass
+- In the initial version of the hooks framework, it was not possible to pass
 any configuration information to the "load" function.  The name of the log
-file must therefore be hard-coded as an absolute path name or communicated
+file had therefore to be hard-coded as an absolute path name or communicated
 to the user code by some other means.
 - "load" must return 0 on success and non-zero on error.  The hooks framework
 will abandon the loading of the library if "load" returns an error status.
@@ -1525,6 +1525,51 @@ which are dedicated to isc::data::Element testing and src/lib/hooks/tests/callou
 which is an example library used in testing. This library expects exactly 3 parameters:
 svalue (which is a string), ivalue (which is an integer) and bvalue (which is a boolean).
 
+@subsection hooksMemoryManagement Memory Management Considerations for Hooks Writer
+
+Both Kea server memory space and hook library memory space share a common
+address space between the opening of the hook (call to dlopen() as the first
+phase of the hook library loading) and the closing of the hook (call to
+dlclose() as the last phase of the hook library unloading). There are
+pointers between the two memory spaces with at least two bad consequences
+when they are not correctly managed:
+
+- Kea uses shared pointers for its objects. If the hook ownership keeps
+ownership of an object this object will be never destroyed leading to
+a trivial memory leak. Some care is recommended when the the hook library
+uses a garbage collector to not postpone releases of no longer used
+objects. Cycles should be avoided too for instance using weak pointers.
+Of course at the opposite if a Kea object is needed ownership on it must
+be kept in order to not get a dangling pointer when it will be destroyed
+at the end of its last reference lifetime.
+
+- Kea can take some pointers to the hook library memory space for instance
+when a hook object is registered. If these pointers are not destroyed
+before the hook library memory space is unmapped by dlclose() this likely
+leads to a crash.
+
+Communication between Kea code and hook library code is provided by
+callout handles. For callout points related to a packet the callout
+handle is associated with the packet, this allows for instance to
+get the same callout handle for all callout points called during
+processing of a query.
+
+Hook libraries are closed i.e. hook library memory spaces are unmapped
+only when there is no active callout handles. This enforces a correct
+behavior at two conditions:
+
+- there is no "wild" dangling pointers, for instance no registered
+objects.
+
+- this can happen i.e. the hook library does not keep a shared pointer
+to a query packet.
+
+To allow hook writers to fulfill these two conditions the unload() entry
+point is called in the first phase of the unloading process since Kea
+version 1.7.10. For instance if the hook library uses the PIMPL code
+pattern the unload() entry point must reset the pointer to the
+hook library implementation.
+
 @subsection hooksMultiThreading Multi-Threading Considerations for Hooks Writers
 
 Multi-threading programming in C++ is not easy. For instance STL containers