mirrors/haproxy
1
0
Fork 0
mirror of https://git.haproxy.org/git/haproxy.git/ synced 2025-08-06 07:07:04 +02:00
Code Issues Projects Releases Wiki Activity

MINOR: stick-table: Always decrement ref count before killing a session

Browse Source 
Guarded functions to kill a sticky session, stksess_kill()
stksess_kill_if_expired(), may or may not decrement and test its reference
counter before really killing it. This depends on a parameter. If it is set
to non-zero value, the ref count is decremented and if it falls to zero, the
session is killed. Otherwise, if this parameter is equal to zero, the
session is killed, regardless the ref count value.

In the code, these functions are always called with a non-zero parameter and
the ref count is always decremented and tested. So, there is no reason to
still have a special case. Especially because it is not really easy to say
if it is supported or not. Does it mean it is possible to kill a sticky
session while it is still referenced somewhere ? probably not. So, does it
mean it is possible to kill a unreferenced session ? This case may be
problematic because the session is accessed outside of any lock and thus may
be released by another thread because it is unreferenced. Enlarging scope of
the lock to avoid any issue is possible but it is a bit of shame to do so
because there is no usage for now.

The best is to simplify the API and remove this case. Now, stksess_kill()
and stksess_kill_if_expired() functions always decrement and test the ref
count before killing a sticky session.
This commit is contained in:
Christopher Faulet 2024-06-26 14:45:24 +02:00
parent 9357873641
commit ad946a704d
4 changed files with 24 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
include/haproxy/session.h
View File

@ -78,7 +78,7 @@ static inline void session_store_counters(struct session *sess)
}
stkctr_set_entry(stkctr, NULL);
stksess_kill_if_expired(stkctr->table, ts, 1);
stksess_kill_if_expired(stkctr->table, ts);
}
}

16
include/haproxy/stick_table.h
View File

@ -45,7 +45,7 @@ struct stktable *stktable_find_by_name(const char *name);
struct stksess *stksess_new(struct stktable *t, struct stktable_key *key);
void stksess_setkey(struct stktable *t, struct stksess *ts, struct stktable_key *key);
void stksess_free(struct stktable *t, struct stksess *ts);
int stksess_kill(struct stktable *t, struct stksess *ts, int decrefcount);
int stksess_kill(struct stktable *t, struct stksess *ts);
int stktable_get_key_shard(struct stktable *t, const void *key, size_t len);
int stktable_init(struct stktable *t, char **err_msg);
@ -215,7 +215,15 @@ static inline int __stksess_kill_if_expired(struct stktable *t, struct stksess *
return 0;
}
static inline void stksess_kill_if_expired(struct stktable *t, struct stksess *ts, int decrefcnt)
/*
* Decrease the refcount of a stksess and relase it if the refcount falls to 0
* _AND_ if the session expired. Note,, the refcount is always decremented.
*
* This function locks the corresponding table shard to proceed. When this
* function is called, the caller must be sure it owns a reference on the
* stksess (refcount >= 1).
*/
static inline void stksess_kill_if_expired(struct stktable *t, struct stksess *ts)
{
uint shard;
size_t len;
@ -232,11 +240,11 @@ static inline void stksess_kill_if_expired(struct stktable *t, struct stksess *t
ALREADY_CHECKED(shard);
HA_RWLOCK_WRLOCK(STK_TABLE_LOCK, &t->shards[shard].sh_lock);
if (!decrefcnt || !HA_ATOMIC_SUB_FETCH(&ts->ref_cnt, 1))
if (!HA_ATOMIC_SUB_FETCH(&ts->ref_cnt, 1))
__stksess_kill_if_expired(t, ts);
HA_RWLOCK_WRUNLOCK(STK_TABLE_LOCK, &t->shards[shard].sh_lock);
}
else if (decrefcnt)
else
HA_ATOMIC_SUB_FETCH(&ts->ref_cnt, 1);
}

4
include/haproxy/stream.h
View File

@ -140,7 +140,7 @@ static inline void stream_store_counters(struct stream *s)
stktable_touch_local(s->stkctr[i].table, ts, 0);
}
stkctr_set_entry(&s->stkctr[i], NULL);
stksess_kill_if_expired(s->stkctr[i].table, ts, 1);
stksess_kill_if_expired(s->stkctr[i].table, ts);
}
}
@ -182,7 +182,7 @@ static inline void stream_stop_content_counters(struct stream *s)
stktable_touch_local(s->stkctr[i].table, ts, 0);
}
stkctr_set_entry(&s->stkctr[i], NULL);
stksess_kill_if_expired(s->stkctr[i].table, ts, 1);
stksess_kill_if_expired(s->stkctr[i].table, ts);
}
}

15
src/stick_table.c
View File

@ -162,11 +162,14 @@ int __stksess_kill(struct stktable *t, struct stksess *ts)
}
/*
* Decrease the refcount if decrefcnt is not 0, and try to kill the stksess.
* Decrease the refcount of a stksess and relase it if the refcount falls to 0.
* Returns non-zero if deleted, zero otherwise.
* This function locks the table
*
* This function locks the corresponding table shard to proceed. When this
* function is called, the caller must be sure it owns a reference on the
* stksess (refcount >= 1).
*/
int stksess_kill(struct stktable *t, struct stksess *ts, int decrefcnt)
int stksess_kill(struct stktable *t, struct stksess *ts)
{
uint shard;
size_t len;
@ -183,7 +186,7 @@ int stksess_kill(struct stktable *t, struct stksess *ts, int decrefcnt)
ALREADY_CHECKED(shard);
HA_RWLOCK_WRLOCK(STK_TABLE_LOCK, &t->shards[shard].sh_lock);
if (!decrefcnt || !HA_ATOMIC_SUB_FETCH(&ts->ref_cnt, 1))
if (!HA_ATOMIC_SUB_FETCH(&ts->ref_cnt, 1))
ret = __stksess_kill(t, ts);
HA_RWLOCK_WRUNLOCK(STK_TABLE_LOCK, &t->shards[shard].sh_lock);
@ -5268,7 +5271,7 @@ static int table_process_entry(struct appctx *appctx, struct stksess *ts, char *
break;
case STK_CLI_ACT_CLR:
if (!stksess_kill(t, ts, 1)) {
if (!stksess_kill(t, ts)) {
/* don't delete an entry which is currently referenced */
return cli_err(appctx, "Entry currently in use, cannot remove\n");
}
@ -5685,7 +5688,7 @@ static void cli_release_show_table(struct appctx *appctx)
struct show_table_ctx *ctx = appctx->svcctx;
if (ctx->state == STATE_DUMP) {
stksess_kill_if_expired(ctx->t, ctx->entry, 1);
stksess_kill_if_expired(ctx->t, ctx->entry);
}
}