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

Module: sip-router Branch: master Commit: 5a89af6ea8b83ecc781d3f169023fde8388a2da6 URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=5a89af6… Author: Peter Dunkley&lt;peter.dunkley(a)crocodile-rcs.com&gt; Committer: Peter Dunkley&lt;peter.dunkley(a)crocodile-rcs.com&gt; 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; _______________________________________________ sr-dev mailing list sr-dev(a)lists.sip-router.org http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev