Fix database purging of stale activewatchers

db/db_op.c

@@ -0,0 +1,25 @@
+/*
+ * Copyright (C) 2021 OpenSIPS
+ *
+ * This file is part of opensips, a free SIP server.
+ *
+ * opensips is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version
+ *
+ * opensips is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301  USA
+ */
+
+#include "db_op.h"
+
+
+const char OP_IS_NULL[] = " IS NULL";
+const char OP_IS_NOT_NULL[] = " IS NOT NULL";

db/db_op.h

@@ -40,6 +40,17 @@
 /** operator negation */
 #define OP_NEQ "!="
 
+/* Special unary operators here, because mysql prepared statements do
+ * not cope with 'column IS ?' (they would cope with 'column <=> ?' but
+ * that's not standard SQL).
+ * (Declared as char array instead of define, so they can be pointer
+ * compared in code.) */
+
+/** unary operator: IS NULL */
+extern const char OP_IS_NULL[]; /* " IS NULL" */
+/** unary operator: IS NOT NULL */
+extern const char OP_IS_NOT_NULL[]; /* " IS NOT NULL" */
+
 
 /**
  * This type represents an expression operator uses for SQL queries.

db/db_ut.c

@@ -336,16 +336,33 @@ int db_print_where(const db_con_t* _c, char* _b, const int _l, const db_key_t* _
 
 	for(i = 0; i < _n; i++) {
 		if (_o) {
-			ret = snprintf(_b + len, _l - len, "%.*s%s",
-				_k[i]->len, _k[i]->s, _o[i]);
-			if (ret < 0 || ret >= (_l - len)) goto error;
-			len += ret;
+			/* Special case: if there is unary operator and
+			 * we use a prepared statement, we must still
+			 * consume one value, even though we're not
+			 * using it. Otherwise the number of values will
+			 * be incorrect. */
+			if ((_o[i] == OP_IS_NULL || _o[i] == OP_IS_NOT_NULL) &&
+					CON_HAS_PS(_c)) {
+				/* ?=NULL will never be true; so we're
+				 * safely consuming a single argument */
+				ret = snprintf(_b + len, _l - len, "(%.*s%s OR ?=NULL)",
+						_k[i]->len, _k[i]->s, _o[i]);
+				if (ret < 0 || ret >= (_l - len)) goto error;
+				len += ret;
+			} else {
+				ret = snprintf(_b + len, _l - len, "%.*s%s",
+						_k[i]->len, _k[i]->s, _o[i]);
+				if (ret < 0 || ret >= (_l - len)) goto error;
+				len += ret;
+			}
 		} else {
 			ret = snprintf(_b + len, _l - len, "%.*s=", _k[i]->len, _k[i]->s);
 			if (ret < 0 || ret >= (_l - len)) goto error;
 			len += ret;
 		}
-		if (CON_HAS_PS(_c)) {
+		if (_o && (_o[i] == OP_IS_NULL || _o[i] == OP_IS_NOT_NULL)) {
+			;
+		} else if (CON_HAS_PS(_c)) {
 			*(_b+len++) = '?';
 		} else {
 			l = _l - len;

modules/presence/subscribe.c

@@ -1356,7 +1356,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];
@@ -1664,32 +1664,32 @@ 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;
 
+	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");
 		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");
 
-		CON_SET_CURR_PS(db, &my_ps_delete);
-		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;
@@ -1698,8 +1698,8 @@ void update_db_subs(db_con_t *db,db_func_t *dbf, shtable_t hash_table,
 		while(sh_tags[i]) {
 			LM_DBG("delete expired subscriptions for tag <%.*s>\n",
 				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");