[sr-dev] git:master: modules_k/rls: Fixed race hazard on rls_presentity table

[Peter Dunkley]

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

Author: Peter Dunkley <peter.dunkley at crocodile-rcs.com>
Committer: Peter Dunkley <peter.dunkley at crocodile-rcs.com>
Date:   Mon Mar 12 21:44:46 2012 +0000

modules_k/rls: Fixed race hazard on rls_presentity table

- During testing I observed that it is possible for two back-end NOTIFYs that
  will affect the same rls_presentity table row to be processed simultaneously.
  This is a problem when the row does not yet exist as the SELECTs can both
  return 0 rows resulting in both processes trying to INSERT - but only one
  will succeed.
- The easiest way to fix this is to use the replace() DB API, but this is not
  available for some databases (for example, PostgreSQL).
- I have updated the code to use replace() when it is available and to do an
  update() then check affected_rows() (and if 0 then do an insert()) when
  replace() is not available.
- The update() and then insert() process makes the window for the race much
  smaller, but doesn't get rid of it completely.  However, with PostgreSQL a
  DB rule can be used to fix it completely.
- PostgreSQL DB rule:
CREATE RULE "rls_presentity_insert_race" AS ON INSERT TO "rls_presentity"
 WHERE EXISTS(
  SELECT 1 FROM rls_presentity WHERE (rlsubs_did, resource_uri)=(NEW.rlsubs_did, NEW.resource_uri)
 ) DO INSTEAD (
  UPDATE rls_presentity
    SET updated=NEW.updated,
        auth_state=NEW.auth_state,
        reason=NEW.reason,
        content_type=NEW.content_type,
        presence_state=NEW.presence_state,
        expires=NEW.expires
    WHERE (rlsubs_did, resource_uri)=(NEW.rlsubs_did, NEW.resource_uri)
 );

---

 modules_k/rls/resource_notify.c |   45 ++++++++++++++++----------------------
 1 files changed, 19 insertions(+), 26 deletions(-)

diff --git a/modules_k/rls/resource_notify.c b/modules_k/rls/resource_notify.c
index df40e82..d0bf435 100644
--- a/modules_k/rls/resource_notify.c
+++ b/modules_k/rls/resource_notify.c
@@ -500,16 +500,15 @@ int rls_handle_notify(struct sip_msg* msg, char* c1, char* c2)
 	str body= {0, 0};
 	ua_pres_t dialog;
 	str* res_id= NULL;
-	db_key_t query_cols[9], result_cols[1];
-	db_val_t query_vals[9];
-	db1_res_t* result= NULL;
+	db_key_t query_cols[8];
+	db_val_t query_vals[8];
 	int n_query_cols= 0;
 	str auth_state= {0, 0};
 	int found= 0;
 	str reason = {0, 0};
 	int auth_flag;
 	struct hdr_field* hdr= NULL;
-	int n, expires= -1;
+	int expires= -1;
 	str content_type= {0, 0};
 	int reply_code = 500;
 	str reply_str = pu_500_rpl;
@@ -755,42 +754,36 @@ int rls_handle_notify(struct sip_msg* msg, char* c1, char* c2)
 		LM_ERR("in use_table\n");
 		goto error;
 	}
-	/* query-> if not present insert // else update */
-	result_cols[0]= &str_updated_col;
-			
-	if(rlpres_dbf.query(rlpres_db, query_cols, 0, query_vals, result_cols,
-					2, 1, 0, &result)< 0)
-	{
-		LM_ERR("in sql query\n");
-		if(result)
-			rlpres_dbf.free_result(rlpres_db, result);
-		goto error;
-	}
-	if(result== NULL)
-		goto error;
-	n= result->n;
-	rlpres_dbf.free_result(rlpres_db, result);
-		
-	if(n<= 0)
+
+	if (rlpres_dbf.replace != NULL)
 	{
-		if(rlpres_dbf.insert(rlpres_db, query_cols, query_vals, n_query_cols)< 0)
+		if(rlpres_dbf.replace(rlpres_db, query_cols, query_vals, n_query_cols)< 0)
 		{
-			LM_ERR("in sql insert\n");
+			LM_ERR("in sql replace\n");
 			goto error;
 		}
-		LM_DBG("Inserted in database table new record\n");
+		LM_DBG("Inserted/replace in database table new record\n");
 	}
 	else
 	{
-		LM_DBG("Updated in db table already existing record\n");
 		if(rlpres_dbf.update(rlpres_db, query_cols, 0, query_vals, query_cols+2,
 						query_vals+2, 2, n_query_cols-2)< 0)
 		{
 			LM_ERR("in sql update\n");
 			goto error;
 		}
-	}
 
+		if (rlpres_dbf.affected_rows(rlpres_db) == 0)
+		{
+			if(rlpres_dbf.insert(rlpres_db, query_cols, query_vals, n_query_cols)< 0)
+			{
+				LM_ERR("in sql insert\n");
+				goto error;
+			}
+			LM_DBG("Inserted in database table new record\n");
+		}
+	}
+		
 	LM_DBG("Updated rlpres_table\n");	
 	/* reply 200OK */
 done: