[sr-dev] git:master: modules_k/presence: Fixed DB insert race hazard on the watchers table

Peter Dunkley peter.dunkley at crocodile-rcs.com
Thu Mar 29 17:13:22 CEST 2012


Module: sip-router
Branch: master
Commit: 5a89af6ea8b83ecc781d3f169023fde8388a2da6
URL:    http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=5a89af6ea8b83ecc781d3f169023fde8388a2da6

Author: Peter Dunkley <peter.dunkley at crocodile-rcs.com>
Committer: Peter Dunkley <peter.dunkley at crocodile-rcs.com>
Date:   Thu Mar 29 16:02:13 2012 +0100

modules_k/presence: Fixed DB insert race hazard on the watchers table

- The time between the query on the watchers table (which determines
  there is no matching entry) and the insert is substantial.  During
  a soak I observed inserts failing because rows had been inserted in
  this time window.
- The fix is to use replace (where available) instead of insert.
- Also fixed a small whitespace issue I noticed, and added an extra
  use_table call (as I think there was one missing).

---

 modules_k/presence/presence.c  |    2 +-
 modules_k/presence/subscribe.c |   32 ++++++++++++++++++++++++++++----
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/modules_k/presence/presence.c b/modules_k/presence/presence.c
index dd415f7..f6f9152 100644
--- a/modules_k/presence/presence.c
+++ b/modules_k/presence/presence.c
@@ -833,7 +833,7 @@ int update_watchers_status(str pres_uri, pres_ev_t* ev, str* rules_doc)
 	}ws_t;
 	ws_t* ws_list= NULL;
 
-    LM_DBG("start\n");
+	LM_DBG("start\n");
 
 	if(ev->content_type.s== NULL)
 	{
diff --git a/modules_k/presence/subscribe.c b/modules_k/presence/subscribe.c
index 0c914c7..0a9db83 100644
--- a/modules_k/presence/subscribe.c
+++ b/modules_k/presence/subscribe.c
@@ -315,6 +315,12 @@ int insert_subs_db(subs_t* s, int type)
 	query_vals[reason_col].val.str_val= s->reason;
 	query_vals[socket_info_col].val.str_val= s->sockinfo_str;
 
+	if (pa_dbf.use_table(pa_db, &active_watchers_table) < 0)
+	{
+		LM_ERR("in use table sql operation\n");	
+		return -1;
+	}
+
 	LM_DBG("inserting subscription in active_watchers table\n");
 	if(pa_dbf.insert(pa_db, query_cols, query_vals, n_query_cols) < 0)
 	{
@@ -2310,10 +2316,28 @@ int insert_db_subs_auth(subs_t* subs)
 		return -1;
 	}
 
-	if(pa_dbf.insert(pa_db, db_keys, db_vals, n_query_cols )< 0)
-	{	
-		LM_ERR("in sql insert\n");
-		return -1;
+	if (pa_dbf.replace != NULL)
+	{
+		if(pa_dbf.replace(pa_db, db_keys, db_vals, n_query_cols,
+					2, 0) < 0)
+		{
+			LM_ERR("in sql replace\n");
+			return -1;
+		}
+	}
+	else
+	{
+		/* If you use insert() instead of replace() be prepared for some
+ 		   DB error messages.  There is a lot of time between the 
+		   query() that indicated there was no matching entry in the DB
+		   and this insert(), so on a multi-user system it is entirely
+		   possible (even likely) that a record will be added after the
+		   query() but before this insert(). */
+		if(pa_dbf.insert(pa_db, db_keys, db_vals, n_query_cols )< 0)
+		{	
+			LM_ERR("in sql insert\n");
+			return -1;
+		}
 	}
 
 	return 0;




More information about the sr-dev mailing list