Hello Willy,
On 05/24/2018 04:16 PM, Willy Tarreau wrote:
Interesting one! However it's not correct, it doesn't decrement the
refcount on the return 0 path so the problem remains when a data
type is looked up in a table where it is not stored. For example :
Nice catch. Thanks for pointing this out.
Given that all functions seem to be written the same way, I suggest
that we change the end and invert the !ptr condition to centralize
the release call. It would give this above :
ptr = stktable_data_ptr(t, ts, STKTABLE_DT_CONN_CUR);
if (ptr)
smp->data.u.sint = stktable_data_cast(ptr, conn_cur);
stktable_release(t, ts);
return !!ptr;
Could you please rework your patch to do this so that I can merge it ?
I have attached the latest patch with these changes.
Thanks,
-- Daniel
>From e47065ac6cc7478d21cfa00c5c45a0ae7cd412bf Mon Sep 17 00:00:00 2001
From: Daniel Corbett
Date: Sun, 27 May 2018 09:47:12 -0400
Subject: [PATCH] BUG/MEDIUM: stick-tables: Decrement ref_cnt in table_*
converters
When using table_* converters ref_cnt was incremented
and never decremented causing entries to not expire.
The root cause appears to be that stktable_lookup_key()
was called within all sample_conv_table_* functions which was
incrementing ref_cnt and not decrementing after completion.
Added stktable_release() to the end of each sample_conv_table_*
function and reworked the end logic to ensure that ref_cnt is
always decremented after use.
This should be backported to 1.8
---
src/stick_table.c | 170 +++---
1 file changed, 86 insertions(+), 84 deletions(-)
diff --git a/src/stick_table.c b/src/stick_table.c
index fcc6fe6..101a4e2 100644
--- a/src/stick_table.c
+++ b/src/stick_table.c
@@ -875,6 +875,7 @@ static int sample_conv_in_table(const struct arg *arg_p, struct sample *smp, voi
smp->data.type = SMP_T_BOOL;
smp->data.u.sint = !!ts;
smp->flags = SMP_F_VOL_TEST;
+ stktable_release(t, ts);
return 1;
}
@@ -907,12 +908,12 @@ static int sample_conv_table_bytes_in_rate(const struct arg *arg_p, struct sampl
return 1;
ptr = stktable_data_ptr(t, ts, STKTABLE_DT_BYTES_IN_RATE);
- if (!ptr)
- return 0; /* parameter not stored */
+ if (ptr)
+ smp->data.u.sint = read_freq_ctr_period(&stktable_data_cast(ptr, bytes_in_rate),
+ t->data_arg[STKTABLE_DT_BYTES_IN_RATE].u);
- smp->data.u.sint = read_freq_ctr_period(&stktable_data_cast(ptr, bytes_in_rate),
- t->data_arg[STKTABLE_DT_BYTES_IN_RATE].u);
- return 1;
+ stktable_release(t, ts);
+ return !!ptr;
}
/* Casts sample to the type of the table specified in arg(0), and looks
@@ -944,11 +945,11 @@ static int sample_conv_table_conn_cnt(const struct arg *arg_p, struct sample *sm
return 1;
ptr = stktable_data_ptr(t, ts, STKTABLE_DT_CONN_CNT);
- if (!ptr)
- return 0; /* parameter not stored */
+ if (ptr)
+ smp->data.u.sint = stktable_data_cast(ptr, conn_cnt);
- smp->data.u.sint = stktable_data_cast(ptr, conn_cnt);
- return 1;
+ stktable_release(t, ts);
+ return !!ptr;
}
/* Casts sample to the type of the table specified in arg(0), and looks
@@ -980,11 +981,11 @@ static int sample_conv_table_conn_cur(const struct arg *arg_p, struct sample *sm
return 1;
ptr = stktable_data_ptr(t, ts, STKTABLE_DT_CONN_CUR);
- if (!ptr)
- return 0; /* parameter not stored */
+ if (ptr)
+ smp->data.u.sint = stktable_data_cast(ptr, conn_cur);
- smp->data.u.sint = stktable_data_cast(ptr, conn_cur);
- return 1;
+ stktable_release(t, ts);
+ return !!ptr;
}
/* Casts sample to the type of the table specified in arg(0), and looks
@@ -1016,12 +1017,12 @@ static int sample_conv_table_conn_rate(const struct arg *arg_p, struct sample *s
return 1;
ptr = stktable_data_ptr(t, ts, STKTABLE_DT_CONN_RATE);
- if (!ptr)
- return 0; /* parameter not stored */
+ if (ptr)
+ smp->data.u.sint = read_freq_ctr_period(&stktable_data_cast(ptr, conn_rate),
+ t->data_arg[STKTABLE_DT_CONN_RATE].u);
- smp->data.u.sint = read_freq_ctr_period(&stktable_data_cast(ptr, conn_rate),
- t->data_arg[STKTABLE_DT_CONN_RATE].u);
- return 1;
+ stktable_release(t, ts);
+ return !!ptr;
}
/* Casts sample to the type of the table specified in arg(0), and looks
@@ -1053,12 +1054,12 @@ static int sample_conv_table_bytes_out_rate(const struct arg *arg_p, struct samp
return 1;
ptr = stktable_data_ptr(t, ts, STKTABLE_DT_BYTES_OUT_RATE);
- if (!ptr)
- return 0; /* parameter not stored */
+ if (ptr)
+ smp->data.u.sint = read_freq_ctr_period(&stktable_data_cast(ptr, bytes_out_rate),
+ t->data_arg[STKTABLE_DT_BYTES_OUT_RATE].u);
- smp->data.u.sint = read_freq_ctr_period(&stktable_data_cast(ptr, bytes_out_rate),
- t->data_arg[STKTABLE_DT_BYTES_OUT_RATE].u);
- return 1;
+ stktable_release(
