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

Module: sip-router Branch: master Commit: 46eb7f2c33231355aa26632bc3d1adb02ee2556f URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=46eb7f2… Author: Peter Dunkley &lt;peter.dunkley(a)crocodile-rcs.com&gt; Committer: Peter Dunkley &lt;peter.dunkley(a)crocodile-rcs.com&gt; 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: