Project

General

Profile

Bug #19674

make_tool does not check type

Added by David Adams over 1 year ago. Updated over 1 year ago.

Status:
Feedback
Priority:
Normal
Assignee:
Category:
-
Target version:
-
Start date:
04/13/2018
Due date:
% Done:

0%

Estimated time:
Occurs In:
Scope:
Internal
Experiment:
DUNE
SSI Package:
Duration:

Description

I find that art::make_tool<Type1>(myParamSet) returns a pointer to the object described by myParamSet for any Type1, i.e. there is no type checking. This leads to undesirable behavior including crashes if the object is not really Type1.

It would be preferable for make_tool to return null or raise an exception if the object is not of type Type1.

An example may be found in dunetpc/dune/ArtSupport/test/test_make_tool.cxx.

History

#1 Updated by Kyle Knoepfel over 1 year ago

  • Status changed from New to Feedback
  • Assignee set to Kyle Knoepfel

What you describe is correct. In order to provide the type-checking desired, additional constraints/prescriptions are necessary for the user to adopt. We will describe in more detail what the issues are in a future message.

#2 Updated by Kyle Knoepfel over 1 year ago

Whenever we load libraries via our own plugin manager, it is necessary to perform a hard cast on the extern C function that creates the plugin. So when you call art::make_tool<MyInterface>(ps), the following steps are performed:

  • Look for the shared object library matching the provided tool_type parameter in the provided ParameterSet ps.
  • Load the library into memory and search for the makeTool extern C function.
  • If the function has been located, its type is hard cast to std::unique_ptr<MyInterface>(fhicl::ParameterSet const&).
  • The hard cast function is then invoked.

It is the hard cast that erases any type safety.

This has not been a problem in the past since art is usually responsible for creating the plugins, and we make sure to avoid the problem by providing the correct types. However, since make_tool is intended to be user-facing, there are additional responsibilities that users must take to avoid this problem.

There are two options we can pursue:

  1. Keep things how they currently are, with the expectation that authors invoking make_tool must make sure that the correct types are specified for any given allowed ParameterSet configuration.
  2. Add additional interface that the user must invoke when defining the tool. The interface would be akin to the implementer/interface API that exists for services. Although more work for the user, it has the benefit of avoiding any disastrous failures similar to the ones you've encountered.

If you'd like to pursue option 2, then we would need to discuss this with the stakeholders to get agreement and determine priority. Please let us know.

#3 Updated by David Adams over 1 year ago

Kyle:

Thanks for working on this. Can we do both, i.e. keep the current interface so we don't break any existing code and provide a second type-safe interface?

For 2 are you talking about decorating the interface header with a base class and/or macro and adding dependency on art/canvas? I find it very attractive now that it has no such dependency or decoration and would prefer a solution that avoids those.

If it makes things easier, I would happily settle for a solution that only works for class tools whose interfaces have virtual methods.

#4 Updated by Kyle Knoepfel over 1 year ago

Our preference is to provide one or the other, however we will discuss the pros and cons of either approach at the next stakeholders meeting. We are also amenable to deprecating the type-unsafe interface for a time while users get accustomed to the type-safe interface.



Also available in: Atom PDF