Browse code

Merge pull request #2462 from NGSegovia/keepalive_fix_race_condition_tm_request

keepalive: added uuid to ka_dest structure to avoid passing the whole struct to tm

Daniel-Constantin Mierla authored on 04/09/2020 11:46:39 • GitHub committed on 04/09/2020 11:46:39
Showing 4 changed files
... ...
@@ -31,6 +31,8 @@
31 31
 #include <time.h>
32 32
 #include "../../core/sr_module.h"
33 33
 #include "../../core/locking.h"
34
+#include "../../core/str.h"
35
+#include "../../core/utils/sruid.h"
34 36
 #include "../tm/tm_load.h"
35 37
 
36 38
 #define KA_INACTIVE_DST 1 /*!< inactive destination */
... ...
@@ -60,6 +62,7 @@ typedef struct _ka_dest
60 62
 	str uri;
61 63
 	str owner; // name of destination "owner"
62 64
 			   // (module asking to monitor this destination
65
+	str  uuid; // Universal id for this record
63 66
 	int flags;
64 67
 	int state;
65 68
 	time_t last_checked, last_up, last_down;
... ...
@@ -85,6 +88,7 @@ typedef struct _ka_destinations_list
85 88
 
86 89
 extern ka_destinations_list_t *ka_destinations_list;
87 90
 extern int ka_counter_del;
91
+extern sruid_t ka_sruid;
88 92
 
89 93
 ticks_t ka_check_timer(ticks_t ticks, struct timer_ln* tl, void* param);
90 94
 
... ...
@@ -96,6 +100,7 @@ int ka_str_copy(str *src, str *dest, char *prefix);
96 100
 int free_destination(ka_dest_t *dest) ;
97 101
 int ka_del_destination(str *uri, str *owner) ;
98 102
 int ka_find_destination(str *uri, str *owner, ka_dest_t **target ,ka_dest_t **head);
103
+int ka_find_destination_by_uuid(str uuid, ka_dest_t **target, ka_dest_t **head);
99 104
 int ka_lock_destination_list();
100 105
 int ka_unlock_destination_list();
101 106
 #endif
... ...
@@ -103,6 +103,10 @@ int ka_add_dest(str *uri, str *owner, int flags, int ping_interval,
103 103
 	if(ka_str_copy(owner, &(dest->owner), NULL) < 0)
104 104
 		goto err;
105 105
 
106
+	if(sruid_next(&ka_sruid) < 0)
107
+		goto err;
108
+	ka_str_copy(&(ka_sruid.uid), &(dest->uuid), NULL);
109
+
106 110
 	dest->flags = flags;
107 111
 	dest->statechanged_clb = statechanged_clb;
108 112
 	dest->response_clb = response_clb;
... ...
@@ -236,6 +240,38 @@ int ka_find_destination(str *uri, str *owner, ka_dest_t **target, ka_dest_t **he
236 240
 	return 0;
237 241
 
238 242
 }
243
+
244
+/*!
245
+* @function ka_find_destination_by_uuid
246
+*
247
+* @param *uuid uuid of ka_dest record
248
+* @param **target searched address in stack
249
+* @param **head which points target
250
+*	*
251
+* @result 1 successful  , 0 fail
252
+*/
253
+int ka_find_destination_by_uuid(str uuid, ka_dest_t **target, ka_dest_t **head){
254
+	ka_dest_t  *dest=0 ,*temp=0;
255
+
256
+	LM_DBG("finding destination with uuid:%.*s\n", uuid.len, uuid.s);
257
+
258
+	for(dest = ka_destinations_list->first ;dest ; temp = dest, dest = dest->next ){
259
+		if(!dest)
260
+			break;
261
+
262
+		if (STR_EQ(uuid, dest->uuid)){
263
+			*head = temp;
264
+			*target = dest;
265
+			LM_DBG("destination is found [target : %p] [head : %p] \r\n",target,temp);
266
+			return 1;
267
+		}
268
+	}
269
+
270
+	return 0;
271
+
272
+}
273
+
274
+
239 275
 /*!
240 276
 * @function free_destination
241 277
 * @abstract free ka_dest_t members
... ...
@@ -259,6 +295,9 @@ int free_destination(ka_dest_t *dest){
259 295
 		if(dest->owner.s)
260 296
 			shm_free(dest->owner.s);
261 297
 
298
+		if(dest->uuid.s)
299
+			shm_free(dest->uuid.s);
300
+		
262 301
 		shm_free(dest);
263 302
 	}
264 303
 
... ...
@@ -67,12 +67,14 @@ ticks_t ka_check_timer(ticks_t ticks, struct timer_ln* tl, void* param)
67 67
         return (ticks_t)(0); /* stops the timer */
68 68
     }
69 69
 
70
+	str *uuid = shm_malloc(sizeof(str));
71
+	ka_str_copy(&(ka_dest->uuid), uuid, NULL);
70 72
     /* Send ping using TM-Module.
71 73
      * int request(str* m, str* ruri, str* to, str* from, str* h,
72 74
      *		str* b, str *oburi,
73 75
      *		transaction_cb cb, void* cbp); */
74 76
     set_uac_req(&uac_r, &ka_ping_method, 0, 0, 0, TMCB_LOCAL_COMPLETED,
75
-            ka_options_callback, (void *)ka_dest);
77
+            ka_options_callback, (void *)uuid);
76 78
 
77 79
     if(tmb.t_request(&uac_r, &ka_dest->uri, &ka_dest->uri, &ka_ping_from,
78 80
                &ka_outbound_proxy)
... ...
@@ -99,8 +101,15 @@ static void ka_options_callback(
99 101
 
100 102
 	char *state_routes[] = {"", "keepalive:dst-up", "keepalive:dst-down"};
101 103
 
102
-	//NOTE: how to be sure destination is still allocated ?
103
-	ka_dest_t *ka_dest = (ka_dest_t *)(*ps->param);
104
+	str *uuid = (str *)(*ps->param);
105
+
106
+	// Retrieve ka_dest by uuid from destination list
107
+	ka_dest_t *ka_dest=0,*hollow=0;
108
+	if (!ka_find_destination_by_uuid(*uuid, &ka_dest, &hollow)) {
109
+		LM_ERR("Couldn't find destination \r\n");
110
+		return;
111
+	}
112
+	shm_free(uuid);
104 113
 
105 114
 	uri.s = t->to.s + 5;
106 115
 	uri.len = t->to.len - 8;
... ...
@@ -61,6 +61,7 @@ extern struct tm_binds tmb;
61 61
 
62 62
 int ka_ping_interval = 30;
63 63
 ka_destinations_list_t *ka_destinations_list = NULL;
64
+sruid_t ka_sruid;
64 65
 str ka_ping_from = str_init("sip:keepalive@kamailio.org");
65 66
 int ka_counter_del = 5;
66 67
 
... ...
@@ -124,6 +125,10 @@ static int mod_init(void)
124 125
 	if(ka_alloc_destinations_list() < 0)
125 126
 		return -1;
126 127
 
128
+	if(sruid_init(&ka_sruid, '-', "ka", SRUID_INC) < 0) {
129
+		return -1;
130
+	}
131
+
127 132
 	return 0;
128 133
 }
129 134