Skip to content

Commit

Permalink
presence: Fix leaking of activewatchers in database (with clustering)
Browse files Browse the repository at this point in the history
If you're using clustering, without tags (or with tags, but without
fallback2db), you would end up with a lot of active watchers in the
database that never got cleaned up.

Before 929ab4d:

- all stale activewatcher records in the db got purged.

After:

- if you're not clustering, all stale active watcher got purged;

- if you are, only those with the sharing_tag set.

However, the sharing tag is not necessarily set:

- it is an optional argument to handle_subscribe;

- and setting it in handle_subscribe does not write to the database
  because of missing code in the fallback2db==0 bit;

- and even it it were set, then adding the argument to handle_subscribe
  does not "activate" the sharing tag.

(Also interesting to know: a 408 or 481 after the
this-subscription-is-expired NOTIFY _would_ cause the individual record
to get deleted. But any other response, including 200, would leave the
record to get sorted by the periodic purge.)

This changeset reverts parts of the aforementioned commit by always
purging stale records if the sharing_tag is NULL. Thus restoring
behaviour to pre-3.1.0 and pre-2.4.8.
  • Loading branch information
wdoekes committed May 11, 2021
1 parent 4942f35 commit acd309a
Showing 1 changed file with 15 additions and 16 deletions.
31 changes: 15 additions & 16 deletions modules/presence/subscribe.c
Original file line number Diff line number Diff line change
Expand Up @@ -1357,7 +1357,7 @@ static inline int is_shtag_active( str *my_tag, str **active_tags)
void update_db_subs(db_con_t *db,db_func_t *dbf, shtable_t hash_table,
int htable_size, int no_lock, handle_expired_func_t handle_expired_func)
{
static db_ps_t my_ps_delete = NULL;
static db_ps_t my_ps_delete = NULL, my_ps_delete_null = NULL;
static db_ps_t my_ps_update = NULL, my_ps_insert = NULL;
db_key_t query_cols[22], update_cols[8];
db_val_t query_vals[22], update_vals[8];
Expand Down Expand Up @@ -1660,33 +1660,33 @@ void update_db_subs(db_con_t *db,db_func_t *dbf, shtable_t hash_table,
lock_release(&hash_table[i].lock);
}

/* now that all records were updated, delete whatever
/* now that all records were updated, delete whatever
was still left as expired */
update_cols[0]= &str_expires_col;
update_cols[0] = &str_expires_col;
update_vals[0].type = DB_INT;
update_vals[0].nul = 0;
update_vals[0].val.int_val = (int)time(NULL);
update_ops[0] = OP_LT;

CON_SET_CURR_PS(db, &my_ps_delete);
update_cols[1] = &str_sharing_tag_col;
update_vals[1].nul = 1;
update_ops[1] = OP_IS_NULL;

if (dbf->use_table(db, &active_watchers_table) < 0) {
LM_ERR("deleting expired information from database\n");
CON_RESET_CURR_PS(db);
return;
}

if (sh_tags==NULL) {

/* no clustering, simply delete all expired subs */
LM_DBG("delete all expired subscriptions\n");
/* no clustering, simply delete all expired subs with NULL sh tags */
LM_DBG("delete all expired subscriptions\n");

if (dbf->delete(db, update_cols, update_ops, update_vals, 1) < 0)
LM_ERR("deleting expired information from database\n");

} else {
CON_SET_CURR_PS(db, &my_ps_delete_null);
if (dbf->delete(db, update_cols, update_ops, update_vals, 2) < 0)
LM_ERR("deleting expired information from database\n");

if (sh_tags != NULL) {
/* clustering, delete only expired subs with active sh tags */
update_cols[1]= &str_sharing_tag_col;
update_vals[1].type = DB_STR;
update_vals[1].nul = 0;
update_ops[1] = OP_EQ;
Expand All @@ -1697,13 +1697,12 @@ void update_db_subs(db_con_t *db,db_func_t *dbf, shtable_t hash_table,
sh_tags[i]->len, sh_tags[i]->s);

update_vals[1].val.str_val = *sh_tags[i];

CON_SET_CURR_PS(db, &my_ps_delete);
if (dbf->delete(db, update_cols, update_ops, update_vals, 2) < 0)
LM_ERR("deleting expired information from database\n");
i++;
}

if (i == 0)
CON_RESET_CURR_PS(db);
}

return;
Expand Down

0 comments on commit acd309a

Please sign in to comment.