Skip to content

Commit

Permalink
Added the function name to the result cache key
Browse files Browse the repository at this point in the history
The function cache only used the args values, which in some cases could
lead to mixing results from different functions with the same arguments.

Ticket: CFE-4244
Changelog: Cashed policy function results now take into account number of arguments and function name.
Signed-off-by: Lars Erik Wik <lars.erik.wik@northern.tech>
Co-authored-by: Alexis Mousset <alexis.mousset@rudder.io>
  • Loading branch information
amousset authored and larsewi committed Sep 13, 2023
1 parent 57745c7 commit 29e60a9
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 2 deletions.
18 changes: 16 additions & 2 deletions libpromises/eval_context.c
Original file line number Diff line number Diff line change
Expand Up @@ -2837,7 +2837,14 @@ bool EvalContextFunctionCacheGet(const EvalContext *ctx,
return false;
}

Rval *rval = FuncCacheMapGet(ctx->function_cache, args);
// The cache key is made of the function name and all args values
Rlist *args_copy = RlistCopy(args);
assert(fp != NULL);
assert(fp->name != NULL);
assert(ctx != NULL);
Rlist *key = RlistPrepend(&args_copy, fp->name, RVAL_TYPE_SCALAR);
Rval *rval = FuncCacheMapGet(ctx->function_cache, key);
RlistDestroy(key);
if (rval)
{
if (rval_out)
Expand All @@ -2863,7 +2870,14 @@ void EvalContextFunctionCachePut(EvalContext *ctx,

Rval *rval_copy = xmalloc(sizeof(Rval));
*rval_copy = RvalCopy(*rval);
FuncCacheMapInsert(ctx->function_cache, RlistCopy(args), rval_copy);

Rlist *args_copy = RlistCopy(args);
assert(fp != NULL);
assert(fp->name != NULL);
assert(ctx != NULL);
Rlist *key = RlistPrepend(&args_copy, fp->name, RVAL_TYPE_SCALAR);

FuncCacheMapInsert(ctx->function_cache, key, rval_copy);
}

/* cfPS and associated machinery */
Expand Down
51 changes: 51 additions & 0 deletions tests/acceptance/01_vars/02_functions/cache_name.cf
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
#######################################################
#
# Test that the function result cache checks function name
#
#######################################################

body common control
{
inputs => { "../../default.cf.sub" };
bundlesequence => { default("$(this.promise_filename)") };
version => "1.0";
}

#######################################################


bundle agent init
{
vars:
"agent_regex" string => ".*cf-agent.*";
}

#######################################################

bundle common test
{
meta:
"description" -> { "CFE-4244" }
string => "Test that the function result cache checks function name";

vars:
"res1" data => findprocesses("${init.agent_regex}");

classes:
# must not reuse result from previous line
# is reused, produces a type error
"_pass" expression => processexists("${init.agent_regex}");
}


#######################################################

bundle agent check
{
methods:
_pass::
"pass" usebundle => dcs_pass("$(this.promise_filename)");

!_pass::
"pass" usebundle => dcs_fail("$(this.promise_filename)");
}

0 comments on commit 29e60a9

Please sign in to comment.