Hi Peter,
You do not create transactions for Subscribe requests before calling
handle_subscribe() function to catch retransmissions?
I wonder how can you get to the situation of another process inserting a
record (with exactly the same callid, to tag & from tag) between the
query and the insert.
Regards,
Anca
On 03/29/2012 06:13 PM, Peter Dunkley wrote:
> 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@crocodile-rcs.com>
> Committer: Peter Dunkley<peter.dunkley@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;
>
>
> _______________________________________________
> sr-dev mailing list
> sr-dev@lists.sip-router.org
> http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
>
_______________________________________________
sr-dev mailing list
sr-dev@lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev