| Maggie Dreyer What is meant by being "thread safe" in this context? Asking because the puppet runtime is not thread safe so what is the purpose of making 3x functions "thread safe"? (I think what is really asked is how to make 3x functions not taint loaded classes which is what makes them leak across environments unless code is completely reloaded). How 3x functions work: The 3x functions use an anonymous Module and methods are created in this module using the names function_<name> and real_function_<name>. This module is then picked up in the implementation of Scope - it calls a method named "extend_with_function_module". Scope is extended with the anonymous module for the "root" environment (unclear if this is meaningful as I think that only held the logging functions and those are 4x), and the anon module for the "current env". The effect of this is that each instance of Scope will have the function_<name> and real_function_<name> available as callable methods. The 3.x function logic is in lib/puppet/parser/functions.rb and in lib/puppet/parser/scope.rb - the ruby_legacy_function_instantiator is what makes 3x loadable the 4x way. There has been several changes to 3x functions - there used to be memory leaks related to the mapping from env to anonymous module - this is now (since Puppet 4.4 - PUP-5813) done with an adapter that associates the information with the env's lifecycle. I suspect that change also fix some other potential cases of leaking. In Puppet 6 when we started loading 3x via ruby_the legacy_function_instantiator we also added a scan that the loaded 3x function is free from dangerous constructs. The constructs it is protecting against would if allowed taint the Scope class as opposed to the anonymous module. So what remains to be done here for safety?
- it is a problem that code can load 3x functions via code paths other than via 4x loaders - this means that they can have dangerous constructs. I would like remove those code paths.
- I don't think that the Scope extend of the anonymous module causes leakage as it is tied to a specific instance of scope
What I think should also be done:
- We should get rid of the use of anonymous modules as it means that the set of available methods for Scope changes as functions are loaded. I suspect that to have negative impact on performance (destroys possible optimization) - but this is only a suspicion.
- Investigate if the extend of the root environment's anon module is really needed - if we do not remove this kind of extend all together.
- We should get rid of the support to call function_<name> and real_function_<name> as those only works for calling 3x functions. This is a problem because 3x logic using those can end up calling a 3x version of a function when there is a 4x function shadowing it (if the called 3x function was loaded via a different code path than via the legacy instantiator). (The right way to call functions is to use Scope.call_function.
- When a 3x function is being evaluated self is bound to an instance of Scope - this opens up for mistakes in the implementation of a function that can lead to dangerous constructs i.e. tainting the Scope class. (It is also bad because the entire Scope implementation is available as "API" and means we cannot change anything in it without risk of breaking stuff). This is hard to fix since it is expected that code in 3x functions can call scope methods directly (e.g. getvar).
In my first attempt to let 4x loaders load 3x functions I took a more radical approach (than what is now used in ruby_legacy_function_instantiator) - basically diverting the call that creates the 3x and taking its body of code as the body for a "dispatched to" method in a 4x function instance. That fixed most of the problems, but naturally also modified the context in which the 3.x function body was evaluated in and thus requiring changes to function_<name> calls, and direct calls to methods on Scope. We could use this method as it would support all 3x functions that does not call methods on scope and that does not call other functions - the remainder would need to be migrated to 4.x. What we really want to do is deprecate and remove 3x functions altogether (although my radical approach would be fine as that basically means we would only have 4x functions while we support the 3x API of creating them). Once we are tid of all the funny stuff wrt. Scope, we can make a much better Scope implementation (smaller, faster, and that would support real lambdas). Hope the above helps. |
