Skip to content

Commit addbbc9

Browse files
committed
presence: Fix leaking of activewatchers in database (with clustering)
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.
1 parent 992a552 commit addbbc9

File tree

1 file changed

+15
-16
lines changed

1 file changed

+15
-16
lines changed

modules/presence/subscribe.c

+15-16
Original file line numberDiff line numberDiff line change
@@ -1357,7 +1357,7 @@ static inline int is_shtag_active( str *my_tag, str **active_tags)
13571357
void update_db_subs(db_con_t *db,db_func_t *dbf, shtable_t hash_table,
13581358
int htable_size, int no_lock, handle_expired_func_t handle_expired_func)
13591359
{
1360-
static db_ps_t my_ps_delete = NULL;
1360+
static db_ps_t my_ps_delete = NULL, my_ps_delete_null = NULL;
13611361
static db_ps_t my_ps_update = NULL, my_ps_insert = NULL;
13621362
db_key_t query_cols[22], update_cols[8];
13631363
db_val_t query_vals[22], update_vals[8];
@@ -1660,33 +1660,33 @@ void update_db_subs(db_con_t *db,db_func_t *dbf, shtable_t hash_table,
16601660
lock_release(&hash_table[i].lock);
16611661
}
16621662

1663-
/* now that all records were updated, delete whatever
1663+
/* now that all records were updated, delete whatever
16641664
was still left as expired */
1665-
update_cols[0]= &str_expires_col;
1665+
update_cols[0] = &str_expires_col;
16661666
update_vals[0].type = DB_INT;
16671667
update_vals[0].nul = 0;
16681668
update_vals[0].val.int_val = (int)time(NULL);
16691669
update_ops[0] = OP_LT;
16701670

1671-
CON_SET_CURR_PS(db, &my_ps_delete);
1671+
update_cols[1] = &str_sharing_tag_col;
1672+
update_vals[1].nul = 1;
1673+
update_ops[1] = OP_IS_NULL;
1674+
16721675
if (dbf->use_table(db, &active_watchers_table) < 0) {
16731676
LM_ERR("deleting expired information from database\n");
16741677
CON_RESET_CURR_PS(db);
16751678
return;
16761679
}
16771680

1678-
if (sh_tags==NULL) {
1679-
1680-
/* no clustering, simply delete all expired subs */
1681-
LM_DBG("delete all expired subscriptions\n");
1681+
/* no clustering, simply delete all expired subs with NULL sh tags */
1682+
LM_DBG("delete all expired subscriptions\n");
16821683

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

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

16991699
update_vals[1].val.str_val = *sh_tags[i];
1700+
1701+
CON_SET_CURR_PS(db, &my_ps_delete);
17001702
if (dbf->delete(db, update_cols, update_ops, update_vals, 2) < 0)
17011703
LM_ERR("deleting expired information from database\n");
17021704
i++;
17031705
}
1704-
1705-
if (i == 0)
1706-
CON_RESET_CURR_PS(db);
17071706
}
17081707

17091708
return;

0 commit comments

Comments
 (0)