Bug #19674
make_tool does not check type
0%
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 almost 3 years 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 almost 3 years 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 providedParameterSet
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:
- 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 allowedParameterSet
configuration. - 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 almost 3 years 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 almost 3 years 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.