Opened 4 years ago

Closed 3 years ago

#4532 closed defect (fixed)

a library load failure has no consequence

Reported by: fdupont Owned by: fdupont
Priority: low Milestone: Kea1.2
Component: hooks Version: git
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DHCP Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

When a library load() fails (i.e., doesn't return 0) it is ignored. This seems a bad side effect on changes about library reloads.

Subtickets

Change History (9)

comment:1 Changed 4 years ago by hschempf

  • Milestone changed from Kea-proposed to Kea1.1

Per June 30 team meeting, accept 1.1 under bug fixing. Est = .5d

comment:2 Changed 4 years ago by fdupont

Currently:

  • HooksLibrariesParser::commit () calls loadLibraries but ignores its return (and does not return something itself).
  • commandLibReloadHandler, the handler of "libreload" calls loadLibraries and checks its return.
  • the handler of "config-reload" used both at startup and by the command channel calls the hook parser commit() as the last step of committing a configuration.

If we change the commit() method to raise an error any configuration with a library which does not return 0 on load() will fail to be rejected. Perhaps it is too much?

comment:3 Changed 4 years ago by stephen

The parser executes in two steps, a build stage - which checks that everything is OK - and a commit stage that implements the changes.

In the Hooks parser, the build step validates a library by calling LibraryManager::validateLibrary(). This attempts to open the library (it calls dlopen()), checks whether there is a version() function and if so, calls it and checks the value returned. Regardless of the success or failure of this step, the library is closed (and the close status ignored). As a library opened multiple times with dlopen() has to be closed the same number of times before it is unloaded, the build stage does not affect any loaded libraries other than incrementing and decrementing a reference count associated with them.

The commit step then closes all the currently open hooks libraries (at this point the libraries should only have been opened once, so closing them unloads them) and opens the new set. The logic of ignoring the return status in the commit step is that Kea has just opened (and closed) the all the libraries it is now loading, so the likelihood of error is small.

But if there is an error, what should commit() do? It is too late to reject the configuration as commit() for other components will have already executed and the running system updated. (Besides, what if the rollback - if possible - generated an error when accessing the libraries?). About all we can do is to add a message at this point summarizing that the configuration committal has failed and that not all libraries may have been loaded: errors output by the hooks code prior to that message will give a detailed reason.

comment:4 Changed 4 years ago by tomek

  • Milestone changed from Kea1.1 to Kea-1.1-final

comment:5 Changed 4 years ago by tomek

  • Milestone changed from Kea-1.1-final to Kea1.1-final

Milestone renamed

comment:6 Changed 3 years ago by tomek

  • Milestone changed from Kea1.1-final to Kea1.2

comment:7 Changed 3 years ago by fdupont

If the bug fix is postponed to 1.2 we have to add this in the known issues for 1.1.

comment:8 Changed 3 years ago by fdupont

  • Owner set to fdupont
  • Status changed from new to accepted

Revisiting it as the parser was migrated and what it does changed...

comment:9 Changed 3 years ago by fdupont

  • Resolution set to fixed
  • Status changed from accepted to closed

Config and now re-config check that an invalid library (e.g. NOT_PRESENT_LIBRARY) gives an error.
Closing.

Note: See TracTickets for help on using tickets.