[sr-dev] git:master:e7514fde: modules/ims_charging: fixed up some locking problems

[Jason Penton]

Module: kamailio
Branch: master
Commit: e7514fde047a8616cea49db76efdc6c98e92030f
URL: https://github.com/kamailio/kamailio/commit/e7514fde047a8616cea49db76efdc6c98e92030f

Author: Jason Penton <jason.penton at gmail.com>
Committer: Jason Penton <jason.penton at gmail.com>
Date: 2015-02-03T16:16:33+02:00

modules/ims_charging: fixed up some locking problems
	- could result in unexpected behaviour and even seg faults

---

Modified: modules/ims_charging/dialog.c
Modified: modules/ims_charging/ims_ro.c
Modified: modules/ims_charging/ro_timer.c

---

Diff:  https://github.com/kamailio/kamailio/commit/e7514fde047a8616cea49db76efdc6c98e92030f.diff
Patch: https://github.com/kamailio/kamailio/commit/e7514fde047a8616cea49db76efdc6c98e92030f.patch

---

diff --git a/modules/ims_charging/dialog.c b/modules/ims_charging/dialog.c
index 1a79175..0f8beb8 100644
--- a/modules/ims_charging/dialog.c
+++ b/modules/ims_charging/dialog.c
@@ -159,14 +159,15 @@ void dlg_terminated(struct dlg_cell *dlg, int type, struct dlg_cb_params *_param
 				//if the Ro session is not active we don't need to do anything. This prevents
 				//double processing for various dialog_terminated callback events.
 				//If however, the call was never answered, then we can continue as normal
+				ro_session_lock(ro_session_table, ro_session_entry);
 				if (!ro_session->active && (ro_session->start_time != 0)) {
 					unref_ro_session(ro_session,1);
 					LM_ERR("Ro Session is not active, but may have been answered [%d]\n", (int)ro_session->start_time);
+					ro_session_unlock(ro_session_table, ro_session_entry);
 					return;
 				}
 			
 	
-				ro_session_lock(ro_session_table, ro_session_entry);
 
 				if (ro_session->active) { // if the call was never activated, there's no timer to remove
 					int ret = remove_ro_timer(&ro_session->ro_tl);
@@ -280,4 +281,4 @@ void add_dlg_data_to_contact(struct dlg_cell *dlg, int type, struct dlg_cb_param
 	}
 	ul.unlock_udomain(domain_t, &impu_data->identity);
     }
-} 
\ No newline at end of file
+} 
diff --git a/modules/ims_charging/ims_ro.c b/modules/ims_charging/ims_ro.c
index 5cd0f5f..142135f 100644
--- a/modules/ims_charging/ims_ro.c
+++ b/modules/ims_charging/ims_ro.c
@@ -487,6 +487,7 @@ int sip_create_ro_ccr_data(struct sip_msg * msg, int dir, Ro_CCR_t ** ro_ccr_dat
     return 0;
 }
 
+/* must be called with lock on ro_session */
 void send_ccr_interim(struct ro_session* ro_session, unsigned int used, unsigned int reserve) {
     AAASession * auth = 0;
 
@@ -647,6 +648,7 @@ void send_ccr_interim(struct ro_session* ro_session, unsigned int used, unsigned
     // to it can be reused later.
     //
     struct ro_session_entry *ro_session_entry = &(ro_session_table->entries[ro_session->h_entry]);
+    ro_session_lock(ro_session_table, ro_session_entry);
     unref_ro_session_unsafe(ro_session, 1, ro_session_entry);//unref from the initial timer that fired this event.
     ro_session_unlock(ro_session_table, ro_session_entry);
 
diff --git a/modules/ims_charging/ro_timer.c b/modules/ims_charging/ro_timer.c
index 5821e8c..75f6eab 100644
--- a/modules/ims_charging/ro_timer.c
+++ b/modules/ims_charging/ro_timer.c
@@ -266,7 +266,7 @@ void resume_ro_session_ontimeout(struct interim_ccr *i_req) {
 	}
 
 	ro_session_entry = &(ro_session_table->entries[i_req->ro_session->h_entry]);
-
+	ro_session_lock(ro_session_table, ro_session_entry);
 	LM_DBG("credit=%d credit_valid_for=%d", i_req->new_credit, i_req->credit_valid_for);
 
 	used_secs = now - i_req->ro_session->last_event_timestamp;
@@ -392,7 +392,6 @@ void ro_session_ontimeout(struct ro_tl *tl) {
 	}
 
 	ro_session_entry = &(ro_session_table->entries[ro_session->h_entry]);
-	ro_session_lock(ro_session_table, ro_session_entry);
 	
 	LM_DBG("event-type=%d", ro_session->event_type);
 	
@@ -467,7 +466,7 @@ void ro_session_ontimeout(struct ro_tl *tl) {
 	update_stat(killed_calls, 1);
 
 	//unref_ro_session_unsafe(ro_session, 1, ro_session_entry); //unref from the initial timer that fired this event.
-	ro_session_unlock(ro_session_table, ro_session_entry);
+//	ro_session_unlock(ro_session_table, ro_session_entry);
 
 	dlgb.lookup_terminate_dlg(ro_session->dlg_h_entry, ro_session->dlg_h_id, NULL);
 	return;