2
0
Эх сурвалжийг харах

modules_k/pua: Fixed race hazard relating to RLS back-end SUBSCRIBEs

- This resulted in the "no presence dialog record for non-TERMINATED state..."
  error message coming out of RLS a lot.
- On the sending side you have can have two dialogs (one temporary and one full)
  stored for a short period of time.  This is because the full dialog is written
  before the temporary one is deleted.  This can make the lookup when a back-end
  NOTIFY is received fail because only one record is expected.  This is now
  fixed - instead of inserting and then deleting we do a swap (while the hash
  table is locked).
- Based on... (cherry picked from commit e627bc31776b521a1078b2a004e8ed179521cae2)
pd 13 жил өмнө
parent
commit
047f66369f

+ 32 - 16
modules_k/pua/hash.c

@@ -215,33 +215,28 @@ void update_htable(ua_pres_t* p, time_t desired_expires, int expires,
 	}
 }
 /* insert in front; so when searching the most recent result is returned*/
-void insert_htable(ua_pres_t* presentity)
+void _insert_htable(ua_pres_t* presentity, unsigned int hash_code)
 {
 	ua_pres_t* p= NULL;
-	unsigned int hash_code;
-
-	hash_code= core_hash(presentity->pres_uri,presentity->watcher_uri, 
-			HASH_SIZE);
-	
-	lock_get(&HashT->p_records[hash_code].lock);
 
-/*	
- *	useless since always checking before calling insert
-	if(get_dialog(presentity, hash_code)!= NULL )
-	{
-		LM_DBG("Dialog already found- do not insert\n");
-		return; 
-	}
-*/	
 	p= HashT->p_records[hash_code].entity;
 
 	presentity->db_flag= INSERTDB_FLAG;
 	presentity->next= p->next;
 	
 	p->next= presentity;
+}
 
-	lock_release(&HashT->p_records[hash_code].lock);
+void insert_htable(ua_pres_t* presentity)
+{
+	unsigned int hash_code;
+
+	hash_code= core_hash(presentity->pres_uri,presentity->watcher_uri, HASH_SIZE);
+	lock_get(&HashT->p_records[hash_code].lock);
+
+	_insert_htable(presentity, hash_code);
 
+	lock_release(&HashT->p_records[hash_code].lock);
 }
 
 /* This function used to perform a search to find the hash table
@@ -302,6 +297,27 @@ void destroy_htable(void)
   return;
 }
 
+int convert_temporary_dialog(ua_pres_t *dialog)
+{
+	ua_pres_t *temp_dialog;
+	unsigned int hash_code;
+
+	hash_code= core_hash(dialog->pres_uri,dialog->watcher_uri, HASH_SIZE); 
+	lock_get(&HashT->p_records[hash_code].lock);
+
+	temp_dialog = get_temporary_dialog(dialog, hash_code);
+	if (temp_dialog)
+		delete_htable(temp_dialog, hash_code);
+	else
+		return -1;
+
+	_insert_htable(dialog, hash_code);
+
+	lock_release(&HashT->p_records[hash_code].lock);
+
+	return 1;
+}
+
 /* must lock the record line before calling this function*/
 ua_pres_t* get_dialog(ua_pres_t* dialog, unsigned int hash_code)
 {

+ 1 - 0
modules_k/pua/hash.h

@@ -126,6 +126,7 @@ int is_dialog(ua_pres_t* dialog);
 
 ua_pres_t* get_dialog(ua_pres_t* dialog, unsigned int hash_code);
 ua_pres_t* get_temporary_dialog(ua_pres_t* dialog, unsigned int hash_code);
+int convert_temporary_dialog(ua_pres_t *dialog);
 
 int get_record_id(ua_pres_t* dialog, str** rec_id);
 typedef int (*get_record_id_t)(ua_pres_t* dialog, str** rec_id);

+ 10 - 6
modules_k/pua/send_subscribe.c

@@ -612,7 +612,11 @@ void subs_cback_func(struct cell *t, int cb_type, struct tmcb_params *ps)
 	LM_DBG("record for subscribe from %.*s to %.*s inserted in datatbase\n",
 			presentity->watcher_uri->len, presentity->watcher_uri->s,
 			presentity->pres_uri->len, presentity->pres_uri->s);
-	insert_htable(presentity);
+	if (convert_temporary_dialog(presentity) < 0)
+	{
+		LM_ERR("Could not convert temporary dialog into a dialog\n");
+		goto error;
+	}
 
 done:
 	if(hentity->ua_flag == REQ_OTHER)
@@ -620,13 +624,13 @@ done:
 		hentity->flag= flag;
 		run_pua_callbacks( hentity, msg);
 	}
+	goto end;
+
 error:	
-	lock_get(&HashT->p_records[hash_code].lock);
-	presentity = get_temporary_dialog(hentity, hash_code);
-	if (presentity!=NULL)
-		delete_htable(presentity, hash_code);
-	lock_release(&HashT->p_records[hash_code].lock);
+	if (presentity->remote_contact.s) shm_free(presentity->remote_contact.s);
+	if (presentity) shm_free(presentity);
 
+end:
 	if(hentity)
 	{	
 		shm_free(hentity);