[Asterisk-code-review] DNS: Fix some corner cases. (asterisk[master])

[Mark Michelson]

Mark Michelson has posted comments on this change.

Change subject: DNS: Fix some corner cases.
......................................................................


Patch Set 1: Code-Review-1

(1 comment)

https://gerrit.asterisk.org/#/c/626/1/main/dns_query_set.c
File main/dns_query_set.c:

Line 196: 		dns_query_set_callback(query->query);
        : 	}
        : 	if (!idx) {
        : 		/*
        : 		 * There were no queries in the set;
        : 		 * therefore all queries are "completed".
        : 		 * Invoke the final callback.
        : 		 */
        : 		query_set->callback(query_set);
        : 		ao2_cleanup(query_set->user_data);
        : 		query_set->user_data = NULL;
        : 	}
        : 	ao2_ref(query_set, -1);
I think this is going to cause problems, especially when ast_query_set_resolve() is called. ast_query_set_resolve() uses ast_query_set_resolve_async() in combination with an ast_cond_t in order to perform the synchronous resolution. If there are no items in the query set, then what will happen is that the synchronous callback will be called, which will signal the condition. Then, after that, ast_query_set_resolve() will perform an ast_cond_wait() on a condition that will never be signaled since it already has been signaled.

Note that I highlighted a section above your addition because it appears that this was already being done, and that is an error, too.

This finding can be verified by adding a new test case to the query set unit tests where a query set with no queries is resolved synchronously.


-- 
To view, visit https://gerrit.asterisk.org/626
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie8be8347d0992e93946d72b6e7b1299727b038f2
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-HasComments: Yes