]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MINOR: lua/vars: prevent get_var() from allocating a new name
authorWilly Tarreau <w@1wt.eu>
Thu, 13 May 2021 11:30:12 +0000 (13:30 +0200)
committerWilly Tarreau <w@1wt.eu>
Thu, 13 May 2021 11:44:32 +0000 (13:44 +0200)
Variable names are stored into a unified list that helps compare them
just based on a pointer instead of duplicating their name with every
variable. This is convenient for those declared in the configuration
but this started to cause issues with Lua when random names would be
created upon each access, eating lots of memory and CPU for lookups,
hence the work in 2.2 with commit 4e172c93f ("MEDIUM: lua: Add
`ifexist` parameter to `set_var`") to address this.

But there remains a corner case with get_var(), which also allocates
a new variables. After a bit of thinking and discussion, it never
makes sense to allocate a new variable name on get_var():
  - if the name exists, it will be returned ;
  - if it does not exist, then the only way for it to appear will
    be that some code calls set_var() on it
  - a call to get_var() after a careful set_var(ifexist) ruins the
    effort on set_var().

For this reason, this patch addresses this issue by making sure that
get_var() will never cause a variable to be allocated. This is done
by modifying vars_get_by_name() to always call register_name() with
alloc=0, since vars_get_by_name() is exclusively used by Lua and the
new CLI's "get/set var" which also benefit from this protection.

It probably makes sense to backport this as far as 2.2 after some
observation period and feedback from users.

For more context and discussions about the issues this was causing,
see https://www.mail-archive.com/haproxy@formilux.org/msg40451.html
and in issue #664.

src/vars.c

index 996141f5db9448a3dc58b5d213f373c2decbfb78..15dcb3c3d1d7d12a1de0dfeb549586dd06bdc5d6 100644 (file)
@@ -583,7 +583,7 @@ int vars_get_by_name(const char *name, size_t len, struct sample *smp)
        enum vars_scope scope;
 
        /* Resolve name and scope. */
-       name = register_name(name, len, &scope, 1, NULL);
+       name = register_name(name, len, &scope, 0, NULL);
        if (!name)
                return 0;