MT#55283 simplify call2_get

Change-Id: I92407ea9ed9ab794e22e00cfa43a61599d768f48

daemon/call.c

@@ -5501,44 +5501,33 @@ call_t *call_get(const str *callid) {
 	return ret;
 }
 
-// special version of call_get() to get two calls while avoiding deadlock
-call_get2_ret_t call_get2(call_t **ret1, call_t **ret2, const str *callid1, const str *callid2) {
-	call_get2_ret_t ret;
+// obtain a reference to a second call while holding a lock to another call
+// may intermittently release lock on the first call!
+// return return another reference to the same call
+call_t *call_get2(call_t *call1, const str *callid2) {
+	call_t *ret;
 
 	while (true) {
 		RWLOCK_R(&rtpe_callhash_lock);
 
-		*ret1 = t_hash_table_lookup(rtpe_callhash, callid1);
-		if (!*ret1)
-			return CG2_NF1;
-		*ret2 = t_hash_table_lookup(rtpe_callhash, callid2);
-		if (!*ret2)
-			return CG2_NF2;
-
-		if (*ret1 == *ret2) {
-			*ret2 = NULL;
-			ret = CG2_SAME;
-			rwlock_lock_w(&(*ret1)->master_lock);
-			obj_hold(*ret1);
+		ret = t_hash_table_lookup(rtpe_callhash, callid2);
+		if (!ret)
+			return NULL;
+
+		if (ret == call1) {
+			obj_hold(ret);
+			return ret;
 		}
-		else {
-			rwlock_lock_w(&(*ret1)->master_lock);
-			if (rwlock_trylock_w(&(*ret2)->master_lock)) {
-				// try again
-				rwlock_unlock_w(&(*ret1)->master_lock);
-				continue;
-			}
 
-			ret = CG2_OK;
-			obj_hold(*ret1);
-			obj_hold(*ret2);
+		if (!rwlock_trylock_w(&ret->master_lock)) {
+			obj_hold(ret);
+			return ret;
 		}
 
-		break;
+		// try again after releasing lock
+		rwlock_unlock_w(&call1->master_lock);
+		rwlock_lock_w(&call1->master_lock);
 	}
-
-	log_info_call(*ret1);
-	return ret;
 }
 
 static gboolean fragment_move(str *key, fragment_q *q, void *c) {
@@ -5547,9 +5536,15 @@ static gboolean fragment_move(str *key, fragment_q *q, void *c) {
 	return TRUE;
 }
 
-// both calls must be locked and a reference held. call2 will be released and set to NULL upon return
-bool call_merge(call_t *call, call_t **call2p) {
-	call_t *call2 = *call2p;
+// both calls must be locked and a reference held. call2 reference will be released if successful
+bool call_merge(call_t *call, call_t *call2) {
+	if (!call2)
+		return true;
+
+	if (call == call2) {
+		obj_put(call2);
+		return true;
+	}
 
 	// chcek for tag collisions: duplicate tags are a failure
 	for (auto_iter(l, call2->monologues.head); l; l = l->next) {
@@ -5650,7 +5645,6 @@ bool call_merge(call_t *call, call_t **call2p) {
 
 	rwlock_unlock_w(&call2->master_lock);
 	obj_release(call2);
-	*call2p = NULL;
 
 	return true;
 }

daemon/call_interfaces.c

@@ -4423,18 +4423,13 @@ static const char *call_inject_ng(ng_command_ctx_t *ctx, bool start) {
 	if (!parser->dict_get_str(input, "source-call-id", &source_call_id))
 		source_call_id = flags.call_id;
 
-	if (str_cmp_str(&source_call_id, &flags.call_id)) {
-		call_get2_ret_t ret = call_get2(&call, &call2, &flags.call_id, &source_call_id);
-		if (ret == CG2_NF1)
-			return "Unknown call-ID";
-		if (ret == CG2_NF2)
-			return "Unknown source call-ID";
-	}
-	else {
-		call = call_get(&flags.call_id);
-		if (!call)
-			return "Unknown call-ID";
-	}
+	call = call_get(&flags.call_id);
+	if (!call)
+		return "Unknown call-ID";
+
+	call2 = call_get2(call, &source_call_id);
+	if (!call2)
+		return "Unknown source call-ID";
 
 	struct call_monologue *dst_ml = call_get_monologue(call, &flags.to_tag);
 	if (!dst_ml)
@@ -4447,10 +4442,9 @@ static const char *call_inject_ng(ng_command_ctx_t *ctx, bool start) {
 	if (src_ml == dst_ml)
 		return "Trying to inject to self";
 
-	if (call2) {
-		if (!call_merge(call, &call2))
-			return "Failed to merge two calls into one";
-	}
+	if (!call_merge(call, call2))
+		return "Failed to merge two calls into one";
+	call2 = NULL;
 
 	int ret = start
 		? monologue_inject_start(src_ml, dst_ml, &flags)
@@ -4485,34 +4479,32 @@ const char *call_connect_ng(ng_command_ctx_t *ctx) {
 	if (!flags.to_tag.s)
 		return "No to-tag in message";
 
-	if (flags.to_call_id.s) {
-		call_get2_ret_t ret = call_get2(&call, &call2, &flags.call_id, &flags.to_call_id);
-		if (ret == CG2_NF1)
-			return "Unknown call-ID";
-		if (ret == CG2_NF2)
+	call = call_get(&flags.call_id);
+	if (!call)
+		return "Unknown call-ID";
+
+	if (flags.to_call_id.len) {
+		call2 = call_get2(call, &flags.to_call_id);
+		if (!call2)
 			return "Unknown to-tag call-ID";
 	}
-	else {
-		call = call_get(&flags.call_id);
-		if (!call)
-			return "Unknown call-ID";
-	}
+	else
+		call2 = obj_get(call);
 
 	struct call_monologue *src_ml = call_get_or_create_monologue(call, &flags.from_tag);
 	if (!src_ml)
 		return "From-tag not found";
 
-	struct call_monologue *dest_ml = call_get_or_create_monologue(call2 ?: call, &flags.to_tag);
+	struct call_monologue *dest_ml = call_get_or_create_monologue(call2, &flags.to_tag);
 	if (!dest_ml)
 		return "To-tag not found";
 
 	if (src_ml == dest_ml)
 		return "Trying to connect to self"; // XXX should this be allowed?
 
-	if (call2) {
-		if (!call_merge(call, &call2))
-			return "Failed to merge two calls into one";
-	}
+	if (!call_merge(call, call2))
+		return "Failed to merge two calls into one";
+	call2 = NULL; // reference released
 
 	dialogue_connect(src_ml, dest_ml, &flags);
 

include/call.h

@@ -881,11 +881,10 @@ struct call_media *call_make_transform_media(struct call_monologue *ml, const st
 		const str *media_id, const endpoint_t *remote, const str *interface);
 __attribute__((nonnull(1)))
 call_t *call_get(const str *callid);
-typedef enum { CG2_OK, CG2_NF1, CG2_NF2, CG2_SAME } call_get2_ret_t;
-__attribute__((nonnull(1, 2, 3, 4)))
-call_get2_ret_t call_get2(call_t **, call_t **, const str *, const str *);
 __attribute__((nonnull(1, 2)))
-bool call_merge(call_t *, call_t **);
+call_t *call_get2(call_t *, const str *);
+__attribute__((nonnull(1)))
+bool call_merge(call_t *, call_t *);
 __attribute__((nonnull(2, 3)))
 int monologue_offer_answer(struct call_monologue *monologues[2], sdp_streams_q *streams, sdp_ng_flags *flags);
 __attribute__((nonnull(1, 2, 3, 4)))